Re: [PATCH] qla2xxx: Fix compile warning

[James Bottomley]

OK, the subject is technically true, but to most people "fix compile
warning" means yet another pointless patch.  This patch isn't
pointless, though, is it?  The driver is actually broken on big endian
systems without it, so the subject should probably say this because
it's important for distros triaging whether to backport this fix if
they backport the nvme components.

On Fri, 2017-06-30 at 16:56 -0700, Himanshu Madhani wrote:
> drivers/scsi/qla2xxx/qla_nvme.c: In function 'qla2x00_start_nvme_mq':
> include/uapi/linux/byteorder/big_endian.h:32:26: warning: large
> integer implicitly truncated to unsigned type [-Woverflow]
>  #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))

This text doesn't explain why you've fixed the bug this way.  It has to
do with the 32 byte reads the card does for the mailbox, so you should
explain what the actual problem the compile warning picked up is and
why you fixed it by doing an entire 32 bit write.

Preferably ASAP because the merge window will open on Monday.

James

[PATCH] qla2xxx: Fix compile warning

[Himanshu Madhani]

drivers/scsi/qla2xxx/qla_nvme.c: In function 'qla2x00_start_nvme_mq':
include/uapi/linux/byteorder/big_endian.h:32:26: warning: large integer
implicitly truncated to unsigned type [-Woverflow]
 #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))

Signed-off-by: Himanshu Madhani 
---
 drivers/scsi/qla2xxx/qla_nvme.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
index 1da8fa8f641d..367cf8613b15 100644
--- a/drivers/scsi/qla2xxx/qla_nvme.c
+++ b/drivers/scsi/qla2xxx/qla_nvme.c
@@ -441,7 +441,8 @@ static int qla2x00_start_nvme_mq(srb_t *sp)
req->ring_ptr++;
}
cont_pkt = (cont_a64_entry_t *)req->ring_ptr;
-   cont_pkt->entry_type = cpu_to_le32(CONTINUE_A64_TYPE);
+   *((uint32_t *)(_pkt->entry_type)) =
+   cpu_to_le32(CONTINUE_A64_TYPE);
 
cur_dsd = (uint32_t *)cont_pkt->dseg_0_address;
avail_dsds = 5;
-- 
2.11.0

Re: [PATCH] qla2xxx: Fix compile warning

[Madhani, Himanshu]

> On Jun 28, 2017, at 3:12 PM, James Bottomley 
>  wrote:
> 
> On Wed, 2017-06-28 at 15:00 -0700, Himanshu Madhani wrote:
>> Fixes following warning
>> 
>> drivers/scsi/qla2xxx/qla_nvme.c: In function 'qla2x00_start_nvme_mq':
>> include/uapi/linux/byteorder/big_endian.h:32:26: warning: large
>> integer
>> implicitly truncated to unsigned type [-Woverflow]
>>  #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
>> 
>> Signed-off-by: Himanshu Madhani 
>> ---
>>  drivers/scsi/qla2xxx/qla_nvme.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>> 
>> diff --git a/drivers/scsi/qla2xxx/qla_nvme.c
>> b/drivers/scsi/qla2xxx/qla_nvme.c
>> index 1da8fa8f641d..14e25e32e622 100644
>> --- a/drivers/scsi/qla2xxx/qla_nvme.c
>> +++ b/drivers/scsi/qla2xxx/qla_nvme.c
>> @@ -441,7 +441,7 @@ static int qla2x00_start_nvme_mq(srb_t *sp)
>>  req->ring_ptr++;
>>  }
>>  cont_pkt = (cont_a64_entry_t *)req-
>>> ring_ptr;
>> -cont_pkt->entry_type =
>> cpu_to_le32(CONTINUE_A64_TYPE);
>> +cont_pkt->entry_type = CONTINUE_A64_TYPE;
>>  
> 
> Well the code is definitely buggy, because it will load 0 on a BE
> system.  However, are you sure this is the right fix?  I thought the
> qlogic engine did groups of 32 bit words, which is why the pre-nvme
> code loads this field as:
> 
>   *((uint32_t *)(_pkt->entry_type)) = cpu_to_le32(CONTINUE_TYPE);
> 
> James

Agree. I need to go back and check which would be right fix. 

Once i’ve confirmed it works on both platform, I’ll send out new patch.

Thanks,
- Himanshu

Re: [PATCH] qla2xxx: Fix compile warning

[James Bottomley]

On Wed, 2017-06-28 at 15:00 -0700, Himanshu Madhani wrote:
> Fixes following warning
> 
> drivers/scsi/qla2xxx/qla_nvme.c: In function 'qla2x00_start_nvme_mq':
> include/uapi/linux/byteorder/big_endian.h:32:26: warning: large
> integer
> implicitly truncated to unsigned type [-Woverflow]
>  #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))
> 
> Signed-off-by: Himanshu Madhani 
> ---
>  drivers/scsi/qla2xxx/qla_nvme.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/qla2xxx/qla_nvme.c
> b/drivers/scsi/qla2xxx/qla_nvme.c
> index 1da8fa8f641d..14e25e32e622 100644
> --- a/drivers/scsi/qla2xxx/qla_nvme.c
> +++ b/drivers/scsi/qla2xxx/qla_nvme.c
> @@ -441,7 +441,7 @@ static int qla2x00_start_nvme_mq(srb_t *sp)
>   req->ring_ptr++;
>   }
>   cont_pkt = (cont_a64_entry_t *)req-
> >ring_ptr;
> - cont_pkt->entry_type =
> cpu_to_le32(CONTINUE_A64_TYPE);
> + cont_pkt->entry_type = CONTINUE_A64_TYPE;
> 

Well the code is definitely buggy, because it will load 0 on a BE
system.  However, are you sure this is the right fix?  I thought the
qlogic engine did groups of 32 bit words, which is why the pre-nvme
code loads this field as:

*((uint32_t *)(_pkt->entry_type)) = cpu_to_le32(CONTINUE_TYPE);

James

[PATCH] qla2xxx: Fix compile warning

[Himanshu Madhani]

Fixes following warning

drivers/scsi/qla2xxx/qla_nvme.c: In function 'qla2x00_start_nvme_mq':
include/uapi/linux/byteorder/big_endian.h:32:26: warning: large integer
implicitly truncated to unsigned type [-Woverflow]
 #define __cpu_to_le32(x) ((__force __le32)__swab32((x)))

Signed-off-by: Himanshu Madhani 
---
 drivers/scsi/qla2xxx/qla_nvme.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qla2xxx/qla_nvme.c b/drivers/scsi/qla2xxx/qla_nvme.c
index 1da8fa8f641d..14e25e32e622 100644
--- a/drivers/scsi/qla2xxx/qla_nvme.c
+++ b/drivers/scsi/qla2xxx/qla_nvme.c
@@ -441,7 +441,7 @@ static int qla2x00_start_nvme_mq(srb_t *sp)
req->ring_ptr++;
}
cont_pkt = (cont_a64_entry_t *)req->ring_ptr;
-   cont_pkt->entry_type = cpu_to_le32(CONTINUE_A64_TYPE);
+   cont_pkt->entry_type = CONTINUE_A64_TYPE;
 
cur_dsd = (uint32_t *)cont_pkt->dseg_0_address;
avail_dsds = 5;
-- 
2.13.1.452.g97e2ff464

Re: [PATCH] qla2xxx: Fix compile warning

[Martin K. Petersen]

Himanshu,

> Fixes following 0-day kernel build warnings

Applied to 4.13/scsi-queue. Thank you!

-- 
Martin K. Petersen  Oracle Linux Engineering

[PATCH] qla2xxx: Fix compile warning

[Himanshu Madhani]

Fixes following 0-day kernel build warnings

drivers/scsi/qla2xxx/qla_init.c:6407:50: warning: format '%lx' expects
argument of type 'long unsigned int', but argument 5 has type 'unsigned
int' [-Wformat=]
drivers/scsi/qla2xxx/qla_init.c:6709:50: warning: format '%lx'
expects argument of type 'long unsigned int', but argument 5 has
type 'unsigned int' [-Wformat=]

Fixes: b95b9452aacf ("scsi: qla2xxx: Fix crash due to mismatch mumber of Q-pair 
creation for Multi queue")
Signed-off-by: Himanshu Madhani 
Signed-off-by: Giridhar Malavali 
---
 drivers/scsi/qla2xxx/qla_init.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_init.c b/drivers/scsi/qla2xxx/qla_init.c
index 436968ad4484..730e7fe4344a 100644
--- a/drivers/scsi/qla2xxx/qla_init.c
+++ b/drivers/scsi/qla2xxx/qla_init.c
@@ -6404,7 +6404,7 @@ qla24xx_load_risc_flash(scsi_qla_host_t *vha, uint32_t 
*srisc_addr,
"-> template size %x bytes\n", dlen);
if (dlen > risc_size * sizeof(*dcode)) {
ql_log(ql_log_warn, vha, 0x0167,
-   "Failed fwdump template exceeds array by %lx bytes\n",
+   "Failed fwdump template exceeds array by %zx bytes\n",
(size_t)(dlen - risc_size * sizeof(*dcode)));
goto default_template;
}
@@ -6706,7 +6706,7 @@ qla24xx_load_risc_blob(scsi_qla_host_t *vha, uint32_t 
*srisc_addr)
"-> template size %x bytes\n", dlen);
if (dlen > risc_size * sizeof(*fwcode)) {
ql_log(ql_log_warn, vha, 0x0177,
-   "Failed fwdump template exceeds array by %lx bytes\n",
+   "Failed fwdump template exceeds array by %zx bytes\n",
(size_t)(dlen - risc_size * sizeof(*fwcode)));
goto default_template;
}
-- 
2.12.0