| Message ID | 20220210114218.632725-6-damien.lemoal@opensource.wdc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | libsas and pm8001 fixes | expand |
On 10/02/2022 11:42, Damien Le Moal wrote: > In pm8001_pci_resume(), the use of the u32 type for the local variable > device_state causes a sparse warning: > > warning: incorrect type in assignment (different base types) > expected unsigned int [usertype] device_state > got restricted pci_power_t [usertype] current_state > > Since this variable is used only once in the function, remove it and > use pdev->current_state directly. While at it, also add a blank line > after the last local variable declaration. > > Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> Regardless of a couple of comments: Reviewed-by: John Garry <john.garry@huawei.com> > --- > drivers/scsi/pm8001/pm8001_init.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c > index d8a2121cb8d9..4b9a26f008a9 100644 > --- a/drivers/scsi/pm8001/pm8001_init.c > +++ b/drivers/scsi/pm8001/pm8001_init.c > @@ -1335,13 +1335,13 @@ static int __maybe_unused pm8001_pci_resume(struct device *dev) > struct pm8001_hba_info *pm8001_ha; > int rc; > u8 i = 0, j; > - u32 device_state; > DECLARE_COMPLETION_ONSTACK(completion); > + > pm8001_ha = sha->lldd_ha; > - device_state = pdev->current_state; > > - pm8001_info(pm8001_ha, "pdev=0x%p, slot=%s, resuming from previous operating state [D%d]\n", > - pdev, pm8001_ha->name, device_state); > + pm8001_info(pm8001_ha, > + "pdev=0x%p, slot=%s, resuming from previous operating state [D%d]\n", I think that we may put this on the same line as pm8001_info Feel free to ignore this: if we're ok with changing logs, I am not sure on the "slot" value - it is already printed with pm8001_info. And printing pdev is suspect, since we should really be using dev_info or pci_info() and friends - but that is a bigger job. Thanks, John > + pdev, pm8001_ha->name, pdev->current_state); > > rc = pci_go_44(pdev); > if (rc)
On 2/10/22 21:04, John Garry wrote: > On 10/02/2022 11:42, Damien Le Moal wrote: >> In pm8001_pci_resume(), the use of the u32 type for the local variable >> device_state causes a sparse warning: >> >> warning: incorrect type in assignment (different base types) >> expected unsigned int [usertype] device_state >> got restricted pci_power_t [usertype] current_state >> >> Since this variable is used only once in the function, remove it and >> use pdev->current_state directly. While at it, also add a blank line >> after the last local variable declaration. >> >> Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> > > Regardless of a couple of comments: > Reviewed-by: John Garry <john.garry@huawei.com> > >> --- >> drivers/scsi/pm8001/pm8001_init.c | 8 ++++---- >> 1 file changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c >> index d8a2121cb8d9..4b9a26f008a9 100644 >> --- a/drivers/scsi/pm8001/pm8001_init.c >> +++ b/drivers/scsi/pm8001/pm8001_init.c >> @@ -1335,13 +1335,13 @@ static int __maybe_unused pm8001_pci_resume(struct device *dev) >> struct pm8001_hba_info *pm8001_ha; >> int rc; >> u8 i = 0, j; >> - u32 device_state; >> DECLARE_COMPLETION_ONSTACK(completion); >> + >> pm8001_ha = sha->lldd_ha; >> - device_state = pdev->current_state; >> >> - pm8001_info(pm8001_ha, "pdev=0x%p, slot=%s, resuming from previous operating state [D%d]\n", >> - pdev, pm8001_ha->name, device_state); >> + pm8001_info(pm8001_ha, >> + "pdev=0x%p, slot=%s, resuming from previous operating state [D%d]\n", > > I think that we may put this on the same line as pm8001_info > > Feel free to ignore this: if we're ok with changing logs, I am not sure > on the "slot" value - it is already printed with pm8001_info. And > printing pdev is suspect, since we should really be using dev_info or > pci_info() and friends - but that is a bigger job. Yeah... This driver debug messages are a nightmare: hard to read format and not-so-useful information. Not to mention that the default log level is way too verbose... We should revisit the messages once we have flushed out all know bugs :) Thanks for the review. > > Thanks, > John > >> + pdev, pm8001_ha->name, pdev->current_state); >> >> rc = pci_go_44(pdev); >> if (rc) >
diff --git a/drivers/scsi/pm8001/pm8001_init.c b/drivers/scsi/pm8001/pm8001_init.c index d8a2121cb8d9..4b9a26f008a9 100644 --- a/drivers/scsi/pm8001/pm8001_init.c +++ b/drivers/scsi/pm8001/pm8001_init.c @@ -1335,13 +1335,13 @@ static int __maybe_unused pm8001_pci_resume(struct device *dev) struct pm8001_hba_info *pm8001_ha; int rc; u8 i = 0, j; - u32 device_state; DECLARE_COMPLETION_ONSTACK(completion); + pm8001_ha = sha->lldd_ha; - device_state = pdev->current_state; - pm8001_info(pm8001_ha, "pdev=0x%p, slot=%s, resuming from previous operating state [D%d]\n", - pdev, pm8001_ha->name, device_state); + pm8001_info(pm8001_ha, + "pdev=0x%p, slot=%s, resuming from previous operating state [D%d]\n", + pdev, pm8001_ha->name, pdev->current_state); rc = pci_go_44(pdev); if (rc)
In pm8001_pci_resume(), the use of the u32 type for the local variable device_state causes a sparse warning: warning: incorrect type in assignment (different base types) expected unsigned int [usertype] device_state got restricted pci_power_t [usertype] current_state Since this variable is used only once in the function, remove it and use pdev->current_state directly. While at it, also add a blank line after the last local variable declaration. Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com> --- drivers/scsi/pm8001/pm8001_init.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)