From: Damien Le Moal <dlemoal@kernel.org>
To: linux-ide@vger.kernel.org, Niklas Cassel <cassel@kernel.org>
Subject: [PATCH 04/10] ata: libata-eh: Avoid unnecessary resets when revalidating devices
Date: Mon, 30 Jun 2025 15:26:31 +0900 [thread overview]
Message-ID: <20250630062637.258329-5-dlemoal@kernel.org> (raw)
In-Reply-To: <20250630062637.258329-1-dlemoal@kernel.org>
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>
---
drivers/ata/libata-eh.c | 29 ++++++++++++++++++++++++++++-
1 file changed, 28 insertions(+), 1 deletion(-)
diff --git a/drivers/ata/libata-eh.c b/drivers/ata/libata-eh.c
index f98d5123e1e4..7f5d13f9ca73 100644
--- a/drivers/ata/libata-eh.c
+++ b/drivers/ata/libata-eh.c
@@ -2071,6 +2071,33 @@ static void ata_eh_get_success_sense(struct ata_link *link)
ata_eh_done(link, dev, ATA_EH_GET_SUCCESS_SENSE);
}
+/*
+ * Check if a link is established. This is a relaxed version of
+ * ata_phys_link_online() which accounts for the fact that this is potentially
+ * called after changing the link power management policy, which may not be
+ * reflected immediately in the SSTAUS register (e.g., we may still be seeing
+ * the PHY in partial, slumber or devsleep Partial power management state.
+ * So check that:
+ * - A device is still present, that is, DET is 1h (Device presence detected
+ * but Phy communication not established) or 3h (Device presence detected and
+ * Phy communication established)
+ * - Communication is established, that is, IPM is not 0h, indicating that PHY
+ * is online or in a low power state.
+ */
+static bool ata_eh_link_established(struct ata_link *link)
+{
+ u32 sstatus;
+ u8 det, ipm;
+
+ if (sata_scr_read(link, SCR_STATUS, &sstatus))
+ return false;
+
+ det = sstatus & 0x0f;
+ ipm = (sstatus >> 8) & 0x0f;
+
+ return (det & 0x01) && ipm;
+}
+
/**
* ata_eh_link_set_lpm - configure SATA interface power management
* @link: link to configure
@@ -3275,7 +3302,7 @@ static int ata_eh_revalidate_and_attach(struct ata_link *link,
goto err;
}
- if (ata_phys_link_offline(ata_dev_phys_link(dev))) {
+ if (!ata_eh_link_established(ata_dev_phys_link(dev))) {
rc = -EIO;
goto err;
}
--
2.50.0
next prev parent reply other threads:[~2025-06-30 6:28 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-06-30 6:26 [PATCH 00/10] Improve link power management Damien Le Moal
2025-06-30 6:26 ` [PATCH 01/10] ata: libata-core: Introduce ata_dev_config_lpm() Damien Le Moal
2025-06-30 14:46 ` Niklas Cassel
2025-07-01 6:09 ` Hannes Reinecke
2025-06-30 6:26 ` [PATCH 02/10] ata: libata-core: Move device LPM quirk settings to ata_dev_config_lpm() Damien Le Moal
2025-06-30 14:47 ` Niklas Cassel
2025-07-01 6:13 ` Hannes Reinecke
2025-07-01 6:43 ` Damien Le Moal
2025-07-01 7:19 ` Hannes Reinecke
2025-06-30 6:26 ` [PATCH 03/10] ata: libata-core: Advertize device support for DIPM and HIPM features Damien Le Moal
2025-06-30 14:47 ` Niklas Cassel
2025-07-01 6:14 ` Hannes Reinecke
2025-06-30 6:26 ` Damien Le Moal [this message]
2025-06-30 14:47 ` [PATCH 04/10] ata: libata-eh: Avoid unnecessary resets when revalidating devices Niklas Cassel
2025-07-01 6:23 ` Hannes Reinecke
2025-07-01 6:48 ` Damien Le Moal
2025-07-01 7:21 ` Hannes Reinecke
2025-07-01 9:24 ` Niklas Cassel
2025-07-01 9:25 ` Damien Le Moal
2025-06-30 6:26 ` [PATCH 05/10] ata: libata-sata: Disallow changing LPM state if not supported Damien Le Moal
2025-06-30 14:49 ` Niklas Cassel
2025-07-01 6:23 ` Hannes Reinecke
2025-06-30 6:26 ` [PATCH 06/10] ata: ahci: Disable DIPM if host lacks support Damien Le Moal
2025-06-30 14:50 ` Niklas Cassel
2025-07-01 6:23 ` Hannes Reinecke
2025-06-30 6:26 ` [PATCH 07/10] ata: ahci: Disallow LPM policy control for external ports Damien Le Moal
2025-06-30 14:50 ` Niklas Cassel
2025-07-01 6:24 ` Hannes Reinecke
2025-06-30 6:26 ` [PATCH 08/10] ata: ahci: Disallow LPM policy control if not supported Damien Le Moal
2025-06-30 8:30 ` Sergey Shtylyov
2025-06-30 15:07 ` Niklas Cassel
2025-07-01 6:25 ` Hannes Reinecke
2025-06-30 6:26 ` [PATCH 09/10] ata: libata-core: Reduce the number of messages signaling broken LPM Damien Le Moal
2025-06-30 15:08 ` Niklas Cassel
2025-07-01 6:26 ` Hannes Reinecke
2025-06-30 6:26 ` [PATCH 10/10] ata: libata_eh: Add debug messages to ata_eh_link_set_lpm() Damien Le Moal
2025-06-30 15:11 ` Niklas Cassel
2025-06-30 15:25 ` Sergey Shtylyov
2025-07-01 6:27 ` Hannes Reinecke
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250630062637.258329-5-dlemoal@kernel.org \
--to=dlemoal@kernel.org \
--cc=cassel@kernel.org \
--cc=linux-ide@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.