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@xxxxxxxxxxxxxxxxxx> > > Regardless of a couple of comments: > Reviewed-by: John Garry <john.garry@xxxxxxxxxx> > >> --- >> 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) > -- Damien Le Moal Western Digital Research
