linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] scsi: qla1280: Fix a use of QLA_64BIT_PTR
@ 2020-01-20 19:00 Nathan Chancellor
  2020-01-21 18:43 ` Nick Desaulniers
  0 siblings, 1 reply; 5+ messages in thread
From: Nathan Chancellor @ 2020-01-20 19:00 UTC (permalink / raw)
  To: Michael Reed, James E.J. Bottomley, Martin K. Petersen
  Cc: Thomas Bogendoerfer, linux-scsi, linux-kernel, clang-built-linux,
	Nathan Chancellor

Clang warns:

../drivers/scsi/qla1280.c:1702:5: warning: 'QLA_64BIT_PTR' is not
defined, evaluates to 0 [-Wundef]
if QLA_64BIT_PTR
    ^
1 warning generated.

The rest of this driver uses #ifdef QLA_64BIT_PTR, do the same thing at
this site to remove this warning.

Fixes: ba304e5b4498 ("scsi: qla1280: Fix dma firmware download, if dma address is 64bit")
Link: https://github.com/ClangBuiltLinux/linux/issues/843
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---
 drivers/scsi/qla1280.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c
index 607cbddcdd14..3337cd341d21 100644
--- a/drivers/scsi/qla1280.c
+++ b/drivers/scsi/qla1280.c
@@ -1699,7 +1699,7 @@ qla1280_load_firmware_pio(struct scsi_qla_host *ha)
 	return err;
 }
 
-#if QLA_64BIT_PTR
+#ifdef QLA_64BIT_PTR
 #define LOAD_CMD	MBC_LOAD_RAM_A64_ROM
 #define DUMP_CMD	MBC_DUMP_RAM_A64_ROM
 #define CMD_ARGS	(BIT_7 | BIT_6 | BIT_4 | BIT_3 | BIT_2 | BIT_1 | BIT_0)
-- 
2.25.0


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] scsi: qla1280: Fix a use of QLA_64BIT_PTR
  2020-01-20 19:00 [PATCH] scsi: qla1280: Fix a use of QLA_64BIT_PTR Nathan Chancellor
@ 2020-01-21 18:43 ` Nick Desaulniers
  2020-01-21 18:58   ` Nathan Chancellor
  2020-01-21 19:07   ` Martin K. Petersen
  0 siblings, 2 replies; 5+ messages in thread
From: Nick Desaulniers @ 2020-01-21 18:43 UTC (permalink / raw)
  To: Nathan Chancellor, Thomas Bogendoerfer
  Cc: Michael Reed, James E.J. Bottomley, Martin K. Petersen,
	linux-scsi, LKML, clang-built-linux

On Mon, Jan 20, 2020 at 11:00 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> Clang warns:
>
> ../drivers/scsi/qla1280.c:1702:5: warning: 'QLA_64BIT_PTR' is not
> defined, evaluates to 0 [-Wundef]
> if QLA_64BIT_PTR
>     ^
> 1 warning generated.
>
> The rest of this driver uses #ifdef QLA_64BIT_PTR, do the same thing at
> this site to remove this warning.
>
> Fixes: ba304e5b4498 ("scsi: qla1280: Fix dma firmware download, if dma address is 64bit")

^ The above SHA is valid only in linux-next. Won't it change when
merged into mainline?

> Link: https://github.com/ClangBuiltLinux/linux/issues/843
> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>

Thanks for the patch.
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>  drivers/scsi/qla1280.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c
> index 607cbddcdd14..3337cd341d21 100644
> --- a/drivers/scsi/qla1280.c
> +++ b/drivers/scsi/qla1280.c
> @@ -1699,7 +1699,7 @@ qla1280_load_firmware_pio(struct scsi_qla_host *ha)
>         return err;
>  }
>
> -#if QLA_64BIT_PTR
> +#ifdef QLA_64BIT_PTR

Thomas should test this, as it implies the previous patch was NEVER
using the "true case" values, making it in effect a
no-functional-change (NFC).

>  #define LOAD_CMD       MBC_LOAD_RAM_A64_ROM
>  #define DUMP_CMD       MBC_DUMP_RAM_A64_ROM
>  #define CMD_ARGS       (BIT_7 | BIT_6 | BIT_4 | BIT_3 | BIT_2 | BIT_1 | BIT_0)
> --
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] scsi: qla1280: Fix a use of QLA_64BIT_PTR
  2020-01-21 18:43 ` Nick Desaulniers
@ 2020-01-21 18:58   ` Nathan Chancellor
  2020-01-21 19:02     ` Nick Desaulniers
  2020-01-21 19:07   ` Martin K. Petersen
  1 sibling, 1 reply; 5+ messages in thread
From: Nathan Chancellor @ 2020-01-21 18:58 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Thomas Bogendoerfer, Michael Reed, James E.J. Bottomley,
	Martin K. Petersen, linux-scsi, LKML, clang-built-linux

On Tue, Jan 21, 2020 at 10:43:06AM -0800, Nick Desaulniers wrote:
> On Mon, Jan 20, 2020 at 11:00 AM Nathan Chancellor
> <natechancellor@gmail.com> wrote:
> >
> > Clang warns:
> >
> > ../drivers/scsi/qla1280.c:1702:5: warning: 'QLA_64BIT_PTR' is not
> > defined, evaluates to 0 [-Wundef]
> > if QLA_64BIT_PTR
> >     ^
> > 1 warning generated.
> >
> > The rest of this driver uses #ifdef QLA_64BIT_PTR, do the same thing at
> > this site to remove this warning.
> >
> > Fixes: ba304e5b4498 ("scsi: qla1280: Fix dma firmware download, if dma address is 64bit")
> 
> ^ The above SHA is valid only in linux-next. Won't it change when
> merged into mainline?

Not unless Martin rebases his tree (in which case, this patch should
just be folded into the original one).

> > Link: https://github.com/ClangBuiltLinux/linux/issues/843
> > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
> 
> Thanks for the patch.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

Thanks for the review :)

> > ---
> >  drivers/scsi/qla1280.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/scsi/qla1280.c b/drivers/scsi/qla1280.c
> > index 607cbddcdd14..3337cd341d21 100644
> > --- a/drivers/scsi/qla1280.c
> > +++ b/drivers/scsi/qla1280.c
> > @@ -1699,7 +1699,7 @@ qla1280_load_firmware_pio(struct scsi_qla_host *ha)
> >         return err;
> >  }
> >
> > -#if QLA_64BIT_PTR
> > +#ifdef QLA_64BIT_PTR
> 
> Thomas should test this, as it implies the previous patch was NEVER
> using the "true case" values, making it in effect a
> no-functional-change (NFC).

QLA_64BIT_PTR is defined to 1 when CONFIG_ARCH_DMA_ADDR_T_64BIT is set
so the true should have always worked, unless I am misunderstanding what
you are saying. The false case should have also worked because it is
still evaluated to 0 but it throws the warning to make sure that was
intended (again, as I understand it).

> >  #define LOAD_CMD       MBC_LOAD_RAM_A64_ROM
> >  #define DUMP_CMD       MBC_DUMP_RAM_A64_ROM
> >  #define CMD_ARGS       (BIT_7 | BIT_6 | BIT_4 | BIT_3 | BIT_2 | BIT_1 | BIT_0)
> > --
> -- 
> Thanks,
> ~Nick Desaulniers

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] scsi: qla1280: Fix a use of QLA_64BIT_PTR
  2020-01-21 18:58   ` Nathan Chancellor
@ 2020-01-21 19:02     ` Nick Desaulniers
  0 siblings, 0 replies; 5+ messages in thread
From: Nick Desaulniers @ 2020-01-21 19:02 UTC (permalink / raw)
  To: Nathan Chancellor
  Cc: Thomas Bogendoerfer, Michael Reed, James E.J. Bottomley,
	Martin K. Petersen, linux-scsi, LKML, clang-built-linux

On Tue, Jan 21, 2020 at 10:58 AM Nathan Chancellor
<natechancellor@gmail.com> wrote:
>
> On Tue, Jan 21, 2020 at 10:43:06AM -0800, Nick Desaulniers wrote:
> > On Mon, Jan 20, 2020 at 11:00 AM Nathan Chancellor
> > <natechancellor@gmail.com> wrote:
> > > -#if QLA_64BIT_PTR
> > > +#ifdef QLA_64BIT_PTR
> >
> > Thomas should test this, as it implies the previous patch was NEVER
> > using the "true case" values, making it in effect a
> > no-functional-change (NFC).
>
> QLA_64BIT_PTR is defined to 1 when CONFIG_ARCH_DMA_ADDR_T_64BIT is set
> so the true should have always worked, unless I am misunderstanding what
> you are saying. The false case should have also worked because it is
> still evaluated to 0 but it throws the warning to make sure that was
> intended (again, as I understand it).
>
> > >  #define LOAD_CMD       MBC_LOAD_RAM_A64_ROM
> > >  #define DUMP_CMD       MBC_DUMP_RAM_A64_ROM
> > >  #define CMD_ARGS       (BIT_7 | BIT_6 | BIT_4 | BIT_3 | BIT_2 | BIT_1 | BIT_0)

Ah, right, so either QLA_64BIT_PTR is defined with a value of 1, or
not defined at all.  My bad.
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] scsi: qla1280: Fix a use of QLA_64BIT_PTR
  2020-01-21 18:43 ` Nick Desaulniers
  2020-01-21 18:58   ` Nathan Chancellor
@ 2020-01-21 19:07   ` Martin K. Petersen
  1 sibling, 0 replies; 5+ messages in thread
From: Martin K. Petersen @ 2020-01-21 19:07 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: Nathan Chancellor, Thomas Bogendoerfer, Michael Reed,
	James E.J. Bottomley, Martin K. Petersen, linux-scsi, LKML,
	clang-built-linux


>> ../drivers/scsi/qla1280.c:1702:5: warning: 'QLA_64BIT_PTR' is not
>> defined, evaluates to 0 [-Wundef]
>> if QLA_64BIT_PTR
>>     ^
>> 1 warning generated.

I already merged Thomas' patch for this issue.

-- 
Martin K. Petersen	Oracle Linux Engineering

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2020-01-21 19:07 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-20 19:00 [PATCH] scsi: qla1280: Fix a use of QLA_64BIT_PTR Nathan Chancellor
2020-01-21 18:43 ` Nick Desaulniers
2020-01-21 18:58   ` Nathan Chancellor
2020-01-21 19:02     ` Nick Desaulniers
2020-01-21 19:07   ` Martin K. Petersen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).