These warnings started to show up after enabling PCI on BMIPS (32-bit
MIPS architecture) and were reported by the kbuild robot.
I don't know whether they are technically correct, in particular the
unused 'page' variable might be unveiling a genuine bug whereby it
should have been used. Please review.
Changes in v2:
- added Acked-by to patch #1
- changed SYSFS_FLAG_FW_SEL_BOOT to contain the typecasting and not
change the way it is formatted before sysfs printing
Florian Fainelli (2):
scsi: qedi: Remove set but unused 'page' variable
scsi: qedi: Fix SYSFS_FLAG_FW_SEL_BOOT formatting
drivers/scsi/qedi/qedi.h | 2 +-
drivers/scsi/qedi/qedi_main.c | 3 ---
2 files changed, 1 insertion(+), 4 deletions(-)
--
2.25.1
The variable page is set but never used throughout qedi_alloc_bdq()
therefore remove it.
Reported-by: kernel test robot <[email protected]>
Acked-by: Manish Rangankar <[email protected]>
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/scsi/qedi/qedi_main.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
index 1dec814d8788..f1c933070884 100644
--- a/drivers/scsi/qedi/qedi_main.c
+++ b/drivers/scsi/qedi/qedi_main.c
@@ -1538,7 +1538,6 @@ static int qedi_alloc_bdq(struct qedi_ctx *qedi)
int i;
struct scsi_bd *pbl;
u64 *list;
- dma_addr_t page;
/* Alloc dma memory for BDQ buffers */
for (i = 0; i < QEDI_BDQ_NUM; i++) {
@@ -1608,11 +1607,9 @@ static int qedi_alloc_bdq(struct qedi_ctx *qedi)
qedi->bdq_pbl_list_num_entries = qedi->bdq_pbl_mem_size /
QEDI_PAGE_SIZE;
list = (u64 *)qedi->bdq_pbl_list;
- page = qedi->bdq_pbl_list_dma;
for (i = 0; i < qedi->bdq_pbl_list_num_entries; i++) {
*list = qedi->bdq_pbl_dma;
list++;
- page += QEDI_PAGE_SIZE;
}
return 0;
--
2.25.1
The format used for formatting SYSFS_FLAG_FW_SEL_BOOT creates the
following warning:
drivers/scsi/qedi/qedi_main.c:2259:35: warning: format specifies type
'char' but the argument has type 'int' [-Wformat]
rc = snprintf(buf, 3, "%hhd\n",
SYSFS_FLAG_FW_SEL_BOOT);
Fix this to cast the constant as an u8 since the intention is to print
it via sysfs as a byte.
Reported-by: kernel test robot <[email protected]>
Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/scsi/qedi/qedi.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/scsi/qedi/qedi.h b/drivers/scsi/qedi/qedi.h
index ce199a7a16b8..421b3a69fd37 100644
--- a/drivers/scsi/qedi/qedi.h
+++ b/drivers/scsi/qedi/qedi.h
@@ -358,7 +358,7 @@ struct qedi_ctx {
bool use_fast_sge;
atomic_t num_offloads;
-#define SYSFS_FLAG_FW_SEL_BOOT 2
+#define SYSFS_FLAG_FW_SEL_BOOT (u8)2
#define IPV6_LEN 41
#define IPV4_LEN 17
struct iscsi_boot_kset *boot_kset;
--
2.25.1
> -----Original Message-----
> From: Florian Fainelli <[email protected]>
> Sent: Saturday, November 27, 2021 1:47 AM
> To: [email protected]
> Cc: Florian Fainelli <[email protected]>; kernel test robot <[email protected]>;
> Nilesh Javali <[email protected]>; Manish Rangankar
> <[email protected]>; GR-QLogic-Storage-Upstream <GR-QLogic-
> [email protected]>; James E.J. Bottomley <[email protected]>;
> Martin K. Petersen <[email protected]>; open list:QLOGIC QL41xxx
> ISCSI DRIVER <[email protected]>
> Subject: [EXT] [PATCH v2 2/2] scsi: qedi: Fix SYSFS_FLAG_FW_SEL_BOOT
> formatting
>
> External Email
>
> ----------------------------------------------------------------------
> The format used for formatting SYSFS_FLAG_FW_SEL_BOOT creates the
> following warning:
>
> drivers/scsi/qedi/qedi_main.c:2259:35: warning: format specifies type 'char' but
> the argument has type 'int' [-Wformat]
> rc = snprintf(buf, 3, "%hhd\n", SYSFS_FLAG_FW_SEL_BOOT);
>
> Fix this to cast the constant as an u8 since the intention is to print it via sysfs as a
> byte.
>
> Reported-by: kernel test robot <[email protected]>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> drivers/scsi/qedi/qedi.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/qedi/qedi.h b/drivers/scsi/qedi/qedi.h index
> ce199a7a16b8..421b3a69fd37 100644
> --- a/drivers/scsi/qedi/qedi.h
> +++ b/drivers/scsi/qedi/qedi.h
> @@ -358,7 +358,7 @@ struct qedi_ctx {
> bool use_fast_sge;
>
> atomic_t num_offloads;
> -#define SYSFS_FLAG_FW_SEL_BOOT 2
> +#define SYSFS_FLAG_FW_SEL_BOOT (u8)2
> #define IPV6_LEN 41
> #define IPV4_LEN 17
> struct iscsi_boot_kset *boot_kset;
> --
> 2.25.1
Acked-by: Manish Rangankar <[email protected]>
Florian,
> The format used for formatting SYSFS_FLAG_FW_SEL_BOOT creates the
> following warning:
>
> drivers/scsi/qedi/qedi_main.c:2259:35: warning: format specifies type
> 'char' but the argument has type 'int' [-Wformat]
> rc = snprintf(buf, 3, "%hhd\n",
> SYSFS_FLAG_FW_SEL_BOOT);
>
> Fix this to cast the constant as an u8 since the intention is to print
> it via sysfs as a byte.
The other occurrences of SYSFS_FLAG_FW_SEL_BOOT use "%d". Since %hh is
deprecated I suggest you just fix the snprintf().
--
Martin K. Petersen Oracle Linux Engineering
Florian,
> The variable page is set but never used throughout qedi_alloc_bdq()
> therefore remove it.
Applied to 5.17/scsi-staging, thanks!
--
Martin K. Petersen Oracle Linux Engineering
On 11/29/2021 7:59 PM, Martin K. Petersen wrote:
>
> Florian,
>
>> The format used for formatting SYSFS_FLAG_FW_SEL_BOOT creates the
>> following warning:
>>
>> drivers/scsi/qedi/qedi_main.c:2259:35: warning: format specifies type
>> 'char' but the argument has type 'int' [-Wformat]
>> rc = snprintf(buf, 3, "%hhd\n",
>> SYSFS_FLAG_FW_SEL_BOOT);
>>
>> Fix this to cast the constant as an u8 since the intention is to print
>> it via sysfs as a byte.
>
> The other occurrences of SYSFS_FLAG_FW_SEL_BOOT use "%d". Since %hh is
> deprecated I suggest you just fix the snprintf().
That was what v1 did here:
https://lkml.org/lkml/2021/11/26/9
however Manish seemed to want that flag to be printed as a byte I am
fine either way.
--
Florian
Florian,
>> The other occurrences of SYSFS_FLAG_FW_SEL_BOOT use "%d". Since %hh
>> is deprecated I suggest you just fix the snprintf().
>
> That was what v1 did here:
>
> https://lkml.org/lkml/2021/11/26/9
>
> however Manish seemed to want that flag to be printed as a byte I am
> fine either way.
Not sure I understand the concern since this is a constant which will
always be "2".
But if you must cast, do it in snprintf() and not in the macro
definition. checkpatch also complains about the cast.
I would prefer for all instances of this to be consistent, though. So
whatever you do, please also fix qla4xxx.
--
Martin K. Petersen Oracle Linux Engineering
On Fri, 26 Nov 2021 12:17:06 -0800, Florian Fainelli wrote:
> These warnings started to show up after enabling PCI on BMIPS (32-bit
> MIPS architecture) and were reported by the kbuild robot.
>
> I don't know whether they are technically correct, in particular the
> unused 'page' variable might be unveiling a genuine bug whereby it
> should have been used. Please review.
>
> [...]
Applied to 5.17/scsi-queue, thanks!
[1/2] scsi: qedi: Remove set but unused 'page' variable
https://git.kernel.org/mkp/scsi/c/6d8619f034f0
--
Martin K. Petersen Oracle Linux Engineering