linux-ide.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 5/6] ide: add ata_dev_has_iordy() helper
@ 2007-06-23 18:05 Bartlomiej Zolnierkiewicz
  2007-06-23 19:36 ` Sergei Shtylyov
  0 siblings, 1 reply; 10+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-06-23 18:05 UTC (permalink / raw)
  To: linux-ide


* Add ata_dev_has_iordy() helper.

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

* Remove no longer needed ide_pio_data_t.use_iordy field.

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
 drivers/ide/ide-lib.c      |    5 -----
 drivers/ide/pci/sl82c105.c |   10 +++++++---
 include/linux/ide.h        |    6 +++++-
 3 files changed, 12 insertions(+), 9 deletions(-)

Index: b/drivers/ide/ide-lib.c
===================================================================
--- a/drivers/ide/ide-lib.c
+++ b/drivers/ide/ide-lib.c
@@ -267,18 +267,15 @@ u8 ide_get_best_pio_mode (ide_drive_t *d
 {
 	int pio_mode;
 	int cycle_time = 0;
-	int use_iordy = 0;
 	struct hd_driveid* id = drive->id;
 	int overridden  = 0;
 
 	if (mode_wanted != 255) {
 		pio_mode = mode_wanted;
-		use_iordy = (pio_mode > 2);
 	} else if (!drive->id) {
 		pio_mode = 0;
 	} else if ((pio_mode = ide_scan_pio_blacklist(id->model)) != -1) {
 		printk(KERN_INFO "%s: is on PIO blacklist\n", drive->name);
-		use_iordy = (pio_mode > 2);
 	} else {
 		pio_mode = id->tPIO;
 		if (pio_mode > 2) {	/* 2 is maximum allowed tPIO value */
@@ -287,7 +284,6 @@ u8 ide_get_best_pio_mode (ide_drive_t *d
 		}
 		if (id->field_valid & 2) {	  /* drive implements ATA2? */
 			if (id->capability & 8) { /* drive supports use_iordy? */
-				use_iordy = 1;
 				cycle_time = id->eide_pio_iordy;
 				if (id->eide_pio_modes & 7) {
 					overridden = 0;
@@ -325,7 +321,6 @@ u8 ide_get_best_pio_mode (ide_drive_t *d
 	if (d) {
 		d->pio_mode = pio_mode;
 		d->cycle_time = cycle_time ? cycle_time : ide_pio_timings[pio_mode].cycle_time;
-		d->use_iordy = use_iordy;
 	}
 	return pio_mode;
 }
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;
+
+	return (cmd_on - 1) << 8 | (cmd_off - 1) | iordy;
 }
 
 /*
@@ -82,7 +86,7 @@ static u8 sl82c105_tune_pio(ide_drive_t 
 
 	pio = ide_get_best_pio_mode(drive, pio, 5, &p);
 
-	drv_ctrl = get_pio_timings(&p);
+	drv_ctrl = get_pio_timings(drive, &p);
 
 	/*
 	 * Store the PIO timings so that we can restore them
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1363,6 +1363,11 @@ extern void ide_toggle_bounce(ide_drive_
 extern int ide_set_xfer_rate(ide_drive_t *drive, u8 rate);
 int ide_use_fast_pio(ide_drive_t *);
 
+static inline int ata_dev_has_iordy(struct hd_driveid *id)
+{
+	return ((id->field_valid & 2) && (id->capability & 8)) ? 1 : 0;
+}
+
 u8 ide_dump_status(ide_drive_t *, const char *, u8);
 
 typedef struct ide_pio_timings_s {
@@ -1374,7 +1379,6 @@ typedef struct ide_pio_timings_s {
 
 typedef struct ide_pio_data_s {
 	u8 pio_mode;
-	u8 use_iordy;
 	unsigned int cycle_time;
 } ide_pio_data_t;
 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 5/6] ide: add ata_dev_has_iordy() helper
  2007-06-23 18:05 [PATCH 5/6] ide: add ata_dev_has_iordy() helper Bartlomiej Zolnierkiewicz
@ 2007-06-23 19:36 ` Sergei Shtylyov
  2007-06-23 22:09   ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 10+ messages in thread
From: Sergei Shtylyov @ 2007-06-23 19:36 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide

Bartlomiej Zolnierkiewicz wrote:

    This one is actually arguable...

> * Add ata_dev_has_iordy() helper.

    I thought IDE subsys make use of only 'ide_' prefixes. :-)

> * 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... :-/
    Maybe you meant the mode is being later adjusted by max_mode thereshold -- 
well, that's a common issue in ide_get_best_pio_mode().

> * Remove no longer needed ide_pio_data_t.use_iordy field.

> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

> Index: b/drivers/ide/ide-lib.c
> ===================================================================
> --- a/drivers/ide/ide-lib.c
> +++ b/drivers/ide/ide-lib.c
> @@ -267,18 +267,15 @@ u8 ide_get_best_pio_mode (ide_drive_t *d
>  {
>  	int pio_mode;
>  	int cycle_time = 0;
> -	int use_iordy = 0;
>  	struct hd_driveid* id = drive->id;
>  	int overridden  = 0;
>  
>  	if (mode_wanted != 255) {
>  		pio_mode = mode_wanted;
> -		use_iordy = (pio_mode > 2);
>  	} else if (!drive->id) {
>  		pio_mode = 0;
>  	} else if ((pio_mode = ide_scan_pio_blacklist(id->model)) != -1) {
>  		printk(KERN_INFO "%s: is on PIO blacklist\n", drive->name);
> -		use_iordy = (pio_mode > 2);
>  	} else {
>  		pio_mode = id->tPIO;
>  		if (pio_mode > 2) {	/* 2 is maximum allowed tPIO value */
> @@ -287,7 +284,6 @@ u8 ide_get_best_pio_mode (ide_drive_t *d
>  		}
>  		if (id->field_valid & 2) {	  /* drive implements ATA2? */
>  			if (id->capability & 8) { /* drive supports use_iordy? */

    The comment above needs fixing too -- kill reference to use_iordy.

> -				use_iordy = 1;
>  				cycle_time = id->eide_pio_iordy;
>  				if (id->eide_pio_modes & 7) {
>  					overridden = 0;
> @@ -325,7 +321,6 @@ u8 ide_get_best_pio_mode (ide_drive_t *d
>  	if (d) {
>  		d->pio_mode = pio_mode;
>  		d->cycle_time = cycle_time ? cycle_time : ide_pio_timings[pio_mode].cycle_time;
> -		d->use_iordy = use_iordy;
>  	}
>  	return pio_mode;
>  }
> 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 
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 
stmt should probably be the first, if not the sole one...

> +	return (cmd_on - 1) << 8 | (cmd_off - 1) | iordy;
>  }
>  
>  /*
> @@ -82,7 +86,7 @@ static u8 sl82c105_tune_pio(ide_drive_t 
>  
>  	pio = ide_get_best_pio_mode(drive, pio, 5, &p);
>  
> -	drv_ctrl = get_pio_timings(&p);
> +	drv_ctrl = get_pio_timings(drive, &p);
>  
>  	/*
>  	 * Store the PIO timings so that we can restore them
> Index: b/include/linux/ide.h
> ===================================================================
> --- a/include/linux/ide.h
> +++ b/include/linux/ide.h
> @@ -1363,6 +1363,11 @@ extern void ide_toggle_bounce(ide_drive_
>  extern int ide_set_xfer_rate(ide_drive_t *drive, u8 rate);
>  int ide_use_fast_pio(ide_drive_t *);
>  
> +static inline int ata_dev_has_iordy(struct hd_driveid *id)
> +{
> +	return ((id->field_valid & 2) && (id->capability & 8)) ? 1 : 0;
> +}
> +
>  u8 ide_dump_status(ide_drive_t *, const char *, u8);
>  
>  typedef struct ide_pio_timings_s {
> @@ -1374,7 +1379,6 @@ typedef struct ide_pio_timings_s {
>  
>  typedef struct ide_pio_data_s {
>  	u8 pio_mode;
> -	u8 use_iordy;
>  	unsigned int cycle_time;
>  } ide_pio_data_t;
>  

MBR, Sergei

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 5/6] ide: add ata_dev_has_iordy() helper
  2007-06-23 19:36 ` Sergei Shtylyov
@ 2007-06-23 22:09   ` Bartlomiej Zolnierkiewicz
  2007-06-24 16:54     ` Sergei Shtylyov
  0 siblings, 1 reply; 10+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-06-23 22:09 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide

On Saturday 23 June 2007, Sergei Shtylyov wrote:
> Bartlomiej Zolnierkiewicz wrote:
> 
>     This one is actually arguable...
> 
> > * Add ata_dev_has_iordy() helper.
> 
>     I thought IDE subsys make use of only 'ide_' prefixes. :-)

Nope, see ide-probe.c. ;-)

> > * 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... :-/
>     Maybe you meant the mode is being later adjusted by max_mode thereshold -- 
> well, that's a common issue in ide_get_best_pio_mode().

Well, yes but as I think about it now it is non-issue
(all ide_get_best_pio_mode() users support PIO3).

Fixed.

> > * Remove no longer needed ide_pio_data_t.use_iordy field.
> 
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> 
> > Index: b/drivers/ide/ide-lib.c
> > ===================================================================
> > --- a/drivers/ide/ide-lib.c
> > +++ b/drivers/ide/ide-lib.c
> > @@ -267,18 +267,15 @@ u8 ide_get_best_pio_mode (ide_drive_t *d
> >  {
> >  	int pio_mode;
> >  	int cycle_time = 0;
> > -	int use_iordy = 0;
> >  	struct hd_driveid* id = drive->id;
> >  	int overridden  = 0;
> >  
> >  	if (mode_wanted != 255) {
> >  		pio_mode = mode_wanted;
> > -		use_iordy = (pio_mode > 2);
> >  	} else if (!drive->id) {
> >  		pio_mode = 0;
> >  	} else if ((pio_mode = ide_scan_pio_blacklist(id->model)) != -1) {
> >  		printk(KERN_INFO "%s: is on PIO blacklist\n", drive->name);
> > -		use_iordy = (pio_mode > 2);
> >  	} else {
> >  		pio_mode = id->tPIO;
> >  		if (pio_mode > 2) {	/* 2 is maximum allowed tPIO value */
> > @@ -287,7 +284,6 @@ u8 ide_get_best_pio_mode (ide_drive_t *d
> >  		}
> >  		if (id->field_valid & 2) {	  /* drive implements ATA2? */
> >  			if (id->capability & 8) { /* drive supports use_iordy? */
> 
>     The comment above needs fixing too -- kill reference to use_iordy.

Done.

> > -				use_iordy = 1;
> >  				cycle_time = id->eide_pio_iordy;
> >  				if (id->eide_pio_modes & 7) {
> >  					overridden = 0;
> > @@ -325,7 +321,6 @@ u8 ide_get_best_pio_mode (ide_drive_t *d
> >  	if (d) {
> >  		d->pio_mode = pio_mode;
> >  		d->cycle_time = cycle_time ? cycle_time : ide_pio_timings[pio_mode].cycle_time;
> > -		d->use_iordy = use_iordy;
> >  	}
> >  	return pio_mode;
> >  }
> > 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 
> 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 

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

> 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()
  - use only ata_dev_has_iordy() in sl82c105 host driver

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Reviewed-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
---
 drivers/ide/ide-lib.c      |    7 +------
 drivers/ide/pci/sl82c105.c |   10 +++++++---
 include/linux/ide.h        |    6 +++++-
 3 files changed, 13 insertions(+), 10 deletions(-)

Index: b/drivers/ide/ide-lib.c
===================================================================
--- a/drivers/ide/ide-lib.c
+++ b/drivers/ide/ide-lib.c
@@ -267,18 +267,15 @@ u8 ide_get_best_pio_mode (ide_drive_t *d
 {
 	int pio_mode;
 	int cycle_time = 0;
-	int use_iordy = 0;
 	struct hd_driveid* id = drive->id;
 	int overridden  = 0;
 
 	if (mode_wanted != 255) {
 		pio_mode = mode_wanted;
-		use_iordy = (pio_mode > 2);
 	} else if (!drive->id) {
 		pio_mode = 0;
 	} else if ((pio_mode = ide_scan_pio_blacklist(id->model)) != -1) {
 		printk(KERN_INFO "%s: is on PIO blacklist\n", drive->name);
-		use_iordy = (pio_mode > 2);
 	} else {
 		pio_mode = id->tPIO;
 		if (pio_mode > 2) {	/* 2 is maximum allowed tPIO value */
@@ -286,8 +283,7 @@ u8 ide_get_best_pio_mode (ide_drive_t *d
 			overridden = 1;
 		}
 		if (id->field_valid & 2) {	  /* drive implements ATA2? */
-			if (id->capability & 8) { /* drive supports use_iordy? */
-				use_iordy = 1;
+			if (id->capability & 8) {
 				cycle_time = id->eide_pio_iordy;
 				if (id->eide_pio_modes & 7) {
 					overridden = 0;
@@ -325,7 +321,6 @@ u8 ide_get_best_pio_mode (ide_drive_t *d
 	if (d) {
 		d->pio_mode = pio_mode;
 		d->cycle_time = cycle_time ? cycle_time : ide_pio_timings[pio_mode].cycle_time;
-		d->use_iordy = use_iordy;
 	}
 	return pio_mode;
 }
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 (ata_dev_has_iordy(drive->id))
+		iordy = 0x40;
+
+	return (cmd_on - 1) << 8 | (cmd_off - 1) | iordy;
 }
 
 /*
@@ -82,7 +86,7 @@ static u8 sl82c105_tune_pio(ide_drive_t 
 
 	pio = ide_get_best_pio_mode(drive, pio, 5, &p);
 
-	drv_ctrl = get_pio_timings(&p);
+	drv_ctrl = get_pio_timings(drive, &p);
 
 	/*
 	 * Store the PIO timings so that we can restore them
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1363,6 +1363,11 @@ extern void ide_toggle_bounce(ide_drive_
 extern int ide_set_xfer_rate(ide_drive_t *drive, u8 rate);
 int ide_use_fast_pio(ide_drive_t *);
 
+static inline int ata_dev_has_iordy(struct hd_driveid *id)
+{
+	return ((id->field_valid & 2) && (id->capability & 8)) ? 1 : 0;
+}
+
 u8 ide_dump_status(ide_drive_t *, const char *, u8);
 
 typedef struct ide_pio_timings_s {
@@ -1374,7 +1379,6 @@ typedef struct ide_pio_timings_s {
 
 typedef struct ide_pio_data_s {
 	u8 pio_mode;
-	u8 use_iordy;
 	unsigned int cycle_time;
 } ide_pio_data_t;
 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 5/6] ide: add ata_dev_has_iordy() helper
  2007-06-23 22:09   ` Bartlomiej Zolnierkiewicz
@ 2007-06-24 16:54     ` Sergei Shtylyov
  2007-06-27 19:02       ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 10+ messages in thread
From: Sergei Shtylyov @ 2007-06-24 16:54 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide

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@gmail.com>

Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

MBR, Sergei


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 5/6] ide: add ata_dev_has_iordy() helper
  2007-06-24 16:54     ` Sergei Shtylyov
@ 2007-06-27 19:02       ` Bartlomiej Zolnierkiewicz
  2007-06-27 20:28         ` Sergei Shtylyov
  0 siblings, 1 reply; 10+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-06-27 19:02 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide


Hello,

On Sunday 24 June 2007, Sergei Shtylyov wrote:

> >>>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 &&. :-<

After checking with ATA-2 spec I would prefer to leave extra p->pio_mode > 2
check (if ata_dev_has_iordy() fails device still _may_ support IORDY).

Fixed in "take 3" of the patch.

> >>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...

I was actually thinking about a different way of fixing this:

- remove 0x08 bit from XFER_PIO_[0,6] defines and add new XFER_PIO_IORDY
  define (<linux/ata.h>)

- check for speed == XFER_PIO_[0,6] in ide-lib.c::ide_config_drive_speed()
  and pmac.c::pmac_ide_do_setfeature(), add XFER_PIO_IORDY if needed

This should be done together with fixing these host drivers that don't
handle IORDY properly.

> > 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...

Added to the existing IDE TODO at

	http://kernel.org/pub/linux/kernel/people/bart/pata-2.6/TODO

Patches adding/removing items are welcomed.

Patches fixing actual issues are welcomed even more.

> >>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...

Corrected in "take 3".

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

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

* Remove no longer needed ide_pio_data_t.use_iordy field.

v2/v3:
* Fix issues noticed by Sergei:
  - correct patch description
  - fix comment in ide_get_best_pio_mode()

Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>
---
 drivers/ide/ide-lib.c      |    7 +------
 drivers/ide/pci/sl82c105.c |   10 +++++++---
 include/linux/ide.h        |    6 +++++-
 3 files changed, 13 insertions(+), 10 deletions(-)

Index: b/drivers/ide/ide-lib.c
===================================================================
--- a/drivers/ide/ide-lib.c
+++ b/drivers/ide/ide-lib.c
@@ -267,18 +267,15 @@ u8 ide_get_best_pio_mode (ide_drive_t *d
 {
 	int pio_mode;
 	int cycle_time = 0;
-	int use_iordy = 0;
 	struct hd_driveid* id = drive->id;
 	int overridden  = 0;
 
 	if (mode_wanted != 255) {
 		pio_mode = mode_wanted;
-		use_iordy = (pio_mode > 2);
 	} else if (!drive->id) {
 		pio_mode = 0;
 	} else if ((pio_mode = ide_scan_pio_blacklist(id->model)) != -1) {
 		printk(KERN_INFO "%s: is on PIO blacklist\n", drive->name);
-		use_iordy = (pio_mode > 2);
 	} else {
 		pio_mode = id->tPIO;
 		if (pio_mode > 2) {	/* 2 is maximum allowed tPIO value */
@@ -286,8 +283,7 @@ u8 ide_get_best_pio_mode (ide_drive_t *d
 			overridden = 1;
 		}
 		if (id->field_valid & 2) {	  /* drive implements ATA2? */
-			if (id->capability & 8) { /* drive supports use_iordy? */
-				use_iordy = 1;
+			if (id->capability & 8) { /* IORDY supported? */
 				cycle_time = id->eide_pio_iordy;
 				if (id->eide_pio_modes & 7) {
 					overridden = 0;
@@ -325,7 +321,6 @@ u8 ide_get_best_pio_mode (ide_drive_t *d
 	if (d) {
 		d->pio_mode = pio_mode;
 		d->cycle_time = cycle_time ? cycle_time : ide_pio_timings[pio_mode].cycle_time;
-		d->use_iordy = use_iordy;
 	}
 	return pio_mode;
 }
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;
+
+	return (cmd_on - 1) << 8 | (cmd_off - 1) | iordy;
 }
 
 /*
@@ -82,7 +86,7 @@ static u8 sl82c105_tune_pio(ide_drive_t 
 
 	pio = ide_get_best_pio_mode(drive, pio, 5, &p);
 
-	drv_ctrl = get_pio_timings(&p);
+	drv_ctrl = get_pio_timings(drive, &p);
 
 	/*
 	 * Store the PIO timings so that we can restore them
Index: b/include/linux/ide.h
===================================================================
--- a/include/linux/ide.h
+++ b/include/linux/ide.h
@@ -1363,6 +1363,11 @@ extern void ide_toggle_bounce(ide_drive_
 extern int ide_set_xfer_rate(ide_drive_t *drive, u8 rate);
 int ide_use_fast_pio(ide_drive_t *);
 
+static inline int ata_dev_has_iordy(struct hd_driveid *id)
+{
+	return ((id->field_valid & 2) && (id->capability & 8)) ? 1 : 0;
+}
+
 u8 ide_dump_status(ide_drive_t *, const char *, u8);
 
 typedef struct ide_pio_timings_s {
@@ -1374,7 +1379,6 @@ typedef struct ide_pio_timings_s {
 
 typedef struct ide_pio_data_s {
 	u8 pio_mode;
-	u8 use_iordy;
 	unsigned int cycle_time;
 } ide_pio_data_t;
 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 5/6] ide: add ata_dev_has_iordy() helper
  2007-06-27 19:02       ` Bartlomiej Zolnierkiewicz
@ 2007-06-27 20:28         ` Sergei Shtylyov
  2007-06-27 22:10           ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 10+ messages in thread
From: Sergei Shtylyov @ 2007-06-27 20:28 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide

Hello.

Bartlomiej Zolnierkiewicz wrote:

>>>>>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 &&. :-<

> After checking with ATA-2 spec I would prefer to leave extra p->pio_mode > 2
> check (if ata_dev_has_iordy() fails device still _may_ support IORDY).

    Indeed, it may... BUT it may not support PIO > 2 (since this also requires 
the "IORDY supported"  bit set).

> Fixed in "take 3" of the patch.

>>>>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...

> I was actually thinking about a different way of fixing this:

> - remove 0x08 bit from XFER_PIO_[0,6] defines and add new XFER_PIO_IORDY
>   define (<linux/ata.h>)

    Nah, that wouldn't match to the ATA definition of these values.

> - check for speed == XFER_PIO_[0,6] in ide-lib.c::ide_config_drive_speed()

    It's in ide-iops.c. ;-)

>   and pmac.c::pmac_ide_do_setfeature(), add XFER_PIO_IORDY if needed

    And what, just pass the mode thru to the Set Features if there's no IORDY 
support?  That would be bogus since there are just *no* subcodes to set the 
specific mode below 0x08 -- the only defined subcodes are 0x00 (set default 
mode), and 0x01 (set default mode w/o IORDY).
    I was thinking of checking if the drive really supports IORDY before 
issuing a command to set PIO mode (and just skipping the command if there's no 
IORDY -- well, maybe adding an extra check that the passed mode is acceptable 
to the drive, i.e. <= its default one).  Should be quite simple to do.

> This should be done together with fixing these host drivers that don't
> handle IORDY properly.

    Erm, not necessarily...

>>>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...

> Added to the existing IDE TODO at

> 	http://kernel.org/pub/linux/kernel/people/bart/pata-2.6/TODO

> Patches adding/removing items are welcomed.

> Patches fixing actual issues are welcomed even more.

    Sigh, I'm trying to get some time (more like time slices :-) off to deal 
with my own issues...

>>>>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...

> Corrected in "take 3".

    Thanks.

> [PATCH] ide: add ata_dev_has_iordy() helper (take 3)

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

> * Remove no longer needed ide_pio_data_t.use_iordy field.

> v2/v3:
> * Fix issues noticed by Sergei:
>   - correct patch description
>   - fix comment in ide_get_best_pio_mode()

> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>

Acked-by: Sergei Shtylyov <sshtylyov@ru.mvista.com>

MBR, Sergei

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 5/6] ide: add ata_dev_has_iordy() helper
  2007-06-27 20:28         ` Sergei Shtylyov
@ 2007-06-27 22:10           ` Bartlomiej Zolnierkiewicz
  2007-06-29 20:24             ` Sergei Shtylyov
  0 siblings, 1 reply; 10+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-06-27 22:10 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide

On Wednesday 27 June 2007, Sergei Shtylyov wrote:

> >>>>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...
> 
> > I was actually thinking about a different way of fixing this:
> 
> > - remove 0x08 bit from XFER_PIO_[0,6] defines and add new XFER_PIO_IORDY
> >   define (<linux/ata.h>)
> 
>     Nah, that wouldn't match to the ATA definition of these values.
> 
> > - check for speed == XFER_PIO_[0,6] in ide-lib.c::ide_config_drive_speed()
> 
>     It's in ide-iops.c. ;-)
> 
> >   and pmac.c::pmac_ide_do_setfeature(), add XFER_PIO_IORDY if needed
> 
>     And what, just pass the mode thru to the Set Features if there's no IORDY 
> support?  That would be bogus since there are just *no* subcodes to set the 
> specific mode below 0x08 -- the only defined subcodes are 0x00 (set default 
> mode), and 0x01 (set default mode w/o IORDY).

Thinko on my side.

Damn, I should have re-check ATA specs before writing this. :)

>     I was thinking of checking if the drive really supports IORDY before 
> issuing a command to set PIO mode (and just skipping the command if there's no 
> IORDY -- well, maybe adding an extra check that the passed mode is acceptable 
> to the drive, i.e. <= its default one).  Should be quite simple to do.

Sounds fine.

> > This should be done together with fixing these host drivers that don't
> > handle IORDY properly.
> 
>     Erm, not necessarily...

Hmm, yes.

I'll just count on you with fixing all this IORDY stuff (as you have
much more expertise in this field) and concentrate on other things.

> >>>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...
> 
> > Added to the existing IDE TODO at
> 
> > 	http://kernel.org/pub/linux/kernel/people/bart/pata-2.6/TODO
> 
> > Patches adding/removing items are welcomed.
> 
> > Patches fixing actual issues are welcomed even more.
> 
>     Sigh, I'm trying to get some time (more like time slices :-) off to deal 
> with my own issues...

Which reminds me about some HPT IDE patches... 8)

Bart

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 5/6] ide: add ata_dev_has_iordy() helper
  2007-06-27 22:10           ` Bartlomiej Zolnierkiewicz
@ 2007-06-29 20:24             ` Sergei Shtylyov
  2007-06-29 22:16               ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 10+ messages in thread
From: Sergei Shtylyov @ 2007-06-29 20:24 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide

Hello.

Bartlomiej Zolnierkiewicz wrote:

[...]

> Damn, I should have re-check ATA specs before writing this. :)

    Yeah, never hurts... but takes time. ;-)

>>    I was thinking of checking if the drive really supports IORDY before 
>>issuing a command to set PIO mode (and just skipping the command if there's no 
>>IORDY -- well, maybe adding an extra check that the passed mode is acceptable 
>>to the drive, i.e. <= its default one).  Should be quite simple to do.

> Sounds fine.

    I'll look into doing this some time... :-)

>>>This should be done together with fixing these host drivers that don't
>>>handle IORDY properly.

>>    Erm, not necessarily...

> Hmm, yes.

> I'll just count on you with fixing all this IORDY stuff (as you have
> much more expertise in this field) and concentrate on other things.

    I'm afraid this compliment is not well deserved. :-<

>>>>>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...

>>>Added to the existing IDE TODO at

>>>	http://kernel.org/pub/linux/kernel/people/bart/pata-2.6/TODO

    I think this list already needs cleaning -- e.g. at least cmd64x.c and 
hpt366.c have been deatl with.

>>>Patches adding/removing items are welcomed.

>>>Patches fixing actual issues are welcomed even more.

>>    Sigh, I'm trying to get some time (more like time slices :-) off to deal 
>>with my own issues...

    By by "my own issues" I don't mean patches -- patches don't help me, it's 
rather injections and pills... +:-)

> Which reminds me about some HPT IDE patches... 8)

    Done now -- at last. However, I've had troubles with SMPT sending them. :-/

> Bart

WBR, Sergei

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 5/6] ide: add ata_dev_has_iordy() helper
  2007-06-29 20:24             ` Sergei Shtylyov
@ 2007-06-29 22:16               ` Bartlomiej Zolnierkiewicz
  2007-07-04 16:33                 ` Sergei Shtylyov
  0 siblings, 1 reply; 10+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2007-06-29 22:16 UTC (permalink / raw)
  To: Sergei Shtylyov; +Cc: linux-ide


Hi,

On Friday 29 June 2007, Sergei Shtylyov wrote:
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
> [...]
> 
> > Damn, I should have re-check ATA specs before writing this. :)
> 
>     Yeah, never hurts... but takes time. ;-)
> 
> >>    I was thinking of checking if the drive really supports IORDY before 
> >>issuing a command to set PIO mode (and just skipping the command if there's no 
> >>IORDY -- well, maybe adding an extra check that the passed mode is acceptable 
> >>to the drive, i.e. <= its default one).  Should be quite simple to do.
> 
> > Sounds fine.
> 
>     I'll look into doing this some time... :-)

Cool.

> >>>This should be done together with fixing these host drivers that don't
> >>>handle IORDY properly.
> 
> >>    Erm, not necessarily...
> 
> > Hmm, yes.
> 
> > I'll just count on you with fixing all this IORDY stuff (as you have
> > much more expertise in this field) and concentrate on other things.
> 
>     I'm afraid this compliment is not well deserved. :-<
> 
> >>>>>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...
> 
> >>>Added to the existing IDE TODO at
> 
> >>>	http://kernel.org/pub/linux/kernel/people/bart/pata-2.6/TODO
> 
>     I think this list already needs cleaning -- e.g. at least cmd64x.c and 
> hpt366.c have been deatl with.

Thanks, removed.

> >>>Patches adding/removing items are welcomed.
> 
> >>>Patches fixing actual issues are welcomed even more.
> 
> >>    Sigh, I'm trying to get some time (more like time slices :-) off to deal 
> >>with my own issues...
> 
>     By by "my own issues" I don't mean patches -- patches don't help me, it's 
> rather injections and pills... +:-)

Heh, sorry I didn't know about that.

I hope that it is nothing serious and wish you quick recovery...

So you could hack some more... :-)

> > Which reminds me about some HPT IDE patches... 8)
> 
>     Done now -- at last. However, I've had troubles with SMPT sending them. :-/

Looks good, applied all three patches.

Thanks,
Bart

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 5/6] ide: add ata_dev_has_iordy() helper
  2007-06-29 22:16               ` Bartlomiej Zolnierkiewicz
@ 2007-07-04 16:33                 ` Sergei Shtylyov
  0 siblings, 0 replies; 10+ messages in thread
From: Sergei Shtylyov @ 2007-07-04 16:33 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-ide

Bartlomiej Zolnierkiewicz wrote:

>>>>   I was thinking of checking if the drive really supports IORDY before 
>>>>issuing a command to set PIO mode (and just skipping the command if there's no 
>>>>IORDY -- well, maybe adding an extra check that the passed mode is acceptable 
>>>>to the drive, i.e. <= its default one).  Should be quite simple to do.

>>>Sounds fine.

>>    I'll look into doing this some time... :-)

> Cool.

    Unfortunately, got assigned to an urgent bug... been trying to pass it 
along but not much hope on this. :-/

>>>>>Patches adding/removing items are welcomed.

>>>>>Patches fixing actual issues are welcomed even more.

>>>>   Sigh, I'm trying to get some time (more like time slices :-) off to deal 
>>>>with my own issues...

>>    By by "my own issues" I don't mean patches -- patches don't help me, it's 
>>rather injections and pills... +:-)

> Heh, sorry I didn't know about that.

> I hope that it is nothing serious and wish you quick recovery...

    From diabetes? Not bloody likely. :-D

> So you could hack some more... :-)

    That's why I now have to fix at least some consequencies (also of a hard 
hacking :-)

MBR, Sergei

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2007-07-04 16:31 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-06-23 18:05 [PATCH 5/6] ide: add ata_dev_has_iordy() helper Bartlomiej Zolnierkiewicz
2007-06-23 19:36 ` Sergei Shtylyov
2007-06-23 22:09   ` Bartlomiej Zolnierkiewicz
2007-06-24 16:54     ` Sergei Shtylyov
2007-06-27 19:02       ` Bartlomiej Zolnierkiewicz
2007-06-27 20:28         ` Sergei Shtylyov
2007-06-27 22:10           ` Bartlomiej Zolnierkiewicz
2007-06-29 20:24             ` Sergei Shtylyov
2007-06-29 22:16               ` Bartlomiej Zolnierkiewicz
2007-07-04 16:33                 ` Sergei Shtylyov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).