Re: [PATCH 5/6] ide: add ata_dev_has_iordy() helper

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

 



Bartlomiej Zolnierkiewicz wrote:

* Use it in sl82c105 host driver so it always gets the correct info
 (use_iordy was incorrectly set for "pio" != 255 cases).

Hm, why incorrectly? ATA specs say PIO3/4 *must* use IORDY flow control, IIRC... :-/

   Moreover, it was me who added that! :-)

	use_iordy = (pio_mode > 2);

Index: b/drivers/ide/pci/sl82c105.c
===================================================================
--- a/drivers/ide/pci/sl82c105.c
+++ b/drivers/ide/pci/sl82c105.c
@@ -52,9 +52,10 @@
 * Convert a PIO mode and cycle time to the required on/off times
 * for the interface.  This has protection against runaway timings.
 */
-static unsigned int get_pio_timings(ide_pio_data_t *p)
+static unsigned int get_pio_timings(ide_drive_t *drive, ide_pio_data_t *p)
{
	unsigned int cmd_on, cmd_off;
+	u8 iordy = 0;

	cmd_on  = (ide_pio_timings[p->pio_mode].active_time + 29) / 30;
	cmd_off = (p->cycle_time - 30 * cmd_on + 29) / 30;
@@ -65,7 +66,10 @@ static unsigned int get_pio_timings(ide_
	if (cmd_off == 0)
		cmd_off = 1;

-	return (cmd_on - 1) << 8 | (cmd_off - 1) | (p->use_iordy ? 0x40 : 0x00);
+	if (p->pio_mode > 2 || ata_dev_has_iordy(drive->id))
+		iordy = 0x40;

This logic, although mimicking the old one from ide_get_best_pio_mode(), is not quite correct. As have been noted before, when you set a PIO mode using

It was actully correct enough, just superfluous -- it was me who was incorrect, mistaking || for &&. :-<

Set Transfer Mode subcode of the Set Features command, you're always selecting the flow control mode, i.e. using IORDY. So, the last condition in this if

So, what actually would need fixing in *all* the drivers if one was aiming at ATA-1 compatibility is *not* issuing that subcommand to such drives...

Oh yes, I keep forgetting about it - some nice FIXME comment
in <linux/ata.h> would be of a great help. :-)

Well, some drivers (like pdc202xx_*) don't do the IORDY thing right for PIO modes < 3 as well...

stmt should probably be the first, if not the sole one...

Fixed, new patch below.

[PATCH] ide: add ata_dev_has_iordy() helper (take 2)

* Add ata_dev_has_iordy() helper and use it sl82c105 host driver.

* Remove no longer needed ide_pio_data_t.use_iordy field.

v2:
* Fix issues noticed by Sergei:
  - correct patch description
  - remove stale comment from ide_get_best_pio_mode()

   I meant something like changing use_iordy to IORDY but it's good as is now...

  - use only ata_dev_has_iordy() in sl82c105 host driver

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>

Acked-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>

MBR, Sergei

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

[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