[PATCH #upstream 1/2] libata: relocate forcing PIO0 on reset

[PATCH #upstream 1/2] libata: relocate forcing PIO0 on reset

[Tejun Heo]

Forcing PIO0 on reset was done inside ata_bus_softreset(), which is a
bit out of place as it should be applied to all resets - hard, soft
and implementation which don't use ata_bus_softreset().  Relocate it
such that...

* For new EH, it's done in ata_eh_reset() before calling prereset.

* For old EH, it's done before calling ap->ops->phy_reset() in
  ata_bus_probe().

This makes PIO0 forced after all resets.  Another difference is that
reset itself is done after PIO0 is forced.

Signed-off-by: Tejun Heo <htejun@gmail.com>
Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
---
 drivers/ata/libata-core.c |   46 +++++++++++++++++++---------------------------
 drivers/ata/libata-eh.c   |   19 +++++++++++++++++++
 2 files changed, 38 insertions(+), 27 deletions(-)

Index: work/drivers/ata/libata-core.c
===================================================================
--- work.orig/drivers/ata/libata-core.c
+++ work/drivers/ata/libata-core.c
@@ -2219,6 +2219,25 @@ int ata_bus_probe(struct ata_port *ap)
 		tries[dev->devno] = ATA_PROBE_MAX_TRIES;
 
  retry:
+	ata_link_for_each_dev(dev, &ap->link) {
+		/* If we issue an SRST then an ATA drive (not ATAPI)
+		 * may change configuration and be in PIO0 timing. If
+		 * we do a hard reset (or are coming from power on)
+		 * this is true for ATA or ATAPI. Until we've set a
+		 * suitable controller mode we should not touch the
+		 * bus as we may be talking too fast.
+		 */
+		dev->pio_mode = XFER_PIO_0;
+
+		/* If the controller has a pio mode setup function
+		 * then use it to set the chipset to rights. Don't
+		 * touch the DMA setup as that will be dealt with when
+		 * configuring devices.
+		 */
+		if (ap->ops->set_piomode)
+			ap->ops->set_piomode(ap, dev);
+	}
+
 	/* reset and determine device classes */
 	ap->ops->phy_reset(ap);
 
@@ -2234,12 +2253,6 @@ int ata_bus_probe(struct ata_port *ap)
 
 	ata_port_probe(ap);
 
-	/* after the reset the device state is PIO 0 and the controller
-	   state is undefined. Record the mode */
-
-	ata_link_for_each_dev(dev, &ap->link)
-		dev->pio_mode = XFER_PIO_0;
-
 	/* read IDENTIFY page and configure devices. We have to do the identify
 	   specific sequence bass-ackwards so that PDIAG- is released by
 	   the slave device */
@@ -3272,8 +3285,6 @@ static int ata_bus_softreset(struct ata_
 			     unsigned long deadline)
 {
 	struct ata_ioports *ioaddr = &ap->ioaddr;
-	struct ata_device *dev;
-	int i = 0;
 
 	DPRINTK("ata%u: bus reset via SRST\n", ap->print_id);
 
@@ -3284,25 +3295,6 @@ static int ata_bus_softreset(struct ata_
 	udelay(20);	/* FIXME: flush */
 	iowrite8(ap->ctl, ioaddr->ctl_addr);
 
-	/* If we issued an SRST then an ATA drive (not ATAPI)
-	 * may have changed configuration and be in PIO0 timing. If
-	 * we did a hard reset (or are coming from power on) this is
-	 * true for ATA or ATAPI. Until we've set a suitable controller
-	 * mode we should not touch the bus as we may be talking too fast.
-	 */
-
-	ata_link_for_each_dev(dev, &ap->link)
-		dev->pio_mode = XFER_PIO_0;
-
-	/* If the controller has a pio mode setup function then use
-	   it to set the chipset to rights. Don't touch the DMA setup
-	   as that will be dealt with when revalidating */
-	if (ap->ops->set_piomode) {
-		ata_link_for_each_dev(dev, &ap->link)
-			if (devmask & (1 << i++))
-				ap->ops->set_piomode(ap, dev);
-	}
-
 	/* wait a while before checking status */
 	ata_wait_after_reset(ap, deadline);
 
Index: work/drivers/ata/libata-eh.c
===================================================================
--- work.orig/drivers/ata/libata-eh.c
+++ work/drivers/ata/libata-eh.c
@@ -2083,6 +2083,25 @@ int ata_eh_reset(struct ata_link *link, 
 
 	ata_eh_about_to_do(link, NULL, ehc->i.action & ATA_EH_RESET_MASK);
 
+	ata_link_for_each_dev(dev, link) {
+		/* If we issue an SRST then an ATA drive (not ATAPI)
+		 * may change configuration and be in PIO0 timing. If
+		 * we do a hard reset (or are coming from power on)
+		 * this is true for ATA or ATAPI. Until we've set a
+		 * suitable controller mode we should not touch the
+		 * bus as we may be talking too fast.
+		 */
+		dev->pio_mode = XFER_PIO_0;
+
+		/* If the controller has a pio mode setup function
+		 * then use it to set the chipset to rights. Don't
+		 * touch the DMA setup as that will be dealt with when
+		 * configuring devices.
+		 */
+		if (ap->ops->set_piomode)
+			ap->ops->set_piomode(ap, dev);
+	}
+
 	/* Determine which reset to use and record in ehc->i.action.
 	 * prereset() may examine and modify it.
 	 */

[PATCH #upstream 2/2] libata: no need to speed down if already at PIO0

[Tejun Heo]

After reset, transfer mode is always PIO0 regardless of
dev->xfer_mask.  Check dev->pio_mode before trying to slow down after
configuration failure.  This prevents bogus speed down before device
is actually configured.

Signed-off-by: Tejun Heo <htejun@gmail.com>
Cc: Alan Cox <lan@lxorguk.ukuu.org.uk>
---
 drivers/ata/libata-eh.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: work/drivers/ata/libata-eh.c
===================================================================
--- work.orig/drivers/ata/libata-eh.c
+++ work/drivers/ata/libata-eh.c
@@ -2437,7 +2437,7 @@ static int ata_eh_handle_dev_fail(struct
 		/* give it just one more chance */
 		ehc->tries[dev->devno] = min(ehc->tries[dev->devno], 1);
 	case -EIO:
-		if (ehc->tries[dev->devno] == 1) {
+		if (ehc->tries[dev->devno] == 1 && dev->pio_mode > XFER_PIO_0) {
 			/* This is the last chance, better to slow
 			 * down than lose it.
 			 */

Re: [PATCH #upstream 2/2] libata: no need to speed down if already at PIO0

[Alan Cox]

On Mon, 29 Oct 2007 16:45:05 +0900
Tejun Heo <htejun@gmail.com> wrote:

> After reset, transfer mode is always PIO0 regardless of
> dev->xfer_mask.  Check dev->pio_mode before trying to slow down after
> configuration failure.  This prevents bogus speed down before device
> is actually configured.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> Cc: Alan Cox <lan@lxorguk.ukuu.org.uk>

Both changesets

Acked-by: Alan Cox <alan@redhat.com>

Re: [PATCH #upstream 1/2] libata: relocate forcing PIO0 on reset

[Jeff Garzik]

Tejun Heo wrote:
> Forcing PIO0 on reset was done inside ata_bus_softreset(), which is a
> bit out of place as it should be applied to all resets - hard, soft
> and implementation which don't use ata_bus_softreset().  Relocate it
> such that...
> 
> * For new EH, it's done in ata_eh_reset() before calling prereset.
> 
> * For old EH, it's done before calling ap->ops->phy_reset() in
>   ata_bus_probe().
> 
> This makes PIO0 forced after all resets.  Another difference is that
> reset itself is done after PIO0 is forced.
> 
> Signed-off-by: Tejun Heo <htejun@gmail.com>
> Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>

applied 1-2 to #upstream-fixes, after renaming existing #upstream to 
#upstream-fixes (thereby assuring those previous changesets will go 
upstream sooner)

Re: [PATCH #upstream 1/2] libata: relocate forcing PIO0 on reset

[Chuck Ebbert]

On 10/29/2007 03:41 AM, Tejun Heo wrote:
>   retry:
> +	ata_link_for_each_dev(dev, &ap->link) {

How do I backport these loops to 2.6.23?

I see something like this in the old code:

	for (i=0; i < ATA_MAX_DEVICES; i++) {
		dev = &ap->device[i];
                if (!ata_dev_enabled(dev))
                        continue;
		do_something(dev);
	}

Re: [PATCH #upstream 1/2] libata: relocate forcing PIO0 on reset

[Tejun Heo]

Chuck Ebbert wrote:
> On 10/29/2007 03:41 AM, Tejun Heo wrote:
>>   retry:
>> +	ata_link_for_each_dev(dev, &ap->link) {
> 
> How do I backport these loops to 2.6.23?
> 
> I see something like this in the old code:
> 
> 	for (i=0; i < ATA_MAX_DEVICES; i++) {
> 		dev = &ap->device[i];
>                 if (!ata_dev_enabled(dev))
>                         continue;
> 		do_something(dev);
> 	}

Yeap, that's pretty much it.

-- 
tejun