[PATCH 1/4] scsi: lpfc: fix unused variable warning

[PATCH 1/4] scsi: lpfc: fix unused variable warning

[Arnd Bergmann]

The newly introduced 'cpu' variable is only used inside of an optional
block, so we get a warning without CONFIG_SCSI_LPFC_DEBUG_FS:

drivers/scsi/lpfc/lpfc_nvme.c: In function 'lpfc_nvme_io_cmd_wqe_cmpl':
drivers/scsi/lpfc/lpfc_nvme.c:968:30: error: unused variable 'cpu' [-Werror=unused-variable]
  uint32_t code, status, idx, cpu;

Move the declaration into the same block to avoid the warning.

Fixes: 63df6d637e33 ("scsi: lpfc: Adapt cpucheck debugfs logic to Hardware Queues")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/lpfc/lpfc_nvme.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/scsi/lpfc/lpfc_nvme.c b/drivers/scsi/lpfc/lpfc_nvme.c
index 72cb8ad9027b..d16ca413110d 100644
--- a/drivers/scsi/lpfc/lpfc_nvme.c
+++ b/drivers/scsi/lpfc/lpfc_nvme.c
@@ -965,7 +965,7 @@ lpfc_nvme_io_cmd_wqe_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *pwqeIn,
 	struct lpfc_nodelist *ndlp;
 	struct lpfc_nvme_fcpreq_priv *freqpriv;
 	struct lpfc_nvme_lport *lport;
-	uint32_t code, status, idx, cpu;
+	uint32_t code, status, idx;
 	uint16_t cid, sqhd, data;
 	uint32_t *ptr;
 
@@ -1139,6 +1139,7 @@ lpfc_nvme_io_cmd_wqe_cmpl(struct lpfc_hba *phba, struct lpfc_iocbq *pwqeIn,
 		lpfc_nvme_ktime(phba, lpfc_ncmd);
 	}
 	if (phba->cpucheck_on & LPFC_CHECK_NVME_IO) {
+		uint32_t cpu;
 		idx = lpfc_ncmd->cur_iocbq.hba_wqidx;
 		cpu = smp_processor_id();
 		if (cpu < LPFC_CHECK_CPU_CNT) {
-- 
2.20.0

[PATCH 2/4] scsi: lpfc: fix 32-bit format string warning

[Arnd Bergmann]

On 32-bit architectures, we see a warning when %ld is used to
print a size_t:

In file included from drivers/scsi/lpfc/lpfc_init.c:62:
drivers/scsi/lpfc/lpfc_init.c: In function 'lpfc_new_io_buf':
drivers/scsi/lpfc/lpfc_logmsg.h:62:45: error: format '%ld' expects argument of type 'long int', but argument 5 has type 'unsigned int' [-Werror=format=]

This is harmless, but portable code should just use %zd to
avoid the warning.

Fixes: 0794d601d174 ("scsi: lpfc: Implement common IO buffers between NVME and SCSI")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/lpfc/lpfc_init.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/lpfc/lpfc_init.c b/drivers/scsi/lpfc/lpfc_init.c
index 3b5873f6751e..6e6bd1b3237d 100644
--- a/drivers/scsi/lpfc/lpfc_init.c
+++ b/drivers/scsi/lpfc/lpfc_init.c
@@ -4090,7 +4090,7 @@ lpfc_new_io_buf(struct lpfc_hba *phba, int num_to_alloc)
 	/* Sanity check to ensure our sizing is right for both SCSI and NVME */
 	if (sizeof(struct lpfc_io_buf) > LPFC_COMMON_IO_BUF_SZ) {
 		lpfc_printf_log(phba, KERN_ERR, LOG_FCP,
-				"6426 Common buffer size %ld exceeds %d\n",
+				"6426 Common buffer size %zd exceeds %d\n",
 				sizeof(struct lpfc_io_buf),
 				LPFC_COMMON_IO_BUF_SZ);
 		return 0;
-- 
2.20.0

Re: [PATCH 2/4] scsi: lpfc: fix 32-bit format string warning

[Martin K. Petersen]

Arnd,

> On 32-bit architectures, we see a warning when %ld is used to
> print a size_t:

Applied to 5.1/scsi-queue, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH 3/4] scsi: qla2xxx: avoid printf format warning

[Arnd Bergmann]

Depending on the target architecture and configuration, both phys_addr_t
and dma_addr_t may be smaller than 'long long', so we get a warning when
printing either of them using the %llx format string:

drivers/scsi/qla2xxx/qla_iocb.c: In function 'qla24xx_walk_and_build_prot_sglist':
drivers/scsi/qla2xxx/qla_iocb.c:1140:46: error: format '%llx' expects argument of type 'long long unsigned int', but argument 6 has type 'dma_addr_t' {aka 'unsigned int'} [-Werror=format=]
         "%s: page boundary crossing (phys=%llx len=%x)\n",
                                           ~~~^
                                           %x
         __func__, sle_phys, sg->length);
                   ~~~~~~~~
drivers/scsi/qla2xxx/qla_iocb.c:1180:29: error: format '%llx' expects argument of type 'long long unsigned int', but argument 7 has type 'dma_addr_t' {aka 'unsigned int'} [-Werror=format=]
        "%s: sg[%x] (phys=%llx sglen=%x) ldma_sg_len: %x dif_bundl_len: %x ldma_needed: %x\n",
                          ~~~^

There are special %pad and %pap format strings in Linux that we could use here,
but since the driver already does 64-bit arithmetic on the values, using a
plain 'u64' seems more consistent here.

Note: A possible related issue may be that the driver possibly checks the
wrong kind of overflow: when an IOMMU is in use, buffers that cross a 32-bit
boundary in physical addresses would still be mapped into dma addresses
within the low 4GB space, so I suspect that we actually want to check
sg_dma_address() instead of sg_phys() here.

Fixes: 50b812755e97 ("scsi: qla2xxx: Fix DMA error when the DIF sg buffer crosses 4GB boundary")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/qla2xxx/qla_iocb.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
index 63f8e3c19841..456a41d2e2c6 100644
--- a/drivers/scsi/qla2xxx/qla_iocb.c
+++ b/drivers/scsi/qla2xxx/qla_iocb.c
@@ -1132,7 +1132,7 @@ qla24xx_walk_and_build_prot_sglist(struct qla_hw_data *ha, srb_t *sp,
 	/* if initiator doing write or target doing read */
 	if (direction_to_device) {
 		for_each_sg(sgl, sg, tot_dsds, i) {
-			dma_addr_t sle_phys = sg_phys(sg);
+			u64 sle_phys = sg_phys(sg);
 
 			/* If SGE addr + len flips bits in upper 32-bits */
 			if (MSD(sle_phys + sg->length) ^ MSD(sle_phys)) {
@@ -1178,7 +1178,7 @@ qla24xx_walk_and_build_prot_sglist(struct qla_hw_data *ha, srb_t *sp,
 
 			ql_dbg(ql_dbg_tgt + ql_dbg_verbose, vha, 0xe023,
 			    "%s: sg[%x] (phys=%llx sglen=%x) ldma_sg_len: %x dif_bundl_len: %x ldma_needed: %x\n",
-			    __func__, i, sg_phys(sg), sglen, ldma_sg_len,
+			    __func__, i, (u64)sg_phys(sg), sglen, ldma_sg_len,
 			    difctx->dif_bundl_len, ldma_needed);
 
 			while (sglen) {
-- 
2.20.0

Re: [PATCH 3/4] scsi: qla2xxx: avoid printf format warning

[Himanshu Madhani]

On 3/4/19, 11:39 AM, "Arnd Bergmann" <arnd@arndb.de> wrote:

    Depending on the target architecture and configuration, both phys_addr_t
    and dma_addr_t may be smaller than 'long long', so we get a warning when
    printing either of them using the %llx format string:
    
    drivers/scsi/qla2xxx/qla_iocb.c: In function 'qla24xx_walk_and_build_prot_sglist':
    drivers/scsi/qla2xxx/qla_iocb.c:1140:46: error: format '%llx' expects argument of type 'long long unsigned int', but argument 6 has type 'dma_addr_t' {aka 'unsigned int'} [-Werror=format=]
             "%s: page boundary crossing (phys=%llx len=%x)\n",
                                               ~~~^
                                               %x
             __func__, sle_phys, sg->length);
                       ~~~~~~~~
    drivers/scsi/qla2xxx/qla_iocb.c:1180:29: error: format '%llx' expects argument of type 'long long unsigned int', but argument 7 has type 'dma_addr_t' {aka 'unsigned int'} [-Werror=format=]
            "%s: sg[%x] (phys=%llx sglen=%x) ldma_sg_len: %x dif_bundl_len: %x ldma_needed: %x\n",
                              ~~~^
    
    There are special %pad and %pap format strings in Linux that we could use here,
    but since the driver already does 64-bit arithmetic on the values, using a
    plain 'u64' seems more consistent here.
    
    Note: A possible related issue may be that the driver possibly checks the
    wrong kind of overflow: when an IOMMU is in use, buffers that cross a 32-bit
    boundary in physical addresses would still be mapped into dma addresses
    within the low 4GB space, so I suspect that we actually want to check
    sg_dma_address() instead of sg_phys() here.
    
    Fixes: 50b812755e97 ("scsi: qla2xxx: Fix DMA error when the DIF sg buffer crosses 4GB boundary")
    Signed-off-by: Arnd Bergmann <arnd@arndb.de>
    ---
     drivers/scsi/qla2xxx/qla_iocb.c | 4 ++--
     1 file changed, 2 insertions(+), 2 deletions(-)
    
    diff --git a/drivers/scsi/qla2xxx/qla_iocb.c b/drivers/scsi/qla2xxx/qla_iocb.c
    index 63f8e3c19841..456a41d2e2c6 100644
    --- a/drivers/scsi/qla2xxx/qla_iocb.c
    +++ b/drivers/scsi/qla2xxx/qla_iocb.c
    @@ -1132,7 +1132,7 @@ qla24xx_walk_and_build_prot_sglist(struct qla_hw_data *ha, srb_t *sp,
     	/* if initiator doing write or target doing read */
     	if (direction_to_device) {
     		for_each_sg(sgl, sg, tot_dsds, i) {
    -			dma_addr_t sle_phys = sg_phys(sg);
    +			u64 sle_phys = sg_phys(sg);
     
     			/* If SGE addr + len flips bits in upper 32-bits */
     			if (MSD(sle_phys + sg->length) ^ MSD(sle_phys)) {
    @@ -1178,7 +1178,7 @@ qla24xx_walk_and_build_prot_sglist(struct qla_hw_data *ha, srb_t *sp,
     
     			ql_dbg(ql_dbg_tgt + ql_dbg_verbose, vha, 0xe023,
     			    "%s: sg[%x] (phys=%llx sglen=%x) ldma_sg_len: %x dif_bundl_len: %x ldma_needed: %x\n",
    -			    __func__, i, sg_phys(sg), sglen, ldma_sg_len,
    +			    __func__, i, (u64)sg_phys(sg), sglen, ldma_sg_len,
     			    difctx->dif_bundl_len, ldma_needed);
     
     			while (sglen) {
    -- 
    2.20.0
    
Looks good

Acked-by: Himanshu Madhani <hmadhani@marvell.com>
    

Re: [PATCH 3/4] scsi: qla2xxx: avoid printf format warning

[Martin K. Petersen]

Arnd,

> Depending on the target architecture and configuration, both
> phys_addr_t and dma_addr_t may be smaller than 'long long', so we get
> a warning when printing either of them using the %llx format string:

Applied to 5.1/scsi-queue, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

[PATCH 4/4] scsi: ufs: hisi: fix ufs_hba_variant_ops passing

[Arnd Bergmann]

Without CONFIG_OF, the of_match_node() helper does not evaluate
its argument, and the compiler warns about the unused variable:

drivers/scsi/ufs/ufs-hisi.c: In function 'ufs_hisi_probe':
drivers/scsi/ufs/ufs-hisi.c:673:17: error: unused variable 'dev' [-Werror=unused-variable]

Rework this code to pass the data directly, and while we're
at it, correctly handle the const pointers.

Fixes: 653fcb07d95e ("scsi: ufs: Add HI3670 SoC UFS driver support")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/ufs/ufs-hisi.c      | 11 ++++-------
 drivers/scsi/ufs/ufshcd-pltfrm.c |  2 +-
 drivers/scsi/ufs/ufshcd-pltfrm.h |  2 +-
 drivers/scsi/ufs/ufshcd.h        |  2 +-
 4 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/ufs/ufs-hisi.c b/drivers/scsi/ufs/ufs-hisi.c
index f2d3df357a97..0e855b5afe82 100644
--- a/drivers/scsi/ufs/ufs-hisi.c
+++ b/drivers/scsi/ufs/ufs-hisi.c
@@ -640,7 +640,7 @@ static int ufs_hi3670_init(struct ufs_hba *hba)
 	return 0;
 }
 
-static struct ufs_hba_variant_ops ufs_hba_hi3660_vops = {
+static const struct ufs_hba_variant_ops ufs_hba_hi3660_vops = {
 	.name = "hi3660",
 	.init = ufs_hi3660_init,
 	.link_startup_notify = ufs_hisi_link_startup_notify,
@@ -649,7 +649,7 @@ static struct ufs_hba_variant_ops ufs_hba_hi3660_vops = {
 	.resume = ufs_hisi_resume,
 };
 
-static struct ufs_hba_variant_ops ufs_hba_hi3670_vops = {
+static const struct ufs_hba_variant_ops ufs_hba_hi3670_vops = {
 	.name = "hi3670",
 	.init = ufs_hi3670_init,
 	.link_startup_notify = ufs_hisi_link_startup_notify,
@@ -669,13 +669,10 @@ MODULE_DEVICE_TABLE(of, ufs_hisi_of_match);
 static int ufs_hisi_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *of_id;
-	struct ufs_hba_variant_ops *vops;
-	struct device *dev = &pdev->dev;
 
-	of_id = of_match_node(ufs_hisi_of_match, dev->of_node);
-	vops = (struct ufs_hba_variant_ops *)of_id->data;
+	of_id = of_match_node(ufs_hisi_of_match, pdev->dev.of_node);
 
-	return ufshcd_pltfrm_init(pdev, vops);
+	return ufshcd_pltfrm_init(pdev, of_id->data);
 }
 
 static int ufs_hisi_remove(struct platform_device *pdev)
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.c b/drivers/scsi/ufs/ufshcd-pltfrm.c
index 895a9b5ac989..27213676329c 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.c
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.c
@@ -297,7 +297,7 @@ static void ufshcd_init_lanes_per_dir(struct ufs_hba *hba)
  * Returns 0 on success, non-zero value on failure
  */
 int ufshcd_pltfrm_init(struct platform_device *pdev,
-		       struct ufs_hba_variant_ops *vops)
+		       const struct ufs_hba_variant_ops *vops)
 {
 	struct ufs_hba *hba;
 	void __iomem *mmio_base;
diff --git a/drivers/scsi/ufs/ufshcd-pltfrm.h b/drivers/scsi/ufs/ufshcd-pltfrm.h
index df64c4180340..1f29e1fd6d52 100644
--- a/drivers/scsi/ufs/ufshcd-pltfrm.h
+++ b/drivers/scsi/ufs/ufshcd-pltfrm.h
@@ -17,7 +17,7 @@
 #include "ufshcd.h"
 
 int ufshcd_pltfrm_init(struct platform_device *pdev,
-		       struct ufs_hba_variant_ops *vops);
+		       const struct ufs_hba_variant_ops *vops);
 void ufshcd_pltfrm_shutdown(struct platform_device *pdev);
 
 #ifdef CONFIG_PM
diff --git a/drivers/scsi/ufs/ufshcd.h b/drivers/scsi/ufs/ufshcd.h
index 69ba7445d2b3..ecfa898b9ccc 100644
--- a/drivers/scsi/ufs/ufshcd.h
+++ b/drivers/scsi/ufs/ufshcd.h
@@ -546,7 +546,7 @@ struct ufs_hba {
 	int nutrs;
 	int nutmrs;
 	u32 ufs_version;
-	struct ufs_hba_variant_ops *vops;
+	const struct ufs_hba_variant_ops *vops;
 	void *priv;
 	unsigned int irq;
 	bool is_irq_enabled;
-- 
2.20.0

RE: [PATCH 4/4] scsi: ufs: hisi: fix ufs_hba_variant_ops passing

[Avri Altman]

> 
> Without CONFIG_OF, the of_match_node() helper does not evaluate
> its argument, and the compiler warns about the unused variable:
> 
> drivers/scsi/ufs/ufs-hisi.c: In function 'ufs_hisi_probe':
> drivers/scsi/ufs/ufs-hisi.c:673:17: error: unused variable 'dev' [-
> Werror=unused-variable]
> 
> Rework this code to pass the data directly, and while we're
> at it, correctly handle the const pointers.
> 
> Fixes: 653fcb07d95e ("scsi: ufs: Add HI3670 SoC UFS driver support")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Avri Altman <avri.altman@wdc.com>

Arnd, Thank you for your comments.
Regards,
Avri

Re: [PATCH 4/4] scsi: ufs: hisi: fix ufs_hba_variant_ops passing

[Martin K. Petersen]

Arnd,

> Without CONFIG_OF, the of_match_node() helper does not evaluate
> its argument, and the compiler warns about the unused variable:
>
> drivers/scsi/ufs/ufs-hisi.c: In function 'ufs_hisi_probe':
> drivers/scsi/ufs/ufs-hisi.c:673:17: error: unused variable 'dev' [-Werror=unused-variable]

Applied to 5.1/scsi-queue, thank you!

-- 
Martin K. Petersen	Oracle Linux Engineering

Re: [PATCH 1/4] scsi: lpfc: fix unused variable warning

[Martin K. Petersen]

Arnd,

> The newly introduced 'cpu' variable is only used inside of an optional
> block, so we get a warning without CONFIG_SCSI_LPFC_DEBUG_FS:

Applied to 5.1/scsi-queue, thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering