Re: [PATCH 2/2] scsi: hisi_sas: Modify the deadline for ata_wait_after_reset()

[Damien Le Moal <dlemoal@kernel.org>]

On 4/1/24 14:49, chenxiang wrote:
> From: Yihang Li <liyihang9@huawei.com>
> 
> We found that the second parameter of function ata_wait_after_reset() is
> incorrectly used. We call smp_ata_check_ready_type() to poll the device
> type until the 30s timeout, so the correct deadline should be (jiffies +
> 30000).
> 
> Fixes: 3c2673a09cf1 ("scsi: hisi_sas: Fix SATA devices missing issue during I_T nexus reset")
> Signed-off-by: xiabing <xiabing12@h-partners.com>
> Signed-off-by: Yihang Li <liyihang9@huawei.com>
> Signed-off-by: Xiang Chen <chenxiang66@hisilicon.com>
> ---
>  drivers/scsi/hisi_sas/hisi_sas_main.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/scsi/hisi_sas/hisi_sas_main.c b/drivers/scsi/hisi_sas/hisi_sas_main.c
> index 097dfe4b620d..7245600aedb2 100644
> --- a/drivers/scsi/hisi_sas/hisi_sas_main.c
> +++ b/drivers/scsi/hisi_sas/hisi_sas_main.c
> @@ -1796,8 +1796,10 @@ static int hisi_sas_debug_I_T_nexus_reset(struct domain_device *device)
>  
>  	if (dev_is_sata(device)) {
>  		struct ata_link *link = &device->sata_dev.ap->link;
> +		unsigned long deadline = ata_deadline(jiffies,
> +				HISI_SAS_WAIT_PHYUP_TIMEOUT / HZ * 1000);

At the very least, you should use jiffies_to_msecs() here. But I do not see the
point though given that ata_deadline will do "jiffies + msecs_to_jiffies()".

So may be calling:

	rc = ata_wait_after_reset(link, jiffies + HISI_SAS_WAIT_PHYUP_TIMEOUT,
				  smp_ata_check_ready_type);

May be simpler and more readable.

>  
> -		rc = ata_wait_after_reset(link, HISI_SAS_WAIT_PHYUP_TIMEOUT,
> +		rc = ata_wait_after_reset(link, deadline,
>  					  smp_ata_check_ready_type);
>  	} else {
>  		msleep(2000);

-- 
Damien Le Moal
Western Digital Research