[PATCH 3/4] ata: libata-sata: Remove debounce delay by default

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

 



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@xxxxxxxxxxxxxxxxxx>
---
 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




[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