libata: improve ata_bus_probe failure handling

PATCH New Tejun Heo 3 participants 37 messages Mar 31, 2006 – Apr 2, 2006

Subsystems (2)

SCSI libata

lore

Share & tools

lore

Tejun Heo

author

2006-03-31 16:38 UTC #1 lore raw

PATCH 1/15 libata: fix ata_set_mode() return value

In reply to Tejun Heo

Make ata_set_mode() return correct error value when ata_dev_set_mode() fails.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

f88bb95c5b31979a787a4150953820c8e77d7f29

drivers/scsi/libata-core.c +2 -1

index e63c1ff..8def7a5 100644
@@ -1856,7 +1856,8 @@ static void ata_set_mode(struct ata_port
 		if (!ata_dev_present(dev))
 			continue;
 
-		if (ata_dev_set_mode(ap, dev))
+		rc = ata_dev_set_mode(ap, dev);
+		if (rc)
 			goto err_out;
 	}

Signature · 1 line

1.2.4

Tejun Heo

author

2006-03-31 16:38 UTC #4 lore raw

PATCH 3/15 libata: separate out ata_spd_string()

In reply to Tejun Heo

Separate out ata_spd_string() from sata_print_link_status(). This will be used by SATA spd configuration routines.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |   23 ++++++++++++++---------
 1 files changed, 14 insertions(+), 9 deletions(-)

f10d6a608edacca5ef8c751bb3c48d2cb47f9166

drivers/scsi/libata-core.c +14 -9

index c7ba523..6a336ba 100644
@@ -397,6 +397,18 @@ static const char *ata_mode_string(unsig
 	return "<n/a>";
 }
 
+static const char *sata_spd_string(unsigned int spd)
+{
+	static const char * const spd_str[] = {
+		"1.5 Gbps",
+		"3.0 Gbps",
+	};
+
+	if (spd == 0 || (spd - 1) >= ARRAY_SIZE(spd_str))
+		return "<unknown>";
+	return spd_str[spd - 1];
+}
+
 static void ata_dev_disable(struct ata_port *ap, struct ata_device *dev)
 {
 	if (ata_dev_present(dev)) {

@@ -1452,7 +1464,6 @@ void ata_port_probe(struct ata_port *ap)
 static void sata_print_link_status(struct ata_port *ap)
 {
 	u32 sstatus, tmp;
-	const char *speed;
 
 	if (!ap->ops->scr_read)
 		return;

@@ -1461,14 +1472,8 @@ static void sata_print_link_status(struc
 
 	if (sata_dev_present(ap)) {
 		tmp = (sstatus >> 4) & 0xf;
-		if (tmp & (1 << 0))
-			speed = "1.5";
-		else if (tmp & (1 << 1))
-			speed = "3.0";
-		else
-			speed = "<unknown>";
-		printk(KERN_INFO "ata%u: SATA link up %s Gbps (SStatus %X)\n",
-		       ap->id, speed, sstatus);
+		printk(KERN_INFO "ata%u: SATA link up %s (SStatus %X)\n",
+		       ap->id, sata_spd_string(tmp), sstatus);
 	} else {
 		printk(KERN_INFO "ata%u: SATA link down (SStatus %X)\n",
 		       ap->id, sstatus);

Signature · 1 line

1.2.4

Tejun Heo

author

2006-03-31 16:38 UTC #0 lore raw

PATCH 1/1 libata: improve ata_bus_probe failure handling

Hello, all.

This patchset is composed of 15 patches and aims to...

1. push more configuration responsibilities to higher level (from ata_set_mode to ata_bus_probe)
2. implement speed down mechanism to be used by ata_bus_probe and later by EH
3. improve ata_bus_probe such that it handles configuration failures better (speed down, reset & retry)

The patches can be grouped as follows.

#01-05 : misc fixes/preprations around the affected code area
#06-08 : make ata_set_mode() fail instead of disabling devices directly
#09-13 : implement SATA SPD / transfer mode speed down mechanism
#14-15 : improve probing process such that it handles failures better

#05 is identical to the previously posted version[1]. #06-08 are reimplementaion of [2].

Thanks.

--
tejun

[1] http://article.gmane.org/gmane.linux.ide/9035
[2] http://article.gmane.org/gmane.linux.ide/9036

Tejun Heo

author

2006-03-31 16:38 UTC #3 lore raw

PATCH 2/15 libata: make ata_bus_probe() return negative errno on failure

ata_bus_probe() uses unsigned int rc to receive negative errno and returns the converted unsigned int value. Convert temporary variables to int and make ata_bus_probe() return negative errno on failure.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

f1cffe0d11839b2aa79cc6ae1517715566b18083

drivers/scsi/libata-core.c +3 -3

index 8def7a5..c7ba523 100644
@@ -1349,13 +1349,13 @@ err_out_nosup:
  *	PCI/etc. bus probe sem.
  *
  *	RETURNS:
- *	Zero on success, non-zero on error.
+ *	Zero on success, negative errno otherwise.
  */
 
 static int ata_bus_probe(struct ata_port *ap)
 {
 	unsigned int classes[ATA_MAX_DEVICES];
-	unsigned int i, rc, found = 0;
+	int i, rc, found = 0;
 
 	ata_port_probe(ap);

@@ -1421,7 +1421,7 @@ static int ata_bus_probe(struct ata_port
 
 err_out_disable:
 	ap->ops->port_disable(ap);
-	return -1;
+	return -ENODEV;
 }
 
 /**

Signature · 1 line

1.2.4

Tejun Heo

author

2006-03-31 16:38 UTC #5 lore raw

PATCH 4/15 libata: convert do_probe_reset() to ata_do_reset()

In reply to Tejun Heo

Make do_probe_reset() generic by pushing classification check into ata_drive_probe_reset() and rename it to ata_do_reset(). This will be used by EH reset.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |   29 ++++++++++++++++-------------
 1 files changed, 16 insertions(+), 13 deletions(-)

de677bcf0480fe1ca900f5156f48af2a639570c4

drivers/scsi/libata-core.c +16 ~~-13~~

index 6a336ba..559abe4 100644
@@ -2371,16 +2371,16 @@ int ata_std_probe_reset(struct ata_port 
 				     ata_std_postreset, classes);
 }
 
-static int do_probe_reset(struct ata_port *ap, ata_reset_fn_t reset,
-			  ata_postreset_fn_t postreset,
-			  unsigned int *classes)
+static int ata_do_reset(struct ata_port *ap,
+			ata_reset_fn_t reset, ata_postreset_fn_t postreset,
+			int verbose, unsigned int *classes)
 {
 	int i, rc;
 
 	for (i = 0; i < ATA_MAX_DEVICES; i++)
 		classes[i] = ATA_DEV_UNKNOWN;
 
-	rc = reset(ap, 0, classes);
+	rc = reset(ap, verbose, classes);
 	if (rc)
 		return rc;

@@ -2400,7 +2400,7 @@ static int do_probe_reset(struct ata_por
 	if (postreset)
 		postreset(ap, classes);
 
-	return classes[0] != ATA_DEV_UNKNOWN ? 0 : -ENODEV;
+	return 0;
 }
 
 /**

@@ -2445,21 +2445,24 @@ int ata_drive_probe_reset(struct ata_por
 		probeinit(ap);
 
 	if (softreset) {
-		rc = do_probe_reset(ap, softreset, postreset, classes);
-		if (rc == 0)
-			return 0;
+		rc = ata_do_reset(ap, softreset, postreset, 0, classes);
+		if (rc == 0 && classes[0] != ATA_DEV_UNKNOWN)
+			goto done;
 	}
 
 	if (!hardreset)
-		return rc;
+		goto done;
 
-	rc = do_probe_reset(ap, hardreset, postreset, classes);
-	if (rc == 0 || rc != -ENODEV)
-		return rc;
+	rc = ata_do_reset(ap, hardreset, postreset, 0, classes);
+	if (rc || classes[0] != ATA_DEV_UNKNOWN)
+		goto done;
 
 	if (softreset)
-		rc = do_probe_reset(ap, softreset, postreset, classes);
+		rc = ata_do_reset(ap, softreset, postreset, 0, classes);
 
+ done:
+	if (rc == 0 && classes[0] == ATA_DEV_UNKNOWN)
+		rc = -ENODEV;
 	return rc;
 }

Signature · 1 line

1.2.4

Tejun Heo

author

2006-03-31 16:38 UTC #10 lore raw

PATCH 7/15 libata: reorganize ata_set_mode()

In reply to Tejun Heo

Merge ata_host_set_pio() and ata_host_set_dma() into ata_set_mode() and use function-level *dev to iterate over devices. This eases soon-to-follow ata_set_mode() interface change.

While at it, kill an unnecessary comment and normalize others.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |   91 +++++++++++++++++---------------------------
 1 files changed, 35 insertions(+), 56 deletions(-)

dd939109a2ed9d9ad567d3688eace13cca31a9e1

drivers/scsi/libata-core.c +35 ~~-56~~

index 913e378..f04561a 100644
@@ -1771,47 +1771,6 @@ static int ata_dev_set_mode(struct ata_p
 	return 0;
 }
 
-static int ata_host_set_pio(struct ata_port *ap)
-{
-	int i;
-
-	for (i = 0; i < ATA_MAX_DEVICES; i++) {
-		struct ata_device *dev = &ap->device[i];
-
-		if (!ata_dev_enabled(dev))
-			continue;
-
-		if (!dev->pio_mode) {
-			printk(KERN_WARNING "ata%u: no PIO support for device %d.\n", ap->id, i);
-			return -1;
-		}
-
-		dev->xfer_mode = dev->pio_mode;
-		dev->xfer_shift = ATA_SHIFT_PIO;
-		if (ap->ops->set_piomode)
-			ap->ops->set_piomode(ap, dev);
-	}
-
-	return 0;
-}
-
-static void ata_host_set_dma(struct ata_port *ap)
-{
-	int i;
-
-	for (i = 0; i < ATA_MAX_DEVICES; i++) {
-		struct ata_device *dev = &ap->device[i];
-
-		if (!ata_dev_enabled(dev) || !dev->dma_mode)
-			continue;
-
-		dev->xfer_mode = dev->dma_mode;
-		dev->xfer_shift = ata_xfer_mode2shift(dev->dma_mode);
-		if (ap->ops->set_dmamode)
-			ap->ops->set_dmamode(ap, dev);
-	}
-}
-
 /**
  *	ata_set_mode - Program timings and issue SET FEATURES - XFER
  *	@ap: port on which timings will be programmed

@@ -1823,20 +1782,20 @@ static void ata_host_set_dma(struct ata_
  */
 static void ata_set_mode(struct ata_port *ap)
 {
+	struct ata_device *dev;
 	int i, rc, used_dma = 0, found = 0;
 
 	/* step 1: calculate xfer_mask */
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
-		struct ata_device *dev = &ap->device[i];
 		unsigned int pio_mask, dma_mask;
 
+		dev = &ap->device[i];
+
 		if (!ata_dev_enabled(dev))
 			continue;
 
 		ata_dev_xfermask(ap, dev);
 
-		/* TODO: let LLDD filter dev->*_mask here */
-
 		pio_mask = ata_pack_xfermask(dev->pio_mask, 0, 0);
 		dma_mask = ata_pack_xfermask(0, dev->mwdma_mask, dev->udma_mask);
 		dev->pio_mode = ata_xfer_mask2mode(pio_mask);

@@ -1850,16 +1809,40 @@ static void ata_set_mode(struct ata_port
 		return;
 
 	/* step 2: always set host PIO timings */
-	rc = ata_host_set_pio(ap);
-	if (rc)
-		goto err_out;
+	for (i = 0; i < ATA_MAX_DEVICES; i++) {
+		dev = &ap->device[i];
+		if (!ata_dev_enabled(dev))
+			continue;
+
+		if (!dev->pio_mode) {
+			printk(KERN_WARNING "ata%u: dev %u no PIO support\n",
+			       ap->id, dev->devno);
+			rc = -EINVAL;
+			goto err_out;
+		}
+
+		dev->xfer_mode = dev->pio_mode;
+		dev->xfer_shift = ATA_SHIFT_PIO;
+		if (ap->ops->set_piomode)
+			ap->ops->set_piomode(ap, dev);
+	}
 
 	/* step 3: set host DMA timings */
-	ata_host_set_dma(ap);
+	for (i = 0; i < ATA_MAX_DEVICES; i++) {
+		dev = &ap->device[i];
+
+		if (!ata_dev_enabled(dev) || !dev->dma_mode)
+			continue;
+
+		dev->xfer_mode = dev->dma_mode;
+		dev->xfer_shift = ata_xfer_mode2shift(dev->dma_mode);
+		if (ap->ops->set_dmamode)
+			ap->ops->set_dmamode(ap, dev);
+	}
 
 	/* step 4: update devices' xfer mode */
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
-		struct ata_device *dev = &ap->device[i];
+		dev = &ap->device[i];
 
 		if (!ata_dev_enabled(dev))
 			continue;

@@ -1869,17 +1852,13 @@ static void ata_set_mode(struct ata_port
 			goto err_out;
 	}
 
-	/*
-	 *	Record simplex status. If we selected DMA then the other
-	 *	host channels are not permitted to do so.
+	/* Record simplex status. If we selected DMA then the other
+	 * host channels are not permitted to do so.
 	 */
-
 	if (used_dma && (ap->host_set->flags & ATA_HOST_SIMPLEX))
 		ap->host_set->simplex_claimed = 1;
 
-	/*
-	 *	Chip specific finalisation
-	 */
+	/* step5: chip specific finalisation */
 	if (ap->ops->post_set_mode)
 		ap->ops->post_set_mode(ap);

Signature · 1 line

1.2.4

Tejun Heo

author

2006-03-31 16:38 UTC #11 lore raw

PATCH 8/15 libata: don't disable devices from ata_set_mode()

In reply to Tejun Heo

When ata_set_mode() fails on a device, make ata_set_mode() return error code and pointer to the device instead of disabling it directly. This gives more control to higher level driving logic.

This patch does not change the end result (configured transfer mode) although it may make libata repeat mode configuration to the peer of a failing device. Later ata_bus_probe() rewrite will make full use of this change.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |   66 +++++++++++++++++++++++++++-----------------
 1 files changed, 40 insertions(+), 26 deletions(-)

0a8249272a7bb4114c8c535ad9c09f6c98fbccf2

drivers/scsi/libata-core.c +40 ~~-26~~

index f04561a..061b0b6 100644
@@ -65,7 +65,7 @@ static unsigned int ata_dev_init_params(
 					struct ata_device *dev,
 					u16 heads,
 					u16 sectors);
-static void ata_set_mode(struct ata_port *ap);
+static int ata_set_mode(struct ata_port *ap, struct ata_device **r_failed_dev);
 static unsigned int ata_dev_set_xfermode(struct ata_port *ap,
 					 struct ata_device *dev);
 static void ata_dev_xfermask(struct ata_port *ap, struct ata_device *dev);

@@ -1368,6 +1368,7 @@ static int ata_bus_probe(struct ata_port
 {
 	unsigned int classes[ATA_MAX_DEVICES];
 	int i, rc, found = 0;
+	struct ata_device *dev;
 
 	ata_port_probe(ap);

@@ -1397,8 +1398,7 @@ static int ata_bus_probe(struct ata_port
 
 	/* read IDENTIFY page and configure devices */
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
-		struct ata_device *dev = &ap->device[i];
-
+		dev = &ap->device[i];
 		dev->class = classes[i];
 
 		if (!ata_dev_enabled(dev))

@@ -1418,20 +1418,26 @@ static int ata_bus_probe(struct ata_port
 		found = 1;
 	}
 
-	if (!found)
-		goto err_out_disable;
-
-	if (ap->ops->set_mode)
-		ap->ops->set_mode(ap);
-	else
-		ata_set_mode(ap);
-
-	if (ap->flags & ATA_FLAG_PORT_DISABLED)
-		goto err_out_disable;
+	/* configure transfer mode */
+	if (ap->ops->set_mode) {
+		/* FIXME: make ->set_mode handle no device case and
+		 * return error code and failing device on failure as
+		 * ata_set_mode() does.
+		 */
+		if (found)
+			ap->ops->set_mode(ap);
+		rc = 0;
+	} else {
+		while (ata_set_mode(ap, &dev))
+			ata_dev_disable(ap, dev);
+	}
 
-	return 0;
+	for (i = 0; i < ATA_MAX_DEVICES; i++)
+		if (ata_dev_enabled(&ap->device[i]))
+			return 0;
 
-err_out_disable:
+	/* no device present, disable port */
+	ata_port_disable(ap);
 	ap->ops->port_disable(ap);
 	return -ENODEV;
 }

@@ -1774,16 +1780,22 @@ static int ata_dev_set_mode(struct ata_p
 /**
  *	ata_set_mode - Program timings and issue SET FEATURES - XFER
  *	@ap: port on which timings will be programmed
+ *	@r_failed_dev: out paramter for failed device
  *
- *	Set ATA device disk transfer mode (PIO3, UDMA6, etc.).
+ *	Set ATA device disk transfer mode (PIO3, UDMA6, etc.).  If
+ *	ata_set_mode() fails, pointer to the failing device is
+ *	returned in @r_failed_dev.
  *
  *	LOCKING:
  *	PCI/etc. bus probe sem.
+ *
+ *	RETURNS:
+ *	0 on success, negative errno otherwise
  */
-static void ata_set_mode(struct ata_port *ap)
+static int ata_set_mode(struct ata_port *ap, struct ata_device **r_failed_dev)
 {
 	struct ata_device *dev;
-	int i, rc, used_dma = 0, found = 0;
+	int i, rc = 0, used_dma = 0, found = 0;
 
 	/* step 1: calculate xfer_mask */
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {

@@ -1806,7 +1818,7 @@ static void ata_set_mode(struct ata_port
 			used_dma = 1;
 	}
 	if (!found)
-		return;
+		goto out;
 
 	/* step 2: always set host PIO timings */
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {

@@ -1818,7 +1830,7 @@ static void ata_set_mode(struct ata_port
 			printk(KERN_WARNING "ata%u: dev %u no PIO support\n",
 			       ap->id, dev->devno);
 			rc = -EINVAL;
-			goto err_out;
+			goto out;
 		}
 
 		dev->xfer_mode = dev->pio_mode;

@@ -1849,7 +1861,7 @@ static void ata_set_mode(struct ata_port
 
 		rc = ata_dev_set_mode(ap, dev);
 		if (rc)
-			goto err_out;
+			goto out;
 	}
 
 	/* Record simplex status. If we selected DMA then the other

@@ -1862,10 +1874,10 @@ static void ata_set_mode(struct ata_port
 	if (ap->ops->post_set_mode)
 		ap->ops->post_set_mode(ap);
 
-	return;
-
-err_out:
-	ata_port_disable(ap);
+ out:
+	if (rc)
+		*r_failed_dev = dev;
+	return rc;
 }
 
 /**

@@ -4278,8 +4290,10 @@ static int ata_start_drive(struct ata_po
 int ata_device_resume(struct ata_port *ap, struct ata_device *dev)
 {
 	if (ap->flags & ATA_FLAG_SUSPENDED) {
+		struct ata_device *failed_dev;
 		ap->flags &= ~ATA_FLAG_SUSPENDED;
-		ata_set_mode(ap);
+		while (ata_set_mode(ap, &failed_dev))
+			ata_dev_disable(ap, failed_dev);
 	}
 	if (!ata_dev_enabled(dev))
 		return 0;

Signature · 1 line

1.2.4

Tejun Heo

author

2006-03-31 16:38 UTC #13 lore raw

PATCH 9/15 libata: implement ata_down_xfermask_limit()

In reply to Tejun Heo

Implement ata_down_xfermask_limit(). This function manipulates @dev->pio/mwdma/udma_mask such that the next lower transfer mode is selected. This will be used by ata_bus_probe() rewrite and later by EH speeding down.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |   52 ++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 52 insertions(+), 0 deletions(-)

8360efb5b24dfe6b4cc63f00cec3648f14b105a0

drivers/scsi/libata-core.c +52 -0

index 061b0b6..5dee649 100644
@@ -65,6 +65,8 @@ static unsigned int ata_dev_init_params(
 					struct ata_device *dev,
 					u16 heads,
 					u16 sectors);
+static int ata_down_xfermask_limit(struct ata_port *ap, struct ata_device *dev,
+				   int force_pio0);
 static int ata_set_mode(struct ata_port *ap, struct ata_device **r_failed_dev);
 static unsigned int ata_dev_set_xfermode(struct ata_port *ap,
 					 struct ata_device *dev);

@@ -1744,6 +1746,56 @@ int ata_timing_compute(struct ata_device
 	return 0;
 }
 
+/**
+ *	ata_down_xfermask_limit - adjust dev xfer masks downward
+ *	@ap: Port associated with device @dev
+ *	@dev: Device to adjust xfer masks
+ *	@force_pio0: Force PIO0
+ *
+ *	Adjust xfer masks of @dev downward.  Note that this function
+ *	does not apply the change.  Invoking ata_set_mode() afterwards
+ *	will apply the limit.
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ *
+ *	RETURNS:
+ *	0 on success, negative errno on failure
+ */
+static int ata_down_xfermask_limit(struct ata_port *ap, struct ata_device *dev,
+				   int force_pio0)
+{
+	unsigned long xfer_mask;
+	int highbit;
+
+	xfer_mask = ata_pack_xfermask(dev->pio_mask, dev->mwdma_mask,
+				      dev->udma_mask);
+
+	if (!xfer_mask)
+		goto fail;
+	/* don't gear down to MWDMA from UDMA, go directly to PIO */
+	if (xfer_mask & ATA_MASK_UDMA)
+		xfer_mask &= ~ATA_MASK_MWDMA;
+
+	highbit = fls(xfer_mask) - 1;
+	xfer_mask &= ~(1 << highbit);
+	if (force_pio0)
+		xfer_mask &= 1 << ATA_SHIFT_PIO;
+	if (!xfer_mask)
+		goto fail;
+
+	ata_unpack_xfermask(xfer_mask, &dev->pio_mask, &dev->mwdma_mask,
+			    &dev->udma_mask);
+
+	printk(KERN_WARNING "ata%u: dev %u limiting speed to %s\n",
+	       ap->id, dev->devno, ata_mode_string(xfer_mask));
+
+	return 0;
+
+ fail:
+	return -EINVAL;
+}
+
 static int ata_dev_set_mode(struct ata_port *ap, struct ata_device *dev)
 {
 	unsigned int err_mask;

Signature · 1 line

1.2.4

Tejun Heo

author

2006-03-31 16:38 UTC #20 lore raw

PATCH 10/15 libata: add dev->sata_spd_limit and helpers

In reply to Tejun Heo

dev->sata_spd_limit contrains SATA PHY speed of the device and is initialized to the configured value on probing.

Currently, This is only valid for dev[0] but still put into ata_device instead of ata_port. This is because 1. it's device constraints (ie. when hotpluggin, it should be cleared with other device info)
2. for port multiplier, link actually belongs to each device.

Three helper functions - ata_down_sata_spd_limit(), ata_set_sata_spd_needed() and ata_set_sata_spd() - are implemented. They will be used by ata_bus_probe() rewrite and later by EH.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |  134 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/libata.h     |    3 +
 2 files changed, 137 insertions(+), 0 deletions(-)

d9f82105c7dbc64bcc679b3974147928eccc5fa1

drivers/scsi/libata-core.c +134 -0

index 5dee649..283994d 100644
@@ -65,6 +65,7 @@ static unsigned int ata_dev_init_params(
 					struct ata_device *dev,
 					u16 heads,
 					u16 sectors);
+static int ata_down_sata_spd_limit(struct ata_port *ap, struct ata_device *dev);
 static int ata_down_xfermask_limit(struct ata_port *ap, struct ata_device *dev,
 				   int force_pio0);
 static int ata_set_mode(struct ata_port *ap, struct ata_device **r_failed_dev);

@@ -1598,6 +1599,131 @@ void ata_port_disable(struct ata_port *a
 	ap->flags |= ATA_FLAG_PORT_DISABLED;
 }
 
+/**
+ *	ata_down_sata_spd_limit - adjust SATA spd limit downward
+ *	@ap: Port associated with device @dev
+ *	@dev: Device to adjust SATA spd limit
+ *
+ *	Adjust SATA spd limit of @dev downward.  Note that this
+ *	function only adjusts the limit.  The change must be applied
+ *	using ata_set_sata_spd().
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ *
+ *	RETURNS:
+ *	0 on success, negative errno on failure
+ */
+static int ata_down_sata_spd_limit(struct ata_port *ap, struct ata_device *dev)
+{
+	u32 spd, mask;
+	int highbit;
+
+	if (ap->cbl != ATA_CBL_SATA || !ap->ops->scr_read)
+		return -EOPNOTSUPP;
+
+	mask = dev->sata_spd_limit;
+	if (mask <= 1)
+		return -EINVAL;
+	highbit = fls(mask) - 1;
+	mask &= ~(1 << highbit);
+
+	spd = (scr_read(ap, SCR_STATUS) >> 4) & 0xf;
+	if (spd <= 1)
+		return -EINVAL;
+	spd--;
+	mask &= (1 << spd) - 1;
+	if (!mask)
+		return -EINVAL;
+
+	dev->sata_spd_limit = mask;
+
+	printk(KERN_WARNING "ata%u: dev %u limiting SATA link speed to %s\n",
+	       ap->id, dev->devno, sata_spd_string(fls(mask)));
+
+	return 0;
+}
+
+static int __ata_set_sata_spd_needed(struct ata_port *ap,
+				     struct ata_device *dev, u32 *scontrol)
+{
+	u32 spd, limit;
+
+	if (!ata_dev_enabled(dev))
+		return 0;
+
+	if (dev->sata_spd_limit == UINT_MAX)
+		limit = 0;
+	else
+		limit = fls(dev->sata_spd_limit);
+
+	spd = (*scontrol >> 4) & 0xf;
+	*scontrol = (*scontrol & ~0xf0) | ((limit & 0xf) << 4);
+
+	return spd != limit;
+}
+
+/**
+ *	ata_set_sata_spd_needed - is SATA spd configuration needed
+ *	@ap: Port associated with device @dev
+ *	@dev: Device in question
+ *
+ *	Test whether the spd limit in SControl matches
+ *	@dev->sata_spd_limit.  This function is used to determine
+ *	whether hardreset is necessary to apply SATA spd
+ *	configuration.
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ *
+ *	RETURNS:
+ *	1 if SATA spd configuration is needed, 0 otherwise.
+ */
+static int ata_set_sata_spd_needed(struct ata_port *ap, struct ata_device *dev)
+{
+	u32 scontrol;
+
+	if (!ata_dev_enabled(dev) ||
+	    ap->cbl != ATA_CBL_SATA || !ap->ops->scr_read)
+		return 0;
+
+	scontrol = scr_read(ap, SCR_CONTROL);
+
+	return __ata_set_sata_spd_needed(ap, dev, &scontrol);
+}
+
+/**
+ *	ata_set_sata_spd - set SATA spd according to spd limit
+ *	@ap: Port associated with device @dev
+ *	@dev: Device to set SATA spd
+ *
+ *	Set SATA spd of @ap according to sata_spd_limit.
+ *
+ *	LOCKING:
+ *	Inherited from caller.
+ *
+ *	RETURNS:
+ *	0 if spd doesn't need to be changed, 1 if spd has been
+ *	changed.  -EOPNOTSUPP if SCR registers are inaccessible.
+ */
+static int ata_set_sata_spd(struct ata_port *ap, struct ata_device *dev)
+{
+	u32 scontrol;
+
+	if (ap->cbl != ATA_CBL_SATA || !ap->ops->scr_read)
+		return -EOPNOTSUPP;
+
+	if (!ata_dev_enabled(dev))
+		return 0;
+
+	scontrol = scr_read(ap, SCR_CONTROL);
+	if (!__ata_set_sata_spd_needed(ap, dev, &scontrol))
+		return 0;
+
+	scr_write(ap, SCR_CONTROL, scontrol);
+	return 1;
+}
+
 /*
  * This mode timing computation functionality is ported over from
  * drivers/ide/ide-timing.h and was originally written by Vojtech Pavlik

@@ -2215,7 +2341,14 @@ static int sata_phy_resume(struct ata_po
 void ata_std_probeinit(struct ata_port *ap)
 {
 	if ((ap->flags & ATA_FLAG_SATA) && ap->ops->scr_read) {
+		u32 spd;
+
 		sata_phy_resume(ap);
+
+		spd = (scr_read(ap, SCR_CONTROL) & 0xf0) >> 4;
+		if (spd)
+			ap->device[0].sata_spd_limit &= (1 << spd) - 1;
+
 		if (sata_dev_present(ap))
 			ata_busy_sleep(ap, ATA_TMOUT_BOOT_QUICK, ATA_TMOUT_BOOT);
 	}

@@ -4512,6 +4645,7 @@ static void ata_host_init(struct ata_por
 		dev->pio_mask = UINT_MAX;
 		dev->mwdma_mask = UINT_MAX;
 		dev->udma_mask = UINT_MAX;
+		dev->sata_spd_limit = UINT_MAX;
 	}
 
 #ifdef ATA_IRQ_TRAP

include/linux/libata.h +3 -0

index c6883ba..d720f5b 100644
@@ -369,6 +369,9 @@ struct ata_device {
 	unsigned int		mwdma_mask;
 	unsigned int		udma_mask;
 
+	/* SATA PHY speed limit */
+	unsigned int		sata_spd_limit;
+
 	/* for CHS addressing */
 	u16			cylinders;	/* Number of cylinders */
 	u16			heads;		/* Number of heads */

Signature · 1 line

1.2.4

Tejun Heo

author

2006-03-31 16:38 UTC #9 lore raw

PATCH 6/15 libata: make ata_set_mode() handle no-device case properly

In reply to Tejun Heo

Make ata_set_mode() return without doing anything if there is no device on the port. This is in preparation for ata_bus_probe() changes.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

62f7d91fc4040f3acd6b6f0ed8b8c863c40727d4

drivers/scsi/libata-core.c +4 -1

index c10c550..913e378 100644
@@ -1823,7 +1823,7 @@ static void ata_host_set_dma(struct ata_
  */
 static void ata_set_mode(struct ata_port *ap)
 {
-	int i, rc, used_dma = 0;
+	int i, rc, used_dma = 0, found = 0;
 
 	/* step 1: calculate xfer_mask */
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {

@@ -1842,9 +1842,12 @@ static void ata_set_mode(struct ata_port
 		dev->pio_mode = ata_xfer_mask2mode(pio_mask);
 		dev->dma_mode = ata_xfer_mask2mode(dma_mask);
 
+		found = 1;
 		if (dev->dma_mode)
 			used_dma = 1;
 	}
+	if (!found)
+		return;
 
 	/* step 2: always set host PIO timings */
 	rc = ata_host_set_pio(ap);

Signature · 1 line

1.2.4

Tejun Heo

author

2006-03-31 16:38 UTC #6 lore raw

PATCH 5/15 libata: implement ata_dev_enabled and disabled()

In reply to Tejun Heo

This patch renames ata_dev_present() to ata_dev_enabled() and adds ata_dev_disabled(). This is to discern the state where a device is present but disabled from not-present state. This disctinction is necessary when configuring transfer mode because device selection timing must not be violated even if a device fails to configure.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |   24 ++++++++++++------------
 drivers/scsi/libata-scsi.c |    4 ++--
 drivers/scsi/sata_mv.c     |    2 +-
 drivers/scsi/sata_sil.c    |    2 +-
 include/linux/libata.h     |   16 +++++++++++++---
 5 files changed, 29 insertions(+), 19 deletions(-)

cdce7bff674e9bcccb20e278bfe371c221ef2433

drivers/scsi/libata-core.c +12 ~~-12~~

index 559abe4..c10c550 100644
@@ -411,7 +411,7 @@ static const char *sata_spd_string(unsig
 
 static void ata_dev_disable(struct ata_port *ap, struct ata_device *dev)
 {
-	if (ata_dev_present(dev)) {
+	if (ata_dev_enabled(dev)) {
 		printk(KERN_WARNING "ata%u: dev %u disabled\n",
 		       ap->id, dev->devno);
 		dev->class++;

@@ -1222,7 +1222,7 @@ static int ata_dev_configure(struct ata_
 	unsigned int xfer_mask;
 	int i, rc;
 
-	if (!ata_dev_present(dev)) {
+	if (!ata_dev_enabled(dev)) {
 		DPRINTK("ENTER/EXIT (host %u, dev %u) -- nodev\n",
 			ap->id, dev->devno);
 		return 0;

@@ -1401,7 +1401,7 @@ static int ata_bus_probe(struct ata_port
 
 		dev->class = classes[i];
 
-		if (!ata_dev_present(dev))
+		if (!ata_dev_enabled(dev))
 			continue;
 
 		WARN_ON(dev->id != NULL);

@@ -1565,7 +1565,7 @@ void sata_phy_reset(struct ata_port *ap)
 struct ata_device *ata_dev_pair(struct ata_port *ap, struct ata_device *adev)
 {
 	struct ata_device *pair = &ap->device[1 - adev->devno];
-	if (!ata_dev_present(pair))
+	if (!ata_dev_enabled(pair))
 		return NULL;
 	return pair;
 }

@@ -1778,7 +1778,7 @@ static int ata_host_set_pio(struct ata_p
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		struct ata_device *dev = &ap->device[i];
 
-		if (!ata_dev_present(dev))
+		if (!ata_dev_enabled(dev))
 			continue;
 
 		if (!dev->pio_mode) {

@@ -1802,7 +1802,7 @@ static void ata_host_set_dma(struct ata_
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		struct ata_device *dev = &ap->device[i];
 
-		if (!ata_dev_present(dev) || !dev->dma_mode)
+		if (!ata_dev_enabled(dev) || !dev->dma_mode)
 			continue;
 
 		dev->xfer_mode = dev->dma_mode;

@@ -1830,7 +1830,7 @@ static void ata_set_mode(struct ata_port
 		struct ata_device *dev = &ap->device[i];
 		unsigned int pio_mask, dma_mask;
 
-		if (!ata_dev_present(dev))
+		if (!ata_dev_enabled(dev))
 			continue;
 
 		ata_dev_xfermask(ap, dev);

@@ -1858,7 +1858,7 @@ static void ata_set_mode(struct ata_port
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		struct ata_device *dev = &ap->device[i];
 
-		if (!ata_dev_present(dev))
+		if (!ata_dev_enabled(dev))
 			continue;
 
 		rc = ata_dev_set_mode(ap, dev);

@@ -2550,7 +2550,7 @@ int ata_dev_revalidate(struct ata_port *
 	u16 *id;
 	int rc;
 
-	if (!ata_dev_present(dev))
+	if (!ata_dev_enabled(dev))
 		return -ENODEV;
 
 	class = dev->class;

@@ -2679,7 +2679,7 @@ static void ata_dev_xfermask(struct ata_
 	/* FIXME: Use port-wide xfermask for now */
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		struct ata_device *d = &ap->device[i];
-		if (!ata_dev_present(d))
+		if (!ata_dev_enabled(d))
 			continue;
 		xfer_mask &= ata_pack_xfermask(d->pio_mask, d->mwdma_mask,
 					       d->udma_mask);

@@ -4299,7 +4299,7 @@ int ata_device_resume(struct ata_port *a
 		ap->flags &= ~ATA_FLAG_SUSPENDED;
 		ata_set_mode(ap);
 	}
-	if (!ata_dev_present(dev))
+	if (!ata_dev_enabled(dev))
 		return 0;
 	if (dev->class == ATA_DEV_ATA)
 		ata_start_drive(ap, dev);

@@ -4317,7 +4317,7 @@ int ata_device_resume(struct ata_port *a
  */
 int ata_device_suspend(struct ata_port *ap, struct ata_device *dev, pm_message_t state)
 {
-	if (!ata_dev_present(dev))
+	if (!ata_dev_enabled(dev))
 		return 0;
 	if (dev->class == ATA_DEV_ATA)
 		ata_flush_cache(ap, dev);

drivers/scsi/libata-scsi.c +2 -2

index 53f5b0d..c1a4b29 100644
@@ -2349,7 +2349,7 @@ ata_scsi_find_dev(struct ata_port *ap, c
 		     (scsidev->lun != 0)))
 		return NULL;
 
-	if (unlikely(!ata_dev_present(dev)))
+	if (unlikely(!ata_dev_enabled(dev)))
 		return NULL;
 
 	if (!atapi_enabled || (ap->flags & ATA_FLAG_NO_ATAPI)) {

@@ -2743,7 +2743,7 @@ void ata_scsi_scan_host(struct ata_port 
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		dev = &ap->device[i];
 
-		if (ata_dev_present(dev))
+		if (ata_dev_enabled(dev))
 			scsi_scan_target(&ap->host->shost_gendev, 0, i, 0, 0);
 	}
 }

drivers/scsi/sata_mv.c +1 -1

index fa901fd..0f7d334 100644
@@ -1991,7 +1991,7 @@ comreset_retry:
 	tf.nsect = readb((void __iomem *) ap->ioaddr.nsect_addr);
 
 	dev->class = ata_dev_classify(&tf);
-	if (!ata_dev_present(dev)) {
+	if (!ata_dev_enabled(dev)) {
 		VPRINTK("Port disabled post-sig: No device present.\n");
 		ata_port_disable(ap);
 	}

drivers/scsi/sata_sil.c +1 -1

index 18c296c..d6c7086 100644
@@ -264,7 +264,7 @@ static void sil_post_set_mode (struct at
 
 	for (i = 0; i < 2; i++) {
 		dev = &ap->device[i];
-		if (!ata_dev_present(dev))
+		if (!ata_dev_enabled(dev))
 			dev_mode[i] = 0;	/* PIO0/1/2 */
 		else if (dev->flags & ATA_DFLAG_PIO)
 			dev_mode[i] = 1;	/* PIO3/4 */

include/linux/libata.h +13 -3

index 0d61357..c6883ba 100644
@@ -672,14 +672,24 @@ static inline unsigned int ata_tag_valid
 	return (tag < ATA_MAX_QUEUE) ? 1 : 0;
 }
 
-static inline unsigned int ata_class_present(unsigned int class)
+static inline unsigned int ata_class_enabled(unsigned int class)
 {
 	return class == ATA_DEV_ATA || class == ATA_DEV_ATAPI;
 }
 
-static inline unsigned int ata_dev_present(const struct ata_device *dev)
+static inline unsigned int ata_class_disabled(unsigned int class)
 {
-	return ata_class_present(dev->class);
+	return class == ATA_DEV_ATA_UNSUP || class == ATA_DEV_ATAPI_UNSUP;
+}
+
+static inline unsigned int ata_dev_enabled(const struct ata_device *dev)
+{
+	return ata_class_enabled(dev->class);
+}
+
+static inline unsigned int ata_dev_disabled(const struct ata_device *dev)
+{
+	return ata_class_disabled(dev->class);
 }
 
 static inline u8 ata_chk_status(struct ata_port *ap)

Signature · 1 line

1.2.4

Tejun Heo

author

2006-03-31 16:38 UTC #29 lore raw

PATCH 13/15 libata: add 1s sleep between resets

In reply to Tejun Heo

Some devices react badly if resets are performed back-to-back. Give devices some time to breath.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

e7f505c001fd4cb43b8123387285a7694790b4ae

drivers/scsi/libata-core.c +2 -0

index 30ee203..bd9ca3b 100644
@@ -2633,6 +2633,7 @@ int ata_drive_probe_reset(struct ata_por
 		rc = ata_do_reset(ap, softreset, postreset, 0, classes);
 		if (rc == 0 && classes[0] != ATA_DEV_UNKNOWN)
 			goto done;
+		ssleep(1);
 	}
 
 	if (!hardreset)

@@ -2649,6 +2650,7 @@ int ata_drive_probe_reset(struct ata_por
 
 		if (ata_down_sata_spd_limit(ap, &ap->device[0]))
 			goto done;
+		ssleep(1);
 	}
 
 	if (softreset)

Signature · 1 line

1.2.4

Tejun Heo

author

2006-03-31 16:38 UTC #32 lore raw

PATCH 14/15 libata: improve ata_bus_probe()

In reply to Tejun Heo

Improve ata_bus_probe() such that configuration failures are handled better. Each device is given ATA_PROBE_MAX_TRIES chances, but any non-transient error (revalidation failure with -ENODEV, configuration failure with -EINVAL...) disables the device directly. Any IO error results in SATA PHY speed down and ata_set_mode() failure lowers transfer mode. The last try always puts a device into PIO-0.

After each failure, the whole port is reset to make sure that the controller and all the devices are in a known and stable state. The reset also applies SATA SPD configuration if necessary.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |   65 +++++++++++++++++++++++++++++++++-----------
 include/linux/libata.h     |    3 ++
 2 files changed, 52 insertions(+), 16 deletions(-)

5e0700f0f67bd53ca50ad34b47d3fb39072a734c

drivers/scsi/libata-core.c +49 ~~-16~~

index bd9ca3b..ac2ca31 100644
@@ -1370,11 +1370,18 @@ err_out_nosup:
 static int ata_bus_probe(struct ata_port *ap)
 {
 	unsigned int classes[ATA_MAX_DEVICES];
-	int i, rc, found = 0;
+	int tries[ATA_MAX_DEVICES];
+	int i, rc, down_xfermask;
 	struct ata_device *dev;
 
 	ata_port_probe(ap);
 
+	for (i = 0; i < ATA_MAX_DEVICES; i++)
+		tries[i] = ATA_PROBE_MAX_TRIES;
+
+ retry:
+	down_xfermask = 0;
+
 	/* reset and determine device classes */
 	for (i = 0; i < ATA_MAX_DEVICES; i++)
 		classes[i] = ATA_DEV_UNKNOWN;

@@ -1404,21 +1411,23 @@ static int ata_bus_probe(struct ata_port
 		dev = &ap->device[i];
 		dev->class = classes[i];
 
-		if (!ata_dev_enabled(dev))
-			continue;
-
-		WARN_ON(dev->id != NULL);
-		if (ata_dev_read_id(ap, dev, &dev->class, 1, &dev->id)) {
-			dev->class = ATA_DEV_NONE;
-			continue;
+		if (!tries[i]) {
+			ata_down_xfermask_limit(ap, dev, 1);
+			ata_dev_disable(ap, dev);
 		}
 
-		if (ata_dev_configure(ap, dev, 1)) {
-			ata_dev_disable(ap, dev);
+		if (!ata_dev_enabled(dev))
 			continue;
-		}
 
-		found = 1;
+		kfree(dev->id);
+		dev->id = NULL;
+		rc = ata_dev_read_id(ap, dev, &dev->class, 1, &dev->id);
+		if (rc)
+			goto fail;
+
+		rc = ata_dev_configure(ap, dev, 1);
+		if (rc)
+			goto fail;
 	}
 
 	/* configure transfer mode */

@@ -1427,12 +1436,18 @@ static int ata_bus_probe(struct ata_port
 		 * return error code and failing device on failure as
 		 * ata_set_mode() does.
 		 */
-		if (found)
-			ap->ops->set_mode(ap);
+		for (i = 0; i < ATA_MAX_DEVICES; i++)
+			if (ata_dev_enabled(&ap->device[i])) {
+				ap->ops->set_mode(ap);
+				break;
+			}
 		rc = 0;
 	} else {
-		while (ata_set_mode(ap, &dev))
-			ata_dev_disable(ap, dev);
+		rc = ata_set_mode(ap, &dev);
+		if (rc) {
+			down_xfermask = 1;
+			goto fail;
+		}
 	}
 
 	for (i = 0; i < ATA_MAX_DEVICES; i++)

@@ -1443,6 +1458,24 @@ static int ata_bus_probe(struct ata_port
 	ata_port_disable(ap);
 	ap->ops->port_disable(ap);
 	return -ENODEV;
+
+ fail:
+	switch (rc) {
+	case -EINVAL:
+	case -ENODEV:
+		tries[dev->devno] = 0;
+		break;
+	case -EIO:
+		ata_down_sata_spd_limit(ap, dev);
+		/* fall through */
+	default:
+		tries[dev->devno]--;
+		if (down_xfermask &&
+		    ata_down_xfermask_limit(ap, dev, tries[dev->devno] == 1))
+			tries[dev->devno] = 0;
+	}
+
+	goto retry;
 }
 
 /**

include/linux/libata.h +3 -0

index d720f5b..c246a15 100644
@@ -211,6 +211,9 @@ enum {
 	/* Masks for port functions */
 	ATA_PORT_PRIMARY	= (1 << 0),
 	ATA_PORT_SECONDARY	= (1 << 1),
+
+	/* how hard are we gonna try to probe/recover devices */
+	ATA_PROBE_MAX_TRIES	= 3,
 };
 
 enum hsm_task_states {

Signature · 1 line

1.2.4

Tejun Heo

author

2006-03-31 16:38 UTC #34 lore raw

PATCH 15/15 libata: consider disabled devices in ata_dev_xfermask()

In reply to Tejun Heo

ata_bus_probe() now marks failed devices properly and leaves meaningful transfer mode masks. This patch makes ata_dev_xfermask() consider disable devices when determining PIO mode to avoid violating device selection timing.

While at it, move port-wide resttriction out of device iteration loop and try to make the function look a bit prettier.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |   34 +++++++++++++++++++++++-----------
 1 files changed, 23 insertions(+), 11 deletions(-)

523b496b7614981773c2c14259bbca4a7ba3a3e8

drivers/scsi/libata-core.c +23 ~~-11~~

index ac2ca31..0d72792 100644
@@ -2902,23 +2902,34 @@ static void ata_dev_xfermask(struct ata_
 	unsigned long xfer_mask;
 	int i;
 
-	xfer_mask = ata_pack_xfermask(ap->pio_mask, ap->mwdma_mask,
-				      ap->udma_mask);
+	xfer_mask = ata_pack_xfermask(ap->pio_mask,
+				      ap->mwdma_mask, ap->udma_mask);
+
+	/* Apply cable rule here.  Don't apply it early because when
+	 * we handle hot plug the cable type can itself change.
+	 */
+	if (ap->cbl == ATA_CBL_PATA40)
+		xfer_mask &= ~(0xF8 << ATA_SHIFT_UDMA);
 
 	/* FIXME: Use port-wide xfermask for now */
 	for (i = 0; i < ATA_MAX_DEVICES; i++) {
 		struct ata_device *d = &ap->device[i];
-		if (!ata_dev_enabled(d))
+
+		if (!ata_dev_enabled(d) && !ata_dev_disabled(d))
+			continue;
+
+		if (ata_dev_disabled(d)) {
+			/* to avoid violating device selection timing */
+			xfer_mask &= ata_pack_xfermask(d->pio_mask,
+						       UINT_MAX, UINT_MAX);
 			continue;
-		xfer_mask &= ata_pack_xfermask(d->pio_mask, d->mwdma_mask,
-					       d->udma_mask);
+		}
+
+		xfer_mask &= ata_pack_xfermask(d->pio_mask,
+					       d->mwdma_mask, d->udma_mask);
 		xfer_mask &= ata_id_xfermask(d->id);
 		if (ata_dma_blacklisted(d))
 			xfer_mask &= ~(ATA_MASK_MWDMA | ATA_MASK_UDMA);
-		/* Apply cable rule here. Don't apply it early because when
-		   we handle hot plug the cable type can itself change */
-		if (ap->cbl == ATA_CBL_PATA40)
-			xfer_mask &= ~(0xF8 << ATA_SHIFT_UDMA);
 	}
 
 	if (ata_dma_blacklisted(dev))

@@ -2929,11 +2940,12 @@ static void ata_dev_xfermask(struct ata_
 		if (hs->simplex_claimed)
 			xfer_mask &= ~(ATA_MASK_MWDMA | ATA_MASK_UDMA);
 	}
+
 	if (ap->ops->mode_filter)
 		xfer_mask = ap->ops->mode_filter(ap, dev, xfer_mask);
 
-	ata_unpack_xfermask(xfer_mask, &dev->pio_mask, &dev->mwdma_mask,
-			    &dev->udma_mask);
+	ata_unpack_xfermask(xfer_mask, &dev->pio_mask,
+			    &dev->mwdma_mask, &dev->udma_mask);
 }
 
 /**

Signature · 1 line

1.2.4

Tejun Heo

author

2006-03-31 16:38 UTC #24 lore raw

PATCH 11/15 libata: preserve SATA SPD setting over hard resets

In reply to Tejun Heo

Don't overwrite SPD setting during hard reset. This change has the (intended) side effect of honoring the BIOS configuration.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

5f8ccbb89585c3d80973c49e086b5f6fd0732d56

drivers/scsi/libata-core.c +9 -3

index 283994d..16095d8 100644
@@ -2310,9 +2310,11 @@ err_out:
 static int sata_phy_resume(struct ata_port *ap)
 {
 	unsigned long timeout = jiffies + (HZ * 5);
-	u32 sstatus;
+	u32 scontrol, sstatus;
 
-	scr_write_flush(ap, SCR_CONTROL, 0x300);
+	scontrol = scr_read(ap, SCR_CONTROL);
+	scontrol = (scontrol & 0x0f0) | 0x300;
+	scr_write_flush(ap, SCR_CONTROL, scontrol);
 
 	/* Wait for phy to become ready, if necessary. */
 	do {

@@ -2432,10 +2434,14 @@ int ata_std_softreset(struct ata_port *a
  */
 int sata_std_hardreset(struct ata_port *ap, int verbose, unsigned int *class)
 {
+	u32 scontrol;
+
 	DPRINTK("ENTER\n");
 
 	/* Issue phy wake/reset */
-	scr_write_flush(ap, SCR_CONTROL, 0x301);
+	scontrol = scr_read(ap, SCR_CONTROL);
+	scontrol = (scontrol & 0x0f0) | 0x301;
+	scr_write_flush(ap, SCR_CONTROL, scontrol);
 
 	/*
 	 * Couldn't find anything in SATA I/II specs, but AHCI-1.1

Signature · 1 line

1.2.4

Tejun Heo

author

2006-03-31 16:38 UTC #26 lore raw

PATCH 12/15 libata: use SATA speeding down in ata_drive_probe_reset()

In reply to Tejun Heo

Make ata_drive_probe_reset() use SATA SPD configuration. Hardreset will be force if speed renegotiation is necessary. Also, if a hardreset fails, PHY speed is stepped down and hardreset is retried until the lowest speed is reached.

Signed-off-by Tejun Heo <[email protected]>

---

 drivers/scsi/libata-core.c |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)

c927e2f9d4688d9d34e7d1d5f7f2e34e86e93827

drivers/scsi/libata-core.c +13 -4

index 16095d8..30ee203 100644
@@ -2629,7 +2629,7 @@ int ata_drive_probe_reset(struct ata_por
 	if (probeinit)
 		probeinit(ap);
 
-	if (softreset) {
+	if (softreset && !ata_set_sata_spd_needed(ap, &ap->device[0])) {
 		rc = ata_do_reset(ap, softreset, postreset, 0, classes);
 		if (rc == 0 && classes[0] != ATA_DEV_UNKNOWN)
 			goto done;

@@ -2638,9 +2638,18 @@ int ata_drive_probe_reset(struct ata_por
 	if (!hardreset)
 		goto done;
 
-	rc = ata_do_reset(ap, hardreset, postreset, 0, classes);
-	if (rc || classes[0] != ATA_DEV_UNKNOWN)
-		goto done;
+	while (1) {
+		ata_set_sata_spd(ap, &ap->device[0]);
+		rc = ata_do_reset(ap, hardreset, postreset, 0, classes);
+		if (rc == 0) {
+			if (classes[0] != ATA_DEV_UNKNOWN)
+				goto done;
+			break;
+		}
+
+		if (ata_down_sata_spd_limit(ap, &ap->device[0]))
+			goto done;
+	}
 
 	if (softreset)
 		rc = ata_do_reset(ap, softreset, postreset, 0, classes);

Signature · 1 line

1.2.4

Alan Cox

2006-03-31 22:24 UTC #14 lore raw

Re: [PATCH 09/15] libata: implement ata_down_xfermask_limit()

In reply to Tejun Heo

Quoted 4 lines · Tejun Heo

Implement ata_down_xfermask_limit(). This function manipulates @dev->pio/mwdma/udma_mask such that the next lower transfer mode is selected. This will be used by ata_bus_probe() rewrite and later by EH speeding down.

This will need care. If it is called during a transfer mode change on error we should document that this function will only be called after the host_set has been quiesced, otherwise drivers that peek a lot at their ap-> mode settings at runtime (eg on an IRQ occuring during a speed change down) are going to burned badly.

It would be nice to see a description of the locking model for a changedown at this point.

Alan

Tejun Heo

author

2006-04-01 00:11 UTC #15 lore raw

Re: [PATCH 09/15] libata: implement ata_down_xfermask_limit()

Quoted 6 lines · Alan Cox

Quoted 4 lines · Tejun Heo
Implement ata_down_xfermask_limit(). This function manipulates @dev->pio/mwdma/udma_mask such that the next lower transfer mode is selected. This will be used by ata_bus_probe() rewrite and later by EH speeding down.

Quoted 9 lines

This will need care. If it is called during a transfer mode change on error we should document that this function will only be called after the host_set has been quiesced, otherwise drivers that peek a lot at their ap-> mode settings at runtime (eg on an IRQ occuring during a speed change down) are going to burned badly.
It would be nice to see a description of the locking model for a changedown at this point.

For new EH, every driver is responsible to freeze itself whenever it enters an unknown state. While frozen, a low level driver is required to either 1. plug interrupt completely from the port or 2. ignore interrupt from the port without any side effect (ACK and clear interrupts unconditionally). Once frozen, only reset can thaw the frozen state.

So, if anything which violates HSM occurs, LLDD freezes the port and EH gets invoked. EH kicks in, resets the port and reconfigure transfer mode if necessary. The EH revive routine looks pretty much like the new ata_bus_probe() except that it uses ata_dev_revalidate in place of ata_dev_read_id().

Thanks.

Signature · 1 line

tejun

Jeff Garzik

2006-04-01 17:29 UTC #7 lore raw

Re: [PATCH 05/15] libata: implement ata_dev_enabled and disabled()

In reply to Tejun Heo

Quoted 7 lines · Tejun Heo

This patch renames ata_dev_present() to ata_dev_enabled() and adds ata_dev_disabled(). This is to discern the state where a device is present but disabled from not-present state. This disctinction is necessary when configuring transfer mode because device selection timing must not be violated even if a device fails to configure.
Signed-off-by Tejun Heo <[email protected]>

Tangent thought: perhaps instead of ata_dev_present() you could implement its antonym, ata_dev_absent(). It seems like a lot of the constructs are "!dev_present()" anyway.

Jeff

Jeff Garzik

2006-04-01 17:34 UTC #2 lore raw

Re: [PATCH 01/15] libata: fix ata_set_mode() return value

In reply to Tejun Heo

Quoted 4 lines · Tejun Heo

Make ata_set_mode() return correct error value when ata_dev_set_mode() fails.
Signed-off-by Tejun Heo <[email protected]>

applied 1-7

Jeff Garzik

Applied

2006-04-01 19:46 UTC #12 lore raw

Re: [PATCH 08/15] libata: don't disable devices from ata_set_mode()

In reply to Tejun Heo

Quoted 10 lines · Tejun Heo

When ata_set_mode() fails on a device, make ata_set_mode() return error code and pointer to the device instead of disabling it directly. This gives more control to higher level driving logic.
This patch does not change the end result (configured transfer mode) although it may make libata repeat mode configuration to the peer of a failing device. Later ata_bus_probe() rewrite will make full use of this change.
Signed-off-by Tejun Heo <[email protected]>

applied

Jeff Garzik

2006-04-01 19:47 UTC #16 lore raw

Re: [PATCH 09/15] libata: implement ata_down_xfermask_limit()

In reply to Tejun Heo

Quoted 6 lines · Tejun Heo

Implement ata_down_xfermask_limit(). This function manipulates @dev->pio/mwdma/udma_mask such that the next lower transfer mode is selected. This will be used by ata_bus_probe() rewrite and later by EH speeding down.
Signed-off-by Tejun Heo <[email protected]>

ACK, though I share Alan's concerns, even with your explanation. We'll see where this leads...

Not applied because it will cause a compile warning.

Jeff

Jeff Garzik

NAK

2006-04-01 19:51 UTC #21 lore raw

Re: [PATCH 10/15] libata: add dev->sata_spd_limit and helpers

In reply to Tejun Heo

Quoted 8 lines · Tejun Heo

dev->sata_spd_limit contrains SATA PHY speed of the device and is initialized to the configured value on probing.
Currently, This is only valid for dev[0] but still put into ata_device instead of ata_port. This is because 1. it's device constraints (ie. when hotpluggin, it should be cleared with other device info)
2. for port multiplier, link actually belongs to each device.

Quoted 5 lines

Three helper functions - ata_down_sata_spd_limit(), ata_set_sata_spd_needed() and ata_set_sata_spd() - are implemented. They will be used by ata_bus_probe() rewrite and later by EH.
Signed-off-by Tejun Heo <[email protected]>

NAK, I definitely don't want to closely associate phy and device parameters, because they are fundamentally two different things.

We program and address PHYs, so perhaps a sata_phy struct is needed.

Port multipliers also get interesting because we need to use ATA-ish commands to talk to the PM phys. Perhaps a struct ata_port_mult inside ata_device is warranted, where one stores an array of sata_phy structs, and other PM-specific details.

Jeff

Jeff Garzik

Applied

2006-04-01 19:52 UTC #25 lore raw

Re: [PATCH 11/15] libata: preserve SATA SPD setting over hard resets

In reply to Tejun Heo

Quoted 4 lines · Tejun Heo

Don't overwrite SPD setting during hard reset. This change has the (intended) side effect of honoring the BIOS configuration.
Signed-off-by Tejun Heo <[email protected]>

applied

Jeff Garzik

2006-04-01 19:56 UTC #27 lore raw

Re: [PATCH 12/15] libata: use SATA speeding down in ata_drive_probe_reset()

In reply to Tejun Heo

Quoted 6 lines · Tejun Heo

Make ata_drive_probe_reset() use SATA SPD configuration. Hardreset will be force if speed renegotiation is necessary. Also, if a hardreset fails, PHY speed is stepped down and hardreset is retried until the lowest speed is reached.
Signed-off-by Tejun Heo <[email protected]>

Tentative ACK: I wonder if a naked COMRESET is the best procedure for changing PHY speeds. We might even power down the phy, change speeds, and then power it up again. Need to reach my SATA docs, and maybe talk to some device/controller vendors.

Jeff

Jeff Garzik

2006-04-01 19:57 UTC #30 lore raw

Re: [PATCH 13/15] libata: add 1s sleep between resets

In reply to Tejun Heo

Quoted 24 lines · Tejun Heo

Some devices react badly if resets are performed back-to-back. Give devices some time to breath.
Signed-off-by Tejun Heo <[email protected]>
---
 drivers/scsi/libata-core.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
e7f505c001fd4cb43b8123387285a7694790b4ae
drivers/scsi/libata-core.c +1 -0
index 30ee203..bd9ca3b 100644
@@ -2633,6 +2633,7 @@ int ata_drive_probe_reset(struct ata_por
 		rc = ata_do_reset(ap, softreset, postreset, 0, classes);
 		if (rc == 0 && classes[0] != ATA_DEV_UNKNOWN)
 			goto done;
+		ssleep(1);
 	}
 
 	if (!hardreset)

drivers/scsi/libata-core.c int ata_drive_probe_reset(struct ata_por · line 2653

Quoted 4 lines

drivers/scsi/libata-core.c +1 -0

@@ -2649,6 +2650,7 @@ int ata_drive_probe_reset(struct ata_por
 
 		if (ata_down_sata_spd_limit(ap, &ap->device[0]))
 			goto done;
+		ssleep(1);

My gut says it should be at least 5 seconds...

Jeff

Jeff Garzik

2006-04-01 19:58 UTC #33 lore raw

Re: [PATCH 14/15] libata: improve ata_bus_probe()

In reply to Tejun Heo

Quoted 7 lines · Tejun Heo

Improve ata_bus_probe() such that configuration failures are handled better. Each device is given ATA_PROBE_MAX_TRIES chances, but any non-transient error (revalidation failure with -ENODEV, configuration failure with -EINVAL...) disables the device directly. Any IO error results in SATA PHY speed down and ata_set_mode() failure lowers transfer mode. The last try always puts a device into PIO-0.

Quoted 5 lines

After each failure, the whole port is reset to make sure that the controller and all the devices are in a known and stable state. The reset also applies SATA SPD configuration if necessary.
Signed-off-by Tejun Heo <[email protected]>

seems sane at first glance

Jeff Garzik

2006-04-01 20:00 UTC #35 lore raw

Re: [PATCH 15/15] libata: consider disabled devices in ata_dev_xfermask()

In reply to Tejun Heo

Quoted 24 lines · Tejun Heo

ata_bus_probe() now marks failed devices properly and leaves meaningful transfer mode masks. This patch makes ata_dev_xfermask() consider disable devices when determining PIO mode to avoid violating device selection timing.
While at it, move port-wide resttriction out of device iteration loop and try to make the function look a bit prettier.
Signed-off-by Tejun Heo <[email protected]>
---
 drivers/scsi/libata-core.c |   34 +++++++++++++++++++++++-----------
 1 files changed, 23 insertions(+), 11 deletions(-)
523b496b7614981773c2c14259bbca4a7ba3a3e8
drivers/scsi/libata-core.c +0 -0
index ac2ca31..0d72792 100644
@@ -2902,23 +2902,34 @@ static void ata_dev_xfermask(struct ata_
 	unsigned long xfer_mask;
 	int i;
 

drivers/scsi/libata-core.c static void ata_dev_xfermask(struct ata_ · line 2912

Quoted 10 lines

drivers/scsi/libata-core.c +8 -2

-	xfer_mask = ata_pack_xfermask(ap->pio_mask, ap->mwdma_mask,
-				      ap->udma_mask);
+	xfer_mask = ata_pack_xfermask(ap->pio_mask,
+				      ap->mwdma_mask, ap->udma_mask);
+
+	/* Apply cable rule here.  Don't apply it early because when
+	 * we handle hot plug the cable type can itself change.
+	 */
+	if (ap->cbl == ATA_CBL_PATA40)
+		xfer_mask &= ~(0xF8 << ATA_SHIFT_UDMA);

ACK, though I wonder if moving the above code to its current position violates "early" described in the comment.

Jeff

Tejun Heo

author

2006-04-02 00:54 UTC #8 lore raw

Re: [PATCH 05/15] libata: implement ata_dev_enabled and disabled()

In reply to Jeff Garzik

Quoted 11 lines · Jeff Garzik

Quoted 7 lines · Tejun Heo
This patch renames ata_dev_present() to ata_dev_enabled() and adds ata_dev_disabled(). This is to discern the state where a device is present but disabled from not-present state. This disctinction is necessary when configuring transfer mode because device selection timing must not be violated even if a device fails to configure.
Signed-off-by Tejun Heo <[email protected]>
Tangent thought: perhaps instead of ata_dev_present() you could implement its antonym, ata_dev_absent(). It seems like a lot of the constructs are "!dev_present()" anyway.

Interesting. I'll bite that. :)

Signature · 1 line

tejun

Tejun Heo

author

2006-04-02 00:55 UTC #17 lore raw

Re: [PATCH 09/15] libata: implement ata_down_xfermask_limit()

In reply to Jeff Garzik

Quoted 6 lines · Jeff Garzik

Quoted 5 lines · Tejun Heo
Implement ata_down_xfermask_limit(). This function manipulates @dev->pio/mwdma/udma_mask such that the next lower transfer mode is selected. This will be used by ata_bus_probe() rewrite and later by EH speeding down.

Quoted 6 lines

Quoted 1 line
Signed-off-by Tejun Heo <[email protected]>
ACK, though I share Alan's concerns, even with your explanation. We'll see where this leads...
Not applied because it will cause a compile warning.

I'll write fuller description of synchronization model in the head message of new EH framework patchset. Sorry about the compile warning.

Signature · 1 line

tejun

Tejun Heo

author

2006-04-02 01:00 UTC #22 lore raw

Re: [PATCH 10/15] libata: add dev->sata_spd_limit and helpers

In reply to Jeff Garzik

Quoted 13 lines · Jeff Garzik

Quoted 12 lines · Tejun Heo
dev->sata_spd_limit contrains SATA PHY speed of the device and is initialized to the configured value on probing.
Currently, This is only valid for dev[0] but still put into ata_device instead of ata_port. This is because 1. it's device constraints (ie. when hotpluggin, it should be cleared with other device info)
2. for port multiplier, link actually belongs to each device.
Three helper functions - ata_down_sata_spd_limit(), ata_set_sata_spd_needed() and ata_set_sata_spd() - are implemented. They will be used by ata_bus_probe() rewrite and later by EH.

Quoted 8 lines

Quoted 1 line
Signed-off-by Tejun Heo <[email protected]>
NAK, I definitely don't want to closely associate phy and device parameters, because they are fundamentally two different things.
We program and address PHYs, so perhaps a sata_phy struct is needed.
Port multipliers also get interesting because we need to use ATA-ish commands to talk to the PM phys. Perhaps a struct ata_port_mult inside ata_device is warranted, where one stores an array of sata_phy structs, and other PM-specific details.

Hmmm... Yeah I also thought about extracting out PHY related information out such that PATAs share them, SATA and PM have their own maybe marked to indicate how they are connected. But that looked like going too far especially because we don't have PM support yet. So, sticking it into dev was sorta middle-ground.

How about putting it into ata_port around ap->cbl? This fits the current model better and should work the same.

Signature · 1 line

tejun

Tejun Heo

author

2006-04-02 01:05 UTC #28 lore raw

Re: [PATCH 12/15] libata: use SATA speeding down in ata_drive_probe_reset()

In reply to Jeff Garzik

Quoted 10 lines · Jeff Garzik

Quoted 6 lines · Tejun Heo
Make ata_drive_probe_reset() use SATA SPD configuration. Hardreset will be force if speed renegotiation is necessary. Also, if a hardreset fails, PHY speed is stepped down and hardreset is retried until the lowest speed is reached.
Signed-off-by Tejun Heo <[email protected]>
Tentative ACK: I wonder if a naked COMRESET is the best procedure for changing PHY speeds. We might even power down the phy, change speeds, and then power it up again. Need to reach my SATA docs, and maybe talk to some device/controller vendors.

AFAICS, SATA doc doesn't say anything about how to change PHY speed. FYI, ICH7 AHCI works fine with the current implementation.

I'm a bit skeptical about doing naked COMRESET from libata core layer. I think the right direction is to make SATA spd configuration the responsibility of hardreset operation.

Signature · 1 line

tejun

Tejun Heo

author

2006-04-02 01:07 UTC #31 lore raw

Re: [PATCH 13/15] libata: add 1s sleep between resets

In reply to Jeff Garzik

Quoted 23 lines · Jeff Garzik

Quoted 22 lines · Tejun Heo
Some devices react badly if resets are performed back-to-back. Give devices some time to breath.
Signed-off-by Tejun Heo <[email protected]>
---
 drivers/scsi/libata-core.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
e7f505c001fd4cb43b8123387285a7694790b4ae
drivers/scsi/libata-core.c +1 -0
index 30ee203..bd9ca3b 100644
@@ -2633,6 +2633,7 @@ int ata_drive_probe_reset(struct ata_por
         rc = ata_do_reset(ap, softreset, postreset, 0, classes);
         if (rc == 0 && classes[0] != ATA_DEV_UNKNOWN)
             goto done;
+        ssleep(1);
     }
 

Quoted 9 lines

drivers/scsi/libata-core.c int ata_drive_probe_reset(struct ata_por · line 2653
Quoted 6 lines
drivers/scsi/libata-core.c +1 -0
     if (!hardreset)
@@ -2649,6 +2650,7 @@ int ata_drive_probe_reset(struct ata_por
 
         if (ata_down_sata_spd_limit(ap, &ap->device[0]))
             goto done;
+        ssleep(1);
My gut says it should be at least 5 seconds...

Okay. And I'll add a little message such that the user staring at the console doesn't get bored during that 5 secs.

Signature · 1 line

tejun

Tejun Heo

author

2006-04-02 01:09 UTC #36 lore raw

Re: [PATCH 15/15] libata: consider disabled devices in ata_dev_xfermask()

In reply to Jeff Garzik

Quoted 25 lines · Jeff Garzik

Quoted 24 lines · Tejun Heo
ata_bus_probe() now marks failed devices properly and leaves meaningful transfer mode masks. This patch makes ata_dev_xfermask() consider disable devices when determining PIO mode to avoid violating device selection timing.
While at it, move port-wide resttriction out of device iteration loop and try to make the function look a bit prettier.
Signed-off-by Tejun Heo <[email protected]>
---
 drivers/scsi/libata-core.c |   34 +++++++++++++++++++++++-----------
 1 files changed, 23 insertions(+), 11 deletions(-)
523b496b7614981773c2c14259bbca4a7ba3a3e8
drivers/scsi/libata-core.c +0 -0
index ac2ca31..0d72792 100644
@@ -2902,23 +2902,34 @@ static void ata_dev_xfermask(struct ata_
     unsigned long xfer_mask;
     int i;
 

Quoted 13 lines

drivers/scsi/libata-core.c static void ata_dev_xfermask(struct ata_ · line 2912
Quoted 10 lines
drivers/scsi/libata-core.c +8 -2
-    xfer_mask = ata_pack_xfermask(ap->pio_mask, ap->mwdma_mask,
-                      ap->udma_mask);
+    xfer_mask = ata_pack_xfermask(ap->pio_mask,
+                      ap->mwdma_mask, ap->udma_mask);
+
+    /* Apply cable rule here.  Don't apply it early because when
+     * we handle hot plug the cable type can itself change.
+     */
+    if (ap->cbl == ATA_CBL_PATA40)
+        xfer_mask &= ~(0xF8 << ATA_SHIFT_UDMA);
ACK, though I wonder if moving the above code to its current position violates "early" described in the comment.

I think not. AFAICS the 'early' in the comment points ata_dev_xfermask() function itself.

Alan?

Signature · 1 line

tejun

Tejun Heo

author

2006-04-02 06:59 UTC #18 lore raw

Re: [PATCH 09/15] libata: implement ata_down_xfermask_limit()

In reply to Tejun Heo

Quoted 9 lines · Tejun Heo

Quoted 8 lines · Jeff Garzik
Quoted 6 lines · Tejun Heo
Implement ata_down_xfermask_limit(). This function manipulates @dev->pio/mwdma/udma_mask such that the next lower transfer mode is selected. This will be used by ata_bus_probe() rewrite and later by EH speeding down.
Signed-off-by Tejun Heo <[email protected]>

Quoted 9 lines

Quoted 5 lines
ACK, though I share Alan's concerns, even with your explanation. We'll see where this leads...
Not applied because it will cause a compile warning.
I'll write fuller description of synchronization model in the head message of new EH framework patchset. Sorry about the compile warning.

Jeff, the compile warning is about ata_down_xfermask_limit() being unused, which is intended. This patch implements the function. Later changes will use it. I'll re-post this patch as is in the next take.

Signature · 1 line

tejun

Jeff Garzik

2006-04-02 08:08 UTC #19 lore raw

Re: [PATCH 09/15] libata: implement ata_down_xfermask_limit()

Quoted 11 lines · Tejun Heo

Quoted 10 lines · Tejun Heo
Quoted 9 lines · Jeff Garzik
Quoted 6 lines · Tejun Heo
Implement ata_down_xfermask_limit(). This function manipulates @dev->pio/mwdma/udma_mask such that the next lower transfer mode is selected. This will be used by ata_bus_probe() rewrite and later by EH speeding down.
Signed-off-by Tejun Heo <[email protected]>
ACK, though I share Alan's concerns, even with your explanation. We'll see where this leads...

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
Not applied because it will cause a compile warning.
I'll write fuller description of synchronization model in the head message of new EH framework patchset. Sorry about the compile warning.
Jeff, the compile warning is about ata_down_xfermask_limit() being unused,

Correct. It just didn't get merged because I didn't merge any users.

Jeff

Jeff Garzik

2006-04-02 08:12 UTC #23 lore raw

Re: [PATCH 10/15] libata: add dev->sata_spd_limit and helpers

In reply to Tejun Heo

Quoted 11 lines · Tejun Heo

Quoted 6 lines · Jeff Garzik
NAK, I definitely don't want to closely associate phy and device parameters, because they are fundamentally two different things.
We program and address PHYs, so perhaps a sata_phy struct is needed.
Port multipliers also get interesting because we need to use ATA-ish commands to talk to the PM phys. Perhaps a struct ata_port_mult inside ata_device is warranted, where one stores an array of sata_phy structs, and other PM-specific details.
Hmmm... Yeah I also thought about extracting out PHY related information out such that PATAs share them, SATA and PM have their own maybe marked to indicate how they are connected. But that looked like going too far especially because we don't have PM support yet. So, sticking it into dev was sorta middle-ground.
How about putting it into ata_port around ap->cbl? This fits the current model better and should work the same.

Sounds good to me.

Jeff