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

[PATCH v2 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.

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

[PATCH v2 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>
Acked-by: Manish Rangankar <mrangankar@marvell.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 v2 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 cast the constant as an u8 since the intention is to print
it via sysfs as a byte.

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

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

[Manish Rangankar]

> -----Original Message-----
> From: Florian Fainelli <f.fainelli@gmail.com>
> Sent: Saturday, November 27, 2021 1:47 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 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 <lkp@intel.com>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
> ---
>  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 <mrangankar@marvell.com>

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

[Martin K. Petersen]

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

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

[Martin K. Petersen]

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

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

[Florian Fainelli]

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

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

[Martin K. Petersen]

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

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

[Martin K. Petersen]

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