All of lore.kernel.org
 help / color / mirror / Atom feed
From: Damien Le Moal <damien.lemoal@opensource.wdc.com>
To: linux-ide@vger.kernel.org
Cc: Paul Menzel <pmenzel@molgen.mpg.de>
Subject: [PATCH 3/4] ata: libata-sata: Remove debounce delay by default
Date: Wed, 23 Mar 2022 17:17:39 +0900	[thread overview]
Message-ID: <20220323081740.540006-4-damien.lemoal@opensource.wdc.com> (raw)
In-Reply-To: <20220323081740.540006-1-damien.lemoal@opensource.wdc.com>

Many adapters and SATA controllers are fine with accesses to the
SControl register right after resuming. For these adapters, the
additional 200ms delay implemented in sata_link_resume() is redundant
and unnecessarily increase boot time.

Drivers can opt out from this additional delay using the
ATA_LFLAG_NO_DEBOUNCE_DELAY link flag, but by default, this additional
delay exists for all adapters.

Reverse the situation and do not add by default this 200ms delay. For
adapters that actually need this delay, rename the
ATA_LFLAG_NO_DEBOUNCE_DELAY link flag to ATA_LFLAG_DEBOUNCE_DELAY and
execute the 200ms delay in sata_link_resume() only if a driver request
it using the new link flag. Otherwise, arbitrarily delay by 1ms only.

Since ata_piix adapters are known to require the longer delay, specify
the ATA_LFLAG_DEBOUNCE_DELAY for all adapters supported by the ata_piix
driver.

Signed-off-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
---
 drivers/ata/ahci.c        | 12 ++----------
 drivers/ata/ahci_brcm.c   |  1 -
 drivers/ata/ata_generic.c |  1 +
 drivers/ata/ata_piix.c    | 17 +++++++++++++++++
 drivers/ata/libata-sata.c | 18 +++++++++++-------
 include/linux/libata.h    |  2 +-
 6 files changed, 32 insertions(+), 19 deletions(-)

diff --git a/drivers/ata/ahci.c b/drivers/ata/ahci.c
index ccf94e8a3056..ac1e227a07cd 100644
--- a/drivers/ata/ahci.c
+++ b/drivers/ata/ahci.c
@@ -51,7 +51,6 @@ enum board_ids {
 	board_ahci,
 	board_ahci_ign_iferr,
 	board_ahci_low_power,
-	board_ahci_no_debounce_delay,
 	board_ahci_nomsi,
 	board_ahci_noncq,
 	board_ahci_nosntf,
@@ -142,13 +141,6 @@ static const struct ata_port_info ahci_port_info[] = {
 		.udma_mask	= ATA_UDMA6,
 		.port_ops	= &ahci_ops,
 	},
-	[board_ahci_no_debounce_delay] = {
-		.flags		= AHCI_FLAG_COMMON,
-		.link_flags	= ATA_LFLAG_NO_DEBOUNCE_DELAY,
-		.pio_mask	= ATA_PIO4,
-		.udma_mask	= ATA_UDMA6,
-		.port_ops	= &ahci_ops,
-	},
 	[board_ahci_nomsi] = {
 		AHCI_HFLAGS	(AHCI_HFLAG_NO_MSI),
 		.flags		= AHCI_FLAG_COMMON,
@@ -445,7 +437,7 @@ static const struct pci_device_id ahci_pci_tbl[] = {
 		board_ahci_al },
 	/* AMD */
 	{ PCI_VDEVICE(AMD, 0x7800), board_ahci }, /* AMD Hudson-2 */
-	{ PCI_VDEVICE(AMD, 0x7801), board_ahci_no_debounce_delay }, /* AMD Hudson-2 (AHCI mode) */
+	{ PCI_VDEVICE(AMD, 0x7801), board_ahci }, /* AMD Hudson-2 (AHCI mode) */
 	{ PCI_VDEVICE(AMD, 0x7900), board_ahci }, /* AMD CZ */
 	{ PCI_VDEVICE(AMD, 0x7901), board_ahci_low_power }, /* AMD Green Sardine */
 	/* AMD is using RAID class only for ahci controllers */
@@ -583,7 +575,7 @@ static const struct pci_device_id ahci_pci_tbl[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9230),
 	  .driver_data = board_ahci_yes_fbs },
 	{ PCI_DEVICE(PCI_VENDOR_ID_MARVELL_EXT, 0x9235),
-	  .driver_data = board_ahci_no_debounce_delay },
+	  .driver_data = board_ahci },
 	{ PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x0642), /* highpoint rocketraid 642L */
 	  .driver_data = board_ahci_yes_fbs },
 	{ PCI_DEVICE(PCI_VENDOR_ID_TTI, 0x0645), /* highpoint rocketraid 644L */
diff --git a/drivers/ata/ahci_brcm.c b/drivers/ata/ahci_brcm.c
index ab8552b1ff2a..e4584aed0ded 100644
--- a/drivers/ata/ahci_brcm.c
+++ b/drivers/ata/ahci_brcm.c
@@ -333,7 +333,6 @@ static struct ata_port_operations ahci_brcm_platform_ops = {
 
 static const struct ata_port_info ahci_brcm_port_info = {
 	.flags		= AHCI_FLAG_COMMON | ATA_FLAG_NO_DIPM,
-	.link_flags	= ATA_LFLAG_NO_DEBOUNCE_DELAY,
 	.pio_mask	= ATA_PIO4,
 	.udma_mask	= ATA_UDMA6,
 	.port_ops	= &ahci_brcm_platform_ops,
diff --git a/drivers/ata/ata_generic.c b/drivers/ata/ata_generic.c
index 20a32e4d501d..f02c824b26e6 100644
--- a/drivers/ata/ata_generic.c
+++ b/drivers/ata/ata_generic.c
@@ -165,6 +165,7 @@ static int ata_generic_init_one(struct pci_dev *dev, const struct pci_device_id
 	u16 command;
 	static const struct ata_port_info info = {
 		.flags = ATA_FLAG_SLAVE_POSS,
+		.link_flags = ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask = ATA_PIO4,
 		.mwdma_mask = ATA_MWDMA2,
 		.udma_mask = ATA_UDMA5,
diff --git a/drivers/ata/ata_piix.c b/drivers/ata/ata_piix.c
index ade5e894563b..dcfcfb5d8a05 100644
--- a/drivers/ata/ata_piix.c
+++ b/drivers/ata/ata_piix.c
@@ -1112,6 +1112,7 @@ static struct ata_port_info piix_port_info[] = {
 	[piix_pata_mwdma] =	/* PIIX3 MWDMA only */
 	{
 		.flags		= PIIX_PATA_FLAGS,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA12_ONLY, /* mwdma1-2 ?? CHECK 0 should be ok but slow */
 		.port_ops	= &piix_pata_ops,
@@ -1120,6 +1121,7 @@ static struct ata_port_info piix_port_info[] = {
 	[piix_pata_33] =	/* PIIX4 at 33MHz */
 	{
 		.flags		= PIIX_PATA_FLAGS,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA12_ONLY, /* mwdma1-2 ?? CHECK 0 should be ok but slow */
 		.udma_mask	= ATA_UDMA2,
@@ -1129,6 +1131,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich_pata_33] =		/* ICH0 - ICH at 33Mhz*/
 	{
 		.flags		= PIIX_PATA_FLAGS,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA12_ONLY, /* Check: maybe MWDMA0 is ok  */
 		.udma_mask	= ATA_UDMA2,
@@ -1138,6 +1141,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich_pata_66] =		/* ICH controllers up to 66MHz */
 	{
 		.flags		= PIIX_PATA_FLAGS,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA12_ONLY, /* MWDMA0 is broken on chip */
 		.udma_mask	= ATA_UDMA4,
@@ -1147,6 +1151,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich_pata_100] =
 	{
 		.flags		= PIIX_PATA_FLAGS | PIIX_FLAG_CHECKINTR,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA12_ONLY,
 		.udma_mask	= ATA_UDMA5,
@@ -1156,6 +1161,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich_pata_100_nomwdma1] =
 	{
 		.flags		= PIIX_PATA_FLAGS | PIIX_FLAG_CHECKINTR,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA2_ONLY,
 		.udma_mask	= ATA_UDMA5,
@@ -1165,6 +1171,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich5_sata] =
 	{
 		.flags		= PIIX_SATA_FLAGS,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA2,
 		.udma_mask	= ATA_UDMA6,
@@ -1174,6 +1181,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich6_sata] =
 	{
 		.flags		= PIIX_SATA_FLAGS,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA2,
 		.udma_mask	= ATA_UDMA6,
@@ -1183,6 +1191,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich6m_sata] =
 	{
 		.flags		= PIIX_SATA_FLAGS,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA2,
 		.udma_mask	= ATA_UDMA6,
@@ -1192,6 +1201,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich8_sata] =
 	{
 		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_SIDPR,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA2,
 		.udma_mask	= ATA_UDMA6,
@@ -1201,6 +1211,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich8_2port_sata] =
 	{
 		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_SIDPR,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA2,
 		.udma_mask	= ATA_UDMA6,
@@ -1210,6 +1221,7 @@ static struct ata_port_info piix_port_info[] = {
 	[tolapai_sata] =
 	{
 		.flags		= PIIX_SATA_FLAGS,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA2,
 		.udma_mask	= ATA_UDMA6,
@@ -1219,6 +1231,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich8m_apple_sata] =
 	{
 		.flags		= PIIX_SATA_FLAGS,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA2,
 		.udma_mask	= ATA_UDMA6,
@@ -1228,6 +1241,7 @@ static struct ata_port_info piix_port_info[] = {
 	[piix_pata_vmw] =
 	{
 		.flags		= PIIX_PATA_FLAGS,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA12_ONLY, /* mwdma1-2 ?? CHECK 0 should be ok but slow */
 		.udma_mask	= ATA_UDMA2,
@@ -1241,6 +1255,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich8_sata_snb] =
 	{
 		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_SIDPR | PIIX_FLAG_PIO16,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA2,
 		.udma_mask	= ATA_UDMA6,
@@ -1251,6 +1266,7 @@ static struct ata_port_info piix_port_info[] = {
 	{
 		.flags		= PIIX_SATA_FLAGS | PIIX_FLAG_SIDPR
 					| PIIX_FLAG_PIO16,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask	= ATA_PIO4,
 		.mwdma_mask	= ATA_MWDMA2,
 		.udma_mask	= ATA_UDMA6,
@@ -1260,6 +1276,7 @@ static struct ata_port_info piix_port_info[] = {
 	[ich8_2port_sata_byt] =
 	{
 		.flags          = PIIX_SATA_FLAGS | PIIX_FLAG_SIDPR | PIIX_FLAG_PIO16,
+		.link_flags	= ATA_LFLAG_DEBOUNCE_DELAY,
 		.pio_mask       = ATA_PIO4,
 		.mwdma_mask     = ATA_MWDMA2,
 		.udma_mask      = ATA_UDMA6,
diff --git a/drivers/ata/libata-sata.c b/drivers/ata/libata-sata.c
index be46833d77a6..87ad03c2e49f 100644
--- a/drivers/ata/libata-sata.c
+++ b/drivers/ata/libata-sata.c
@@ -302,9 +302,18 @@ static int __sata_link_resume(struct ata_link *link,
 			      unsigned long deadline)
 {
 	int tries = ATA_LINK_RESUME_TRIES;
+	unsigned int db_delay = 1;
 	u32 scontrol, serror;
 	int rc;
 
+	/*
+	 * Some PHYs react badly if SControl is pounded immediately after
+	 * resuming. For drivers requesting it, delay 200ms before debouncing.
+	 * Otherwise, only delay by 1ms (arbitrary delay).
+	 */
+	if (link->flags & ATA_LFLAG_DEBOUNCE_DELAY)
+		db_delay = 200;
+
 	if ((rc = sata_scr_read(link, SCR_CONTROL, &scontrol)))
 		return rc;
 
@@ -317,13 +326,8 @@ static int __sata_link_resume(struct ata_link *link,
 		scontrol = (scontrol & 0x0f0) | 0x300;
 		if ((rc = sata_scr_write(link, SCR_CONTROL, scontrol)))
 			return rc;
-		/*
-		 * Some PHYs react badly if SStatus is pounded
-		 * immediately after resuming.  Delay 200ms before
-		 * debouncing.
-		 */
-		if (!(link->flags & ATA_LFLAG_NO_DEBOUNCE_DELAY))
-			ata_msleep(link->ap, 200);
+
+		ata_msleep(link->ap, db_delay);
 
 		/* is SControl restored correctly? */
 		if ((rc = sata_scr_read(link, SCR_CONTROL, &scontrol)))
diff --git a/include/linux/libata.h b/include/linux/libata.h
index 166263d9bbc7..cc3a8e9c78b0 100644
--- a/include/linux/libata.h
+++ b/include/linux/libata.h
@@ -143,7 +143,7 @@ enum {
 	ATA_LFLAG_NO_LPM	= (1 << 8), /* disable LPM on this link */
 	ATA_LFLAG_RST_ONCE	= (1 << 9), /* limit recovery to one reset */
 	ATA_LFLAG_CHANGED	= (1 << 10), /* LPM state changed on this link */
-	ATA_LFLAG_NO_DEBOUNCE_DELAY = (1 << 11), /* no debounce delay on link resume */
+	ATA_LFLAG_DEBOUNCE_DELAY = (1 << 11), /* Debounce delay on link resume */
 
 	/* struct ata_port flags */
 	ATA_FLAG_SLAVE_POSS	= (1 << 0), /* host supports slave dev */
-- 
2.35.1


  parent reply	other threads:[~2022-03-23  8:19 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-23  8:17 [PATCH 0/4] Remove link debounce delays by default Damien Le Moal
2022-03-23  8:17 ` [PATCH 1/4] ata: libata-sata: Simplify sata_link_resume() interface Damien Le Moal
2022-03-23  8:17 ` [PATCH 2/4] ata: libata-sata: Introduce struct sata_deb_timing Damien Le Moal
2022-03-23 14:09   ` Paul Menzel
2022-03-23 23:51     ` Damien Le Moal
2022-03-23  8:17 ` Damien Le Moal [this message]
2022-03-23 14:25   ` [PATCH 3/4] ata: libata-sata: Remove debounce delay by default Paul Menzel
2022-03-23  8:17 ` [PATCH 4/4] ata: libata-sata: Improve sata_link_debounce() Damien Le Moal
2022-03-25  9:10   ` Paul Menzel
2022-03-28  7:19     ` Damien Le Moal
2022-03-28  7:41       ` Paul Menzel
2022-03-28  9:13         ` Damien Le Moal
2022-03-28  9:43         ` Damien Le Moal
2022-03-23  8:43 ` [PATCH 0/4] Remove link debounce delays by default Paul Menzel
2022-03-23  8:45   ` Paul Menzel
2022-03-23  9:34   ` Damien Le Moal
2022-03-23  9:39   ` Damien Le Moal
2022-03-23 10:59     ` Paul Menzel
2022-03-23 23:58       ` Damien Le Moal
2022-03-24  5:30         ` Paul Menzel
2022-03-24  6:15           ` Damien Le Moal
2022-03-24  6:35             ` Paul Menzel
2022-03-24  6:51               ` Damien Le Moal

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=20220323081740.540006-4-damien.lemoal@opensource.wdc.com \
    --to=damien.lemoal@opensource.wdc.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=pmenzel@molgen.mpg.de \
    /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.