Re: [PATCH 05/20] scsi: pm8001: Remove local variable in pm8001_pci_resume()

[John Garry <john.garry@huawei.com>]

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)