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

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

[Florian Fainelli]

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

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

[Florian Fainelli]

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

Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
 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

[PATCH 2/2] scsi: qedi: Fix SYSFS_FLAG_FW_SEL_BOOT formatting

[Florian Fainelli]

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 <f.fainelli@gmail.com>
---
 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

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

[Manish Rangankar]

> -----Original Message-----
> From: Florian Fainelli <f.fainelli@gmail.com>
> Sent: Friday, November 26, 2021 10:45 AM
> To: linux-kernel@vger.kernel.org
> Cc: Florian Fainelli <f.fainelli@gmail.com>; Nilesh Javali <njavali@marvell.com>;
> Manish Rangankar <mrangankar@marvell.com>; GR-QLogic-Storage-Upstream
> <GR-QLogic-Storage-Upstream@marvell.com>; James E.J. Bottomley
> <jejb@linux.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>;
> open list:QLOGIC QL41xxx ISCSI DRIVER <linux-scsi@vger.kernel.org>
> 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 <f.fainelli@gmail.com>
> ---
>  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 ?

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

[Manish Rangankar]

> -----Original Message-----
> From: Florian Fainelli <f.fainelli@gmail.com>
> Sent: Friday, November 26, 2021 10:45 AM
> To: linux-kernel@vger.kernel.org
> Cc: Florian Fainelli <f.fainelli@gmail.com>; kernel test robot <lkp@intel.com>;
> Nilesh Javali <njavali@marvell.com>; Manish Rangankar
> <mrangankar@marvell.com>; GR-QLogic-Storage-Upstream <GR-QLogic-
> Storage-Upstream@marvell.com>; James E.J. Bottomley <jejb@linux.ibm.com>;
> Martin K. Petersen <martin.petersen@oracle.com>; open list:QLOGIC QL41xxx
> ISCSI DRIVER <linux-scsi@vger.kernel.org>
> 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 <lkp@intel.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  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 <mrangankar@marvell.com>

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

[Florian Fainelli]

On 11/26/2021 12:43 AM, Manish Rangankar wrote:
> 
> 
>> -----Original Message-----
>> From: Florian Fainelli <f.fainelli@gmail.com>
>> Sent: Friday, November 26, 2021 10:45 AM
>> To: linux-kernel@vger.kernel.org
>> Cc: Florian Fainelli <f.fainelli@gmail.com>; Nilesh Javali <njavali@marvell.com>;
>> Manish Rangankar <mrangankar@marvell.com>; GR-QLogic-Storage-Upstream
>> <GR-QLogic-Storage-Upstream@marvell.com>; James E.J. Bottomley
>> <jejb@linux.ibm.com>; Martin K. Petersen <martin.petersen@oracle.com>;
>> open list:QLOGIC QL41xxx ISCSI DRIVER <linux-scsi@vger.kernel.org>
>> 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 <f.fainelli@gmail.com>
>> ---
>>   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

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

[Florian Fainelli]

On 11/26/2021 12:52 AM, Manish Rangankar wrote:
> 
> 
>> -----Original Message-----
>> From: Florian Fainelli <f.fainelli@gmail.com>
>> Sent: Friday, November 26, 2021 10:45 AM
>> To: linux-kernel@vger.kernel.org
>> Cc: Florian Fainelli <f.fainelli@gmail.com>; kernel test robot <lkp@intel.com>;
>> Nilesh Javali <njavali@marvell.com>; Manish Rangankar
>> <mrangankar@marvell.com>; GR-QLogic-Storage-Upstream <GR-QLogic-
>> Storage-Upstream@marvell.com>; James E.J. Bottomley <jejb@linux.ibm.com>;
>> Martin K. Petersen <martin.petersen@oracle.com>; open list:QLOGIC QL41xxx
>> ISCSI DRIVER <linux-scsi@vger.kernel.org>
>> 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 <lkp@intel.com>
>> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
>> ---
>>   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 <mrangankar@marvell.com>

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

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

[Manish Rangankar]

> >>   	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 <mrangankar@marvell.com>
> 
> 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.