Re: [PATCH] ata: libata-sata: retry hardreset when device detected but PHY not established

[yangxingui]

On 2026/4/27 21:17, Niklas Cassel wrote:
> On Sat, Apr 25, 2026 at 02:04:47PM +0800, Xingui Yang wrote:
> > When sata_link_hardreset() detects that the link is offline, it currently
> > returns immediately without distinguishing the reason. According to SATA
> > specification, the SStatus register's det filed (bits 0-3) indicates:
> >    - 0x0: No device detected, PHY not communicating
> >    - 0x1: Device detected but PHY communication not established
> >    - 0x3: Device detected and PHY communication established
> >
> > This patch helps improve device detection reliability and adds a check
> > when the link is offline but det filed shows 0x1, return -EAGAIN to
> > trigger retry, rather than giving up immediately.
> >
> > Signed-off-by: Xingui Yang <yangxingui@xxxxxxxxxx>
> > ---
> >   drivers/ata/libata-sata.c | 12 +++++++++++-
> >   1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
> > index b9d635088f5f..e5bb92c38e38 100644
> > --- a/drivers/ata/libata-sata.c
> > +++ b/drivers/ata/libata-sata.c
> > @@ -667,8 +667,18 @@ int sata_link_hardreset(struct ata_link *link, const unsigned int *timing,
> >   	if (rc)
> >   		goto out;
> >   	/* if link is offline nothing more to do */
> > -	if (ata_phys_link_offline(link))
> > +	if (ata_phys_link_offline(link)) {
> > +		u32 sstatus;
> > +
> > +		if (sata_scr_read(link, SCR_STATUS, &sstatus) == 0 &&
> > +		    (sstatus & 0xf) == 0x1) {
> > +			ata_link_warn(link, "device detected but PHY not ready (SStatus %X), retrying\n",
> > +				      sstatus);
> > +			rc = -EAGAIN;
> > +		}
> > +
>
> This looks like you are more or less duplicating the function
> ata_eh_link_established(), untrouced in commit 4371fe1ba400 ("ata:
> libata-eh: Avoid unnecessary resets when revalidating devices").
>
> Could you perhaps try to reuse this function?
>
> (It is currently private, so you would need to make it public.)

This looks like a pretty good suggestion, but according to the log 
print, when an exception occurs, the ipm field is 0, indicating that the 
communication has not been established. It might not be suitable to use 
this interface yet.

[   25.242373][ T1286] ata1: SATA link down (SStatus 1 SControl 300)

Thanks,
Xingui