Re: [PATCH 1/3] ata: add ata_id_pio_need_iordy() helper

[Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>]

On Saturday 13 June 2009 22:49:26 Sergei Shtylyov wrote:
> Hello.
> 
> Bartlomiej Zolnierkiewicz wrote:
> 
> > Cc: Jeff Garzik <jgarzik@pobox.com>
> > Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> > ---
> > against Linus' tree
> >
> >  include/linux/ata.h |   11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > Index: b/include/linux/ata.h
> > ===================================================================
> > --- a/include/linux/ata.h
> > +++ b/include/linux/ata.h
> > @@ -800,6 +800,17 @@ static inline int ata_id_is_ssd(const u1
> >  	return id[ATA_ID_ROT_SPEED] == 0x01;
> >  }
> >  
> > +static inline int ata_id_pio_need_iordy(const u16 *id, const u8 pio)
> > +{
> > +	/* CF spec. r4.1 Table 22 says no IORDY on PIO5 and PIO6.  */
> > +	if (ata_id_is_cfa(id) && pio > 4)
> >   
> 
>    It actually makes sense to put the cheap 'pio' check first. We 
> shouldn't call ata_id_is_cfa() for all modes.

It is a micro-optimization but why not... revised version follows.

From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Subject: [PATCH] ata: add ata_id_pio_need_iordy() helper (v2)

v2:
Minor fixes per Sergei's review.

Cc: Sergei Shtylyov <sshtylyov@ru.mvista.com>
Cc: Jeff Garzik <jgarzik@pobox.com>
Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
---
against Linus' tree

 include/linux/ata.h |   14 ++++++++++++++
 1 file changed, 14 insertions(+)

Index: b/include/linux/ata.h
===================================================================
--- a/include/linux/ata.h
+++ b/include/linux/ata.h
@@ -800,6 +800,20 @@ static inline int ata_id_is_ssd(const u1
 	return id[ATA_ID_ROT_SPEED] == 0x01;
 }
 
+static inline int ata_id_pio_need_iordy(const u16 *id, const u8 pio)
+{
+	/* CF spec. r4.1 Table 22 says no IORDY on PIO5 and PIO6. */
+	if (pio > 4 && ata_id_is_cfa(id))
+		return 0;
+	/* For PIO3 and higher it is mandatory. */
+	if (pio > 2)
+		return 1;
+	/* Turn it on when possible. */
+	if (ata_id_has_iordy(id))
+		return 1;
+	return 0;
+}
+
 static inline int ata_drive_40wire(const u16 *dev_id)
 {
 	if (ata_id_is_sata(dev_id))