On 8/13/25 12:27 PM, Damien Le Moal wrote:
> Commit 4371fe1ba400 ("ata: libata-eh: Avoid unnecessary resets when
> revalidating devices") replaced the call to ata_phys_link_offline() in
> ata_eh_revalidate_and_attach() with the new function
> ata_eh_link_established() which relaxes the checks on a device link
> state to account for low power mode transitions. However, this changed
> assumed that the device port has a valid scr_read method to obstain the
> SSTATUS register for the port. This is not always the case, especially
Hum, another nit... Don't the SATA specs call this register SStatus?
> with older IDE/PATA adapters (e.g. as emulated with qemu). For such
> adapter, ata_eh_link_established() will always return false, causing
> ata_eh_revalidate_and_attach() to go into its error path and ultimately
> to the device being disabled.
>
> Avoid this by restoring the previous behavior, which is to assume that
> the link is online if reading the port SSTATUS register fails.
>
> Reported-by: Shin'ichiro Kawasaki <shinichiro.kawasaki@xxxxxxx>
> Fixes: 4371fe1ba400 ("ata: libata-eh: Avoid unnecessary resets when revalidating devices")
> Signed-off-by: Damien Le Moal <dlemoal@xxxxxxxxxx>
> ---
> drivers/ata/libata-eh.c | 7 ++++++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
> index 2946ae6d4b2c..354d2c0abcf3 100644
> --- a/drivers/ata/libata-eh.c
> +++ b/drivers/ata/libata-eh.c
> @@ -2089,8 +2089,13 @@ static bool ata_eh_link_established(struct ata_link *link)
> u32 sstatus;
> u8 det, ipm;
>
> + /*
> + * For old IDE/PATA adapters that do not have a valid scr_read method,
> + * or if reading the SSTATUS register fails, assume that the device is
Same comment here...
> + * present. Device probe will determine if that is really the case.
> + */
> if (sata_scr_read(link, SCR_STATUS, &sstatus))
> - return false;
> + return true;
>
> det = sstatus & 0x0f;
> ipm = (sstatus >> 8) & 0x0f;
MBR, Sergey