| Message ID | 20200120190021.26460-1-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | scsi: qla1280: Fix a use of QLA_64BIT_PTR | expand |
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) > --
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
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.
>> ../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.
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)
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(-)