[PATCH] libata: IORDY handling

2007-11-19 Thread Alan Cox 
I believe this version meets all Sergei's objections

Correct the logic for when we issue a set features for transfer mode

- If the device has IORDY and the controller has IORDY - set the mode
- If the device has IORDY and the controller does not - turn IORDY off
- If neither has IORDY do nothing

Signed-off-by: Alan Cox <[EMAIL PROTECTED]>

diff -u --new-file --recursive --exclude-from /usr/src/exclude 
linux.vanilla-2.6.24-rc2-mm1/drivers/ata/libata-core.c 
linux-2.6.24-rc2-mm1/drivers/ata/libata-core.c
--- linux.vanilla-2.6.24-rc2-mm1/drivers/ata/libata-core.c  2007-11-16 
17:55:11.0 +
+++ linux-2.6.24-rc2-mm1/drivers/ata/libata-core.c  2007-11-16 
23:42:27.0 +
@@ -4392,7 +4400,14 @@
tf.feature = SETFEATURES_XFER;
tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE | ATA_TFLAG_POLLING;
tf.protocol = ATA_PROT_NODATA;
-   tf.nsect = dev->xfer_mode;
+   /* If we are using IORDY we must send the mode setting command */
+   if (ata_pio_need_iordy(dev))
+   tf.nsect = dev->xfer_mode;
+   /* If the device has IORDY and the controller does not - turn it off */
+   else if (ata_id_has_iordy(dev->id))
+   tf.nsect = 0x01;
+   else /* In the ancient relic department - skip all of this */
+   return 0;
 
err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0, 0);
 
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] libata: IORDY handling

2007-11-23 Thread Jeff Garzik 

Alan Cox wrote:

I believe this version meets all Sergei's objections

Correct the logic for when we issue a set features for transfer mode

- If the device has IORDY and the controller has IORDY - set the mode
- If the device has IORDY and the controller does not - turn IORDY off
- If neither has IORDY do nothing

Signed-off-by: Alan Cox <[EMAIL PROTECTED]>


applied #upstream


-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] libata: IORDY handling, quiet handling

2007-02-05 Thread Alan 
For further review

Turn on the IORDY handling logic.

If a controller doesnt support IORDY don't try and use it
If a device doesn't support IORDY don't try and use it
Otherwise use it whenever possible.
Always use IORDY for PIO3 and higher (it's mandatory)

Send the correct set_features command if operating without IORDY

Signed-off-by: Alan Cox <[EMAIL PROTECTED]>


diff -u --new-file --recursive --exclude-from /usr/src/exclude 
linux.vanilla-2.6.20-rc6-mm3/drivers/ata/libata-core.c 
linux-2.6.20-rc6-mm3/drivers/ata/libata-core.c
--- linux.vanilla-2.6.20-rc6-mm3/drivers/ata/libata-core.c  2007-01-31 
14:20:39.0 +
+++ linux-2.6.20-rc6-mm3/drivers/ata/libata-core.c  2007-02-01 
16:14:01.0 +
@@ -1404,30 +1446,44 @@
  * Check if the current speed of the device requires IORDY. Used
  * by various controllers for chip configuration.
  */
-
+ 
 unsigned int ata_pio_need_iordy(const struct ata_device *adev)
 {
-   int pio;
-   int speed = adev->pio_mode - XFER_PIO_0;
-
-   if (speed < 2)
+   /* Controller doesn't support  IORDY. Probably a pointless check
+  as the caller should know this */
+   if (adev->ap->flags & ATA_FLAG_NO_IORDY)
return 0;
-   if (speed > 2)
+   /* PIO3 and higher it is mandatory */
+   if (adev->pio_mode > XFER_PIO_2)
return 1;
+   /* We turn it on when possible */
+   if (ata_id_has_iordy(adev->id))
+   return 1;
+   return 0;
+}
 
+/**
+ * ata_pio_mask_no_iordy   -   Return the non IORDY mask
+ * @adev: ATA device
+ *
+ * Compute the highest mode possible if we are not using iordy. Return
+ * -1 if no iordy mode is available.
+ */
+ 
+static u32 ata_pio_mask_no_iordy(const struct ata_device *adev)
+{
/* If we have no drive specific rule, then PIO 2 is non IORDY */
-
if (adev->id[ATA_ID_FIELD_VALID] & 2) { /* EIDE */
-   pio = adev->id[ATA_ID_EIDE_PIO];
+   u16 pio = adev->id[ATA_ID_EIDE_PIO];
/* Is the speed faster than the drive allows non IORDY ? */
if (pio) {
/* This is cycle times not frequency - watch the logic! 
*/
if (pio > 240)  /* PIO2 is 240nS per cycle */
-   return 1;
-   return 0;
+   return 3 << ATA_SHIFT_PIO;
+   return 7 << ATA_SHIFT_PIO;
}
}
-   return 0;
+   return 3 << ATA_SHIFT_PIO;
 }
 
 /**
@@ -3410,6 +3470,9 @@
   dev->mwdma_mask, dev->udma_mask);
xfer_mask &= ata_id_xfermask(dev->id);
 
+   if (ap->flags & ATA_FLAG_NO_IORDY)
+   xfer_mask &= ata_pio_mask_no_iordy(dev);
+
/*
 *  CFA Advanced TrueIDE timings are not allowed on a shared
 *  cable
@@ -3466,8 +3529,18 @@
tf.command = ATA_CMD_SET_FEATURES;
tf.feature = SETFEATURES_XFER;
tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+
+   /* Older CFA may not support this command */
+   if (ata_id_is_cfa(dev->id))
+   tf.flags |= ATA_TFLAG_QUIET;
+
tf.protocol = ATA_PROT_NODATA;
-   tf.nsect = dev->xfer_mode;
+
+   /* Ancient devices may need us to avoid IORDY */
+   if (ata_pio_need_iordy(dev))
+   tf.nsect = dev->xfer_mode;
+   else
+   tf.nsect = 0x01;
 
err_mask = ata_exec_internal(dev, &tf, NULL, DMA_NONE, NULL, 0);
 
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] libata: IORDY handling, quiet handling

2007-02-05 Thread Sergei Shtylyov 

Alan wrote:

For further review



Turn on the IORDY handling logic.



If a controller doesnt support IORDY don't try and use it



If a device doesn't support IORDY don't try and use it


   Looks like we shouldn't change it default PIO mode at all in this case. 
See below why...



Otherwise use it whenever possible.
Always use IORDY for PIO3 and higher (it's mandatory)



Send the correct set_features command if operating without IORDY



Signed-off-by: Alan Cox <[EMAIL PROTECTED]>


diff -u --new-file --recursive --exclude-from /usr/src/exclude 
linux.vanilla-2.6.20-rc6-mm3/drivers/ata/libata-core.c 
linux-2.6.20-rc6-mm3/drivers/ata/libata-core.c
--- linux.vanilla-2.6.20-rc6-mm3/drivers/ata/libata-core.c  2007-01-31 
14:20:39.0 +
+++ linux-2.6.20-rc6-mm3/drivers/ata/libata-core.c  2007-02-01 
16:14:01.0 +
@@ -1404,30 +1446,44 @@
  * Check if the current speed of the device requires IORDY. Used
  * by various controllers for chip configuration.
  */
-
+ 
 unsigned int ata_pio_need_iordy(const struct ata_device *adev)

 {
-   int pio;
-   int speed = adev->pio_mode - XFER_PIO_0;
-
-   if (speed < 2)
+   /* Controller doesn't support  IORDY. Probably a pointless check
+  as the caller should know this */
+   if (adev->ap->flags & ATA_FLAG_NO_IORDY)
return 0;
-   if (speed > 2)
+   /* PIO3 and higher it is mandatory */
+   if (adev->pio_mode > XFER_PIO_2)
return 1;
+   /* We turn it on when possible */
+   if (ata_id_has_iordy(adev->id))
+   return 1;
+   return 0;
+}
 
+/**

+ * ata_pio_mask_no_iordy   -   Return the non IORDY mask
+ * @adev: ATA device
+ *
+ * Compute the highest mode possible if we are not using iordy. Return
+ * -1 if no iordy mode is available.
+ */
+ 
+static u32 ata_pio_mask_no_iordy(const struct ata_device *adev)

+{
/* If we have no drive specific rule, then PIO 2 is non IORDY */
-
if (adev->id[ATA_ID_FIELD_VALID] & 2) {  /* EIDE */
-   pio = adev->id[ATA_ID_EIDE_PIO];
+   u16 pio = adev->id[ATA_ID_EIDE_PIO];
/* Is the speed faster than the drive allows non IORDY ? */
if (pio) {
/* This is cycle times not frequency - watch the logic! 
*/
if (pio > 240)   /* PIO2 is 240nS per cycle */
-   return 1;
-   return 0;
+   return 3 << ATA_SHIFT_PIO;
+   return 7 << ATA_SHIFT_PIO;
}
}
-   return 0;
+   return 3 << ATA_SHIFT_PIO;
 }


   Well, after looking into ATA-1, it seems that the logic may have been and 
may be still is somewhat wrong here.  There was PIO2 already defined but no 
EIDE fields yet (and yet optional IORDY line already here).  We probably 
should always consider PIO2 non-IORDY indeed (unless told not to by the word 
67) but go figure... :-)



@@ -3466,8 +3529,18 @@
tf.command = ATA_CMD_SET_FEATURES;
tf.feature = SETFEATURES_XFER;
tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+
+   /* Older CFA may not support this command */
+   if (ata_id_is_cfa(dev->id))
+   tf.flags |= ATA_TFLAG_QUIET;
+
tf.protocol = ATA_PROT_NODATA;
-   tf.nsect = dev->xfer_mode;
+
+   /* Ancient devices may need us to avoid IORDY */
+   if (ata_pio_need_iordy(dev))
+   tf.nsect = dev->xfer_mode;
+   else
+   tf.nsect = 0x01;


   Note that ATA-1 did *not* yet define this subcode (only 0 for "block 
transfer"). I think that we should not change the PIO modes at all on 
non-IORDY drives, otherwise it's becoming pointless and messy -- you're not 
setting what you're told here and force the drive's default mode instead... 
why then change it at all?


MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] libata: IORDY handling, quiet handling

2007-02-05 Thread Sergei Shtylyov 

Hello, I wrote:


@@ -3466,8 +3529,18 @@
 tf.command = ATA_CMD_SET_FEATURES;
 tf.feature = SETFEATURES_XFER;
 tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+
+/* Older CFA may not support this command */
+if (ata_id_is_cfa(dev->id))
+tf.flags |= ATA_TFLAG_QUIET;
+
 tf.protocol = ATA_PROT_NODATA;
-tf.nsect = dev->xfer_mode;
+
+/* Ancient devices may need us to avoid IORDY */
+if (ata_pio_need_iordy(dev))
+tf.nsect = dev->xfer_mode;
+else
+tf.nsect = 0x01;


   Note that ATA-1 did *not* yet define this subcode (only 0 for "block 
transfer"). I think that we should not change the PIO modes at all on 
non-IORDY drives, otherwise it's becoming pointless and messy -- you're


   After thinking a bit more: we should not do it on non-EIDE drives 
(however, this is probably the same thing).
   If our host by some mischance has no IORDY support and an EIDE drive has 
it and supports disabling (otherwise there's little we can do), we must issue 
subcode 0x01 and tune our host controller to the default PIO mode.


not setting what you're told here and force the drive's default mode 
instead... why then change it at all?


   Anyway, all this logic should be one layer higher than this function...

MBR, Sergei
-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] libata: IORDY handling, quiet handling

2007-02-23 Thread Jeff Garzik 

Alan wrote:

For further review

Turn on the IORDY handling logic.

If a controller doesnt support IORDY don't try and use it
If a device doesn't support IORDY don't try and use it
Otherwise use it whenever possible.
Always use IORDY for PIO3 and higher (it's mandatory)

Send the correct set_features command if operating without IORDY

Signed-off-by: Alan Cox <[EMAIL PROTECTED]>


modulo TFLAG_QUIET mentioned in previous email, this seems OK to me.


-
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html