Re: [PATCH] ata: libata-eh: Fix link state check for IDE/PATA ports

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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





[Index of Archives]     [Linux Filesystems]     [Linux SCSI]     [Linux RAID]     [Git]     [Kernel Newbies]     [Linux Newbie]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Samba]     [Device Mapper]

  Powered by Linux