2021-11-26 05:17:40

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH 0/2] scsi: qedi: Couple of warning fixes

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.

Florian Fainelli (2):
scsi: qedi: Remove set but unused 'page' variable
scsi: qedi: Fix SYSFS_FLAG_FW_SEL_BOOT formatting

drivers/scsi/qedi/qedi_main.c | 5 +----
1 file changed, 1 insertion(+), 4 deletions(-)

--
2.25.1



2021-11-26 05:17:44

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH 1/2] scsi: qedi: Remove set but unused 'page' variable

The variable page is set but never used throughout qedi_alloc_bdq()
therefore remove it.

Reported-by: kernel test robot <[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


2021-11-26 05:17:46

by Florian Fainelli

[permalink] [raw]
Subject: [PATCH 2/2] scsi: qedi: Fix SYSFS_FLAG_FW_SEL_BOOT formatting

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 use %d since this is a plain integer.

Signed-off-by: Florian Fainelli <[email protected]>
---
drivers/scsi/qedi/qedi_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c
index f1c933070884..367a0337b53e 100644
--- a/drivers/scsi/qedi/qedi_main.c
+++ b/drivers/scsi/qedi/qedi_main.c
@@ -2254,7 +2254,7 @@ qedi_show_boot_tgt_info(struct qedi_ctx *qedi, int type,
mchap_secret);
break;
case ISCSI_BOOT_TGT_FLAGS:
- rc = snprintf(buf, 3, "%hhd\n", SYSFS_FLAG_FW_SEL_BOOT);
+ rc = snprintf(buf, 3, "%d\n", SYSFS_FLAG_FW_SEL_BOOT);
break;
case ISCSI_BOOT_TGT_NIC_ASSOC:
rc = snprintf(buf, 3, "0\n");
--
2.25.1


2021-11-26 08:46:17

by Manish Rangankar

[permalink] [raw]
Subject: RE: [EXT] [PATCH 2/2] scsi: qedi: Fix SYSFS_FLAG_FW_SEL_BOOT formatting



> -----Original Message-----
> From: Florian Fainelli <[email protected]>
> Sent: Friday, November 26, 2021 10:45 AM
> To: [email protected]
> Cc: Florian Fainelli <[email protected]>; Nilesh Javali <[email protected]>;
> Manish Rangankar <[email protected]>; GR-QLogic-Storage-Upstream
> <[email protected]>; James E.J. Bottomley
> <[email protected]>; Martin K. Petersen <[email protected]>;
> open list:QLOGIC QL41xxx ISCSI DRIVER <[email protected]>
> Subject: [EXT] [PATCH 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 use %d since this is a plain integer.
>
> Signed-off-by: Florian Fainelli <[email protected]>
> ---
> drivers/scsi/qedi/qedi_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c index
> f1c933070884..367a0337b53e 100644
> --- a/drivers/scsi/qedi/qedi_main.c
> +++ b/drivers/scsi/qedi/qedi_main.c
> @@ -2254,7 +2254,7 @@ qedi_show_boot_tgt_info(struct qedi_ctx *qedi, int
> type,
> mchap_secret);
> break;
> case ISCSI_BOOT_TGT_FLAGS:
> - rc = snprintf(buf, 3, "%hhd\n", SYSFS_FLAG_FW_SEL_BOOT);
> + rc = snprintf(buf, 3, "%d\n", SYSFS_FLAG_FW_SEL_BOOT);
> break;
> case ISCSI_BOOT_TGT_NIC_ASSOC:
> rc = snprintf(buf, 3, "0\n");
> --
> 2.25.1

SYSFS_FLAG_FW_SEL_BOOT is always going to have value 2, that's why it is given %hhd to limit the size to 1 byte.
Is there other way to suppress this warning, such as typecasting or any other ?


2021-11-26 08:54:48

by Manish Rangankar

[permalink] [raw]
Subject: RE: [EXT] [PATCH 1/2] scsi: qedi: Remove set but unused 'page' variable



> -----Original Message-----
> From: Florian Fainelli <[email protected]>
> Sent: Friday, November 26, 2021 10:45 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 1/2] scsi: qedi: Remove set but unused 'page' variable
>
> External Email
>
> ----------------------------------------------------------------------
> The variable page is set but never used throughout qedi_alloc_bdq() therefore
> remove it.
>
> Reported-by: kernel test robot <[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

Thanks,
Acked-by: Manish Rangankar <[email protected]>

2021-11-27 05:35:19

by Florian Fainelli

[permalink] [raw]
Subject: Re: [EXT] [PATCH 2/2] scsi: qedi: Fix SYSFS_FLAG_FW_SEL_BOOT formatting



On 11/26/2021 12:43 AM, Manish Rangankar wrote:
>
>
>> -----Original Message-----
>> From: Florian Fainelli <[email protected]>
>> Sent: Friday, November 26, 2021 10:45 AM
>> To: [email protected]
>> Cc: Florian Fainelli <[email protected]>; Nilesh Javali <[email protected]>;
>> Manish Rangankar <[email protected]>; GR-QLogic-Storage-Upstream
>> <[email protected]>; James E.J. Bottomley
>> <[email protected]>; Martin K. Petersen <[email protected]>;
>> open list:QLOGIC QL41xxx ISCSI DRIVER <[email protected]>
>> Subject: [EXT] [PATCH 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 use %d since this is a plain integer.
>>
>> Signed-off-by: Florian Fainelli <[email protected]>
>> ---
>> drivers/scsi/qedi/qedi_main.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/scsi/qedi/qedi_main.c b/drivers/scsi/qedi/qedi_main.c index
>> f1c933070884..367a0337b53e 100644
>> --- a/drivers/scsi/qedi/qedi_main.c
>> +++ b/drivers/scsi/qedi/qedi_main.c
>> @@ -2254,7 +2254,7 @@ qedi_show_boot_tgt_info(struct qedi_ctx *qedi, int
>> type,
>> mchap_secret);
>> break;
>> case ISCSI_BOOT_TGT_FLAGS:
>> - rc = snprintf(buf, 3, "%hhd\n", SYSFS_FLAG_FW_SEL_BOOT);
>> + rc = snprintf(buf, 3, "%d\n", SYSFS_FLAG_FW_SEL_BOOT);
>> break;
>> case ISCSI_BOOT_TGT_NIC_ASSOC:
>> rc = snprintf(buf, 3, "0\n");
>> --
>> 2.25.1
>
> SYSFS_FLAG_FW_SEL_BOOT is always going to have value 2, that's why it is given %hhd to limit the size to 1 byte.
> Is there other way to suppress this warning, such as typecasting or any other ?

Yes typecasting would work, if you are fine with that.
--
Florian

2021-11-27 05:35:25

by Florian Fainelli

[permalink] [raw]
Subject: Re: [EXT] [PATCH 1/2] scsi: qedi: Remove set but unused 'page' variable



On 11/26/2021 12:52 AM, Manish Rangankar wrote:
>
>
>> -----Original Message-----
>> From: Florian Fainelli <[email protected]>
>> Sent: Friday, November 26, 2021 10:45 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 1/2] scsi: qedi: Remove set but unused 'page' variable
>>
>> External Email
>>
>> ----------------------------------------------------------------------
>> The variable page is set but never used throughout qedi_alloc_bdq() therefore
>> remove it.
>>
>> Reported-by: kernel test robot <[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
>
> Thanks,
> Acked-by: Manish Rangankar <[email protected]>

Thanks for taking a look, does not that make the loop iterating the list
even more useless now, though? Should not page have been used for
something in that function?
--
Florian

2021-11-27 09:17:07

by Manish Rangankar

[permalink] [raw]
Subject: RE: [EXT] [PATCH 1/2] scsi: qedi: Remove set but unused 'page' variable

> >> 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
> >
> > Thanks,
> > Acked-by: Manish Rangankar <[email protected]>
>
> Thanks for taking a look, does not that make the loop iterating the list even
> more useless now, though? Should not page have been used for something in
> that function?
> --

We need list to build structure of bdq list in firmware understandable format.