Re: [PATCH 04/10] ata: libata-eh: Avoid unnecessary resets when revalidating devices

[Niklas Cassel <cassel@kernel.org>]

On Mon, Jun 30, 2025 at 03:26:31PM +0900, Damien Le Moal wrote:
> In ata_eh_revalidate_and_attach(), a link LPM policy is always
> set to ATA_LPM_MAX_POWER before calling ata_dev_revalidate() to ensure
> that the call to ata_phys_link_offline() does not return true, thus
> causing an unnecessary device reset. This change was introduced
> with commit 71d7b6e51ad3 ("ata: libata-eh: avoid needless hard reset
> when revalidating link").
> 
> However, setting the link LPM policy to ATA_LPM_MAX_POWER may be
> visible only after some time, depending on the power state the link was
> in. E.g. transitioning out of the Partial state should take no longer
> than a few microseconds, but transitioning out of the Slumber or
> DevSleep state may take several milliseconds. So despite the changes
> introduced with commit 71d7b6e51ad3 ("ata: libata-eh: avoid needless
> hard reset when revalidating link"), we can still endup with
> ata_phys_link_offline() seeing a link SCR_STATUS register signaling that
> the device is present (DET is equal to 1h) but that the link PHY is
> still in a low power mode (e.g. IPM is 2h, signaling "Interface in
> Partial power management state"). In such cases, ata_phys_link_offline()
> returns true, causing an EIO return for ata_eh_revalidate_and_attach()
> and a device reset.
> 
> Avoid such unnecessary device resets by introducing a relaxed version
> of the link offline test implemented by ata_phys_link_offline() with
> the new helper function ata_eh_link_established(). This functions
> returns true if for the link SCR_STATUS register we see that:
>  - A device is still present, that is, the DET field is 1h (Device
>    presence detected but Phy communication not established) or 3h
>    (Device presence detected and Phy communication established).
>  - Communication is established, that is, the IPM field is not 0h,
>    indicating that the PHY is online or in a low power state.
> 
> Signed-off-by: Damien Le Moal <dlemoal@kernel.org>

Reviewed-by: Niklas Cassel <cassel@kernel.org>