Re: [patch 1/7] libata: check for AN support

2007-06-11 Thread Kristen Carlson Accardi 
On Thu, 24 May 2007 23:15:56 -0400
Jeff Garzik [EMAIL PROTECTED] wrote:

 Kristen Carlson Accardi wrote:
  Check to see if an ATAPI device supports Asynchronous Notification.
  If so, enable it.
  
  Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED]
  ---
  Andrew, I cleaned up the function header to properly comply with kernel
  doc requirements.  Other than that, this patch is the same.  
 
 I would ask for a simple revision:  update ata_dev_set_AN() such that it 
 takes a second argument 'enable'.  This boolean indicates to the 
 function whether SETFEATURES_SATA_ENABLE or SETFEATURES_SATA_DISABLE 
 should be passed to the device.
 
 Otherwise than that, it's ready to merge I would say.
 

Jeff - can you fold this into the original patch, or would you like me
to resubmit the whole thing?

Kristen

Modify ata_dev_set_AN to take a second argument 'enable'.  This
boolean indicates to the function whether SETFEATURES_SATA_ENABLE
or SETFEATURES_SATA_DISABLE should be passed to the device.

Signed-off-by:  Kristen Carlson Accardi [EMAIL PROTECTED]

Index: 2.6-git/drivers/ata/libata-core.c
===
--- 2.6-git.orig/drivers/ata/libata-core.c
+++ 2.6-git/drivers/ata/libata-core.c
@@ -70,7 +70,7 @@ const unsigned long sata_deb_timing_long
 static unsigned int ata_dev_init_params(struct ata_device *dev,
u16 heads, u16 sectors);
 static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
-static unsigned int ata_dev_set_AN(struct ata_device *dev);
+static unsigned int ata_dev_set_AN(struct ata_device *dev, u8 enable);
 static void ata_dev_xfermask(struct ata_device *dev);
 
 unsigned int ata_print_id = 1;
@@ -2010,7 +2010,7 @@ int ata_dev_configure(struct ata_device 
if ((ap-flags  ATA_FLAG_AN)  ata_id_has_AN(id)) {
int err;
/* issue SET feature command to turn this on */
-   err = ata_dev_set_AN(dev);
+   err = ata_dev_set_AN(dev, SETFEATURES_SATA_ENABLE);
if (err)
ata_dev_printk(dev, KERN_ERR,
unable to set AN, err %x\n,
@@ -3966,6 +3966,7 @@ static unsigned int ata_dev_set_xfermode
 /**
  * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
  * @dev: Device to which command will be sent
+ * @enable: Whether to enable or disable the feature
  *
  * Issue SET FEATURES - SATA FEATURES command to device @dev
  * on port @ap with sector count set to indicate Asynchronous
@@ -3977,7 +3978,7 @@ static unsigned int ata_dev_set_xfermode
  * RETURNS:
  * 0 on success, AC_ERR_* mask otherwise.
  */
-static unsigned int ata_dev_set_AN(struct ata_device *dev)
+static unsigned int ata_dev_set_AN(struct ata_device *dev, u8 enable)
 {
struct ata_taskfile tf;
unsigned int err_mask;
@@ -3987,7 +3988,7 @@ static unsigned int ata_dev_set_AN(struc
 
ata_tf_init(dev, tf);
tf.command = ATA_CMD_SET_FEATURES;
-   tf.feature = SETFEATURES_SATA_ENABLE;
+   tf.feature = enable;
tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
tf.protocol = ATA_PROT_NODATA;
tf.nsect = SATA_AN;
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 1/7] libata: check for AN support

2007-05-24 Thread Jeff Garzik 

Kristen Carlson Accardi wrote:

Check to see if an ATAPI device supports Asynchronous Notification.
If so, enable it.

Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED]
---
Andrew, I cleaned up the function header to properly comply with kernel
doc requirements.  Other than that, this patch is the same.  


I would ask for a simple revision:  update ata_dev_set_AN() such that it 
takes a second argument 'enable'.  This boolean indicates to the 
function whether SETFEATURES_SATA_ENABLE or SETFEATURES_SATA_DISABLE 
should be passed to the device.


Otherwise than that, it's ready to merge I would say.

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

Re: [patch 1/7] libata: check for AN support

2007-05-10 Thread Randy Dunlap 
On Wed, 9 May 2007 22:09:52 -0700 Andrew Morton wrote:

 On Wed, 9 May 2007 16:38:09 -0700 Kristen Carlson Accardi [EMAIL PROTECTED] 
 wrote:
 
   /**
  + * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
  + *   with sector count set to indicate
  + *   Asynchronous Notification feature
 
 I think kenreldoc requires that all this be on a single line?

Correct.

  + * @dev: Device to which command will be sent
  + *
  + * Issue SET FEATURES - SATA FEATURES command to device @dev
  + * on port @ap.
  + *
  + * LOCKING:
  + * PCI/etc. bus probe sem.
  + *
  + * RETURNS:
  + * 0 on success, AC_ERR_* mask otherwise.
  + */
 
 ooh, locking and return value documentation.  Often missed, and nice.

---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 1/7] libata: check for AN support

2007-05-10 Thread Kristen Carlson Accardi 
Check to see if an ATAPI device supports Asynchronous Notification.
If so, enable it.

Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED]
---
Andrew, I cleaned up the function header to properly comply with kernel
doc requirements.  Other than that, this patch is the same.  

Index: 2.6-mm/drivers/ata/libata-core.c
===
--- 2.6-mm.orig/drivers/ata/libata-core.c
+++ 2.6-mm/drivers/ata/libata-core.c
@@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long
 static unsigned int ata_dev_init_params(struct ata_device *dev,
u16 heads, u16 sectors);
 static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
+static unsigned int ata_dev_set_AN(struct ata_device *dev);
 static void ata_dev_xfermask(struct ata_device *dev);
 
 unsigned int ata_print_id = 1;
@@ -1983,6 +1984,22 @@ int ata_dev_configure(struct ata_device 
}
dev-cdb_len = (unsigned int) rc;
 
+   /*
+* check to see if this ATAPI device supports
+* Asynchronous Notification
+*/
+   if ((ap-flags  ATA_FLAG_AN)  ata_id_has_AN(id)) {
+   int err;
+   /* issue SET feature command to turn this on */
+   err = ata_dev_set_AN(dev);
+   if (err)
+   ata_dev_printk(dev, KERN_ERR,
+   unable to set AN, err %x\n,
+   err);
+   else
+   dev-flags |= ATA_DFLAG_AN;
+   }
+
if (ata_id_cdb_intr(dev-id)) {
dev-flags |= ATA_DFLAG_CDB_INTR;
cdb_intr_string = , CDB intr;
@@ -3910,6 +3927,41 @@ static unsigned int ata_dev_set_xfermode
 }
 
 /**
+ * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
+ * @dev: Device to which command will be sent
+ *
+ * Issue SET FEATURES - SATA FEATURES command to device @dev
+ * on port @ap with sector count set to indicate Asynchronous
+ * Notification feature
+ *
+ * LOCKING:
+ * PCI/etc. bus probe sem.
+ *
+ * RETURNS:
+ * 0 on success, AC_ERR_* mask otherwise.
+ */
+static unsigned int ata_dev_set_AN(struct ata_device *dev)
+{
+   struct ata_taskfile tf;
+   unsigned int err_mask;
+
+   /* set up set-features taskfile */
+   DPRINTK(set features - SATA features\n);
+
+   ata_tf_init(dev, tf);
+   tf.command = ATA_CMD_SET_FEATURES;
+   tf.feature = SETFEATURES_SATA_ENABLE;
+   tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+   tf.protocol = ATA_PROT_NODATA;
+   tf.nsect = SATA_AN;
+
+   err_mask = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0);
+
+   DPRINTK(EXIT, err_mask=%x\n, err_mask);
+   return err_mask;
+}
+
+/**
  * ata_dev_init_params - Issue INIT DEV PARAMS command
  * @dev: Device to which command will be sent
  * @heads: Number of heads (taskfile parameter)
Index: 2.6-mm/include/linux/ata.h
===
--- 2.6-mm.orig/include/linux/ata.h
+++ 2.6-mm/include/linux/ata.h
@@ -204,6 +204,12 @@ enum {
 
SETFEATURES_SPINUP  = 0x07, /* Spin-up drive */
 
+   SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */
+   SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */
+
+   /* SETFEATURE Sector counts for SATA features */
+   SATA_AN = 0x05,  /* Asynchronous Notification */
+
/* ATAPI stuff */
ATAPI_PKT_DMA   = (1  0),
ATAPI_DMADIR= (1  2), /* ATAPI data dir:
@@ -309,6 +315,9 @@ struct ata_taskfile {
 #define ata_id_queue_depth(id) (((id)[75]  0x1f) + 1)
 #define ata_id_removeable(id)  ((id)[0]  (1  7))
 #define ata_id_has_dword_io(id)((id)[50]  (1  0))
+#define ata_id_has_AN(id)  \
+   ( (((id)[76] != 0x)  ((id)[76] != 0x))  \
+ ((id)[78]  (1  5)) )
 #define ata_id_iordy_disable(id) ((id)[49]  (1  10))
 #define ata_id_has_iordy(id) ((id)[49]  (1  9))
 #define ata_id_u32(id,n)   \
Index: 2.6-mm/include/linux/libata.h
===
--- 2.6-mm.orig/include/linux/libata.h
+++ 2.6-mm/include/linux/libata.h
@@ -136,6 +136,7 @@ enum {
ATA_DFLAG_CDB_INTR  = (1  2), /* device asserts INTRQ when ready 
for CDB */
ATA_DFLAG_NCQ   = (1  3), /* device supports NCQ */
ATA_DFLAG_FLUSH_EXT = (1  4), /* do FLUSH_EXT instead of FLUSH */
+   ATA_DFLAG_AN= (1  5), /* device supports Async 
notification */
ATA_DFLAG_CFG_MASK  = (1  8) - 1,
 
ATA_DFLAG_PIO   = (1  8), /* device limited to PIO mode */
@@ -174,6 +175,7 @@ enum {
ATA_FLAG_SETXFER_POLLING= (1  14), /* use

[patch 1/7] libata: check for AN support

2007-05-09 Thread Kristen Carlson Accardi 
Check to see if an ATAPI device supports Asynchronous Notification.
If so, enable it.

Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED]

Index: 2.6-git/drivers/ata/libata-core.c
===
--- 2.6-git.orig/drivers/ata/libata-core.c
+++ 2.6-git/drivers/ata/libata-core.c
@@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long
 static unsigned int ata_dev_init_params(struct ata_device *dev,
u16 heads, u16 sectors);
 static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
+static unsigned int ata_dev_set_AN(struct ata_device *dev);
 static void ata_dev_xfermask(struct ata_device *dev);
 
 unsigned int ata_print_id = 1;
@@ -1981,6 +1982,22 @@ int ata_dev_configure(struct ata_device 
}
dev-cdb_len = (unsigned int) rc;
 
+   /*
+* check to see if this ATAPI device supports
+* Asynchronous Notification
+*/
+   if ((ap-flags  ATA_FLAG_AN)  ata_id_has_AN(id)) {
+   int err;
+   /* issue SET feature command to turn this on */
+   err = ata_dev_set_AN(dev);
+   if (err)
+   ata_dev_printk(dev, KERN_ERR,
+   unable to set AN, err %x\n,
+   err);
+   else
+   dev-flags |= ATA_DFLAG_AN;
+   }
+
if (ata_id_cdb_intr(dev-id)) {
dev-flags |= ATA_DFLAG_CDB_INTR;
cdb_intr_string = , CDB intr;
@@ -3908,6 +3925,42 @@ static unsigned int ata_dev_set_xfermode
 }
 
 /**
+ * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
+ *   with sector count set to indicate
+ *   Asynchronous Notification feature
+ * @dev: Device to which command will be sent
+ *
+ * Issue SET FEATURES - SATA FEATURES command to device @dev
+ * on port @ap.
+ *
+ * LOCKING:
+ * PCI/etc. bus probe sem.
+ *
+ * RETURNS:
+ * 0 on success, AC_ERR_* mask otherwise.
+ */
+static unsigned int ata_dev_set_AN(struct ata_device *dev)
+{
+   struct ata_taskfile tf;
+   unsigned int err_mask;
+
+   /* set up set-features taskfile */
+   DPRINTK(set features - SATA features\n);
+
+   ata_tf_init(dev, tf);
+   tf.command = ATA_CMD_SET_FEATURES;
+   tf.feature = SETFEATURES_SATA_ENABLE;
+   tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+   tf.protocol = ATA_PROT_NODATA;
+   tf.nsect = SATA_AN;
+
+   err_mask = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0);
+
+   DPRINTK(EXIT, err_mask=%x\n, err_mask);
+   return err_mask;
+}
+
+/**
  * ata_dev_init_params - Issue INIT DEV PARAMS command
  * @dev: Device to which command will be sent
  * @heads: Number of heads (taskfile parameter)
Index: 2.6-git/include/linux/ata.h
===
--- 2.6-git.orig/include/linux/ata.h
+++ 2.6-git/include/linux/ata.h
@@ -204,6 +204,12 @@ enum {
 
SETFEATURES_SPINUP  = 0x07, /* Spin-up drive */
 
+   SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */
+   SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */
+
+   /* SETFEATURE Sector counts for SATA features */
+   SATA_AN = 0x05,  /* Asynchronous Notification */
+
/* ATAPI stuff */
ATAPI_PKT_DMA   = (1  0),
ATAPI_DMADIR= (1  2), /* ATAPI data dir:
@@ -309,6 +315,9 @@ struct ata_taskfile {
 #define ata_id_queue_depth(id) (((id)[75]  0x1f) + 1)
 #define ata_id_removeable(id)  ((id)[0]  (1  7))
 #define ata_id_has_dword_io(id)((id)[50]  (1  0))
+#define ata_id_has_AN(id)  \
+   ( (((id)[76] != 0x)  ((id)[76] != 0x))  \
+ ((id)[78]  (1  5)) )
 #define ata_id_iordy_disable(id) ((id)[49]  (1  10))
 #define ata_id_has_iordy(id) ((id)[49]  (1  9))
 #define ata_id_u32(id,n)   \
Index: 2.6-git/include/linux/libata.h
===
--- 2.6-git.orig/include/linux/libata.h
+++ 2.6-git/include/linux/libata.h
@@ -136,6 +136,7 @@ enum {
ATA_DFLAG_CDB_INTR  = (1  2), /* device asserts INTRQ when ready 
for CDB */
ATA_DFLAG_NCQ   = (1  3), /* device supports NCQ */
ATA_DFLAG_FLUSH_EXT = (1  4), /* do FLUSH_EXT instead of FLUSH */
+   ATA_DFLAG_AN= (1  5), /* device supports Async 
notification */
ATA_DFLAG_CFG_MASK  = (1  8) - 1,
 
ATA_DFLAG_PIO   = (1  8), /* device limited to PIO mode */
@@ -174,6 +175,7 @@ enum {
ATA_FLAG_SETXFER_POLLING= (1  14), /* use polling for SETXFER */
ATA_FLAG_IGN_SIMPLEX= (1  15), /* ignore SIMPLEX

Re: [patch 1/7] libata: check for AN support

2007-05-09 Thread Andrew Morton 
On Wed, 9 May 2007 16:38:09 -0700 Kristen Carlson Accardi [EMAIL PROTECTED] 
wrote:

  /**
 + *   ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
 + *   with sector count set to indicate
 + *   Asynchronous Notification feature

I think kenreldoc requires that all this be on a single line?

 + *   @dev: Device to which command will be sent
 + *
 + *   Issue SET FEATURES - SATA FEATURES command to device @dev
 + *   on port @ap.
 + *
 + *   LOCKING:
 + *   PCI/etc. bus probe sem.
 + *
 + *   RETURNS:
 + *   0 on success, AC_ERR_* mask otherwise.
 + */

ooh, locking and return value documentation.  Often missed, and nice.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 1/7] libata: check for AN support

2007-05-09 Thread Jeff Garzik 

Andrew Morton wrote:

+ * @dev: Device to which command will be sent
+ *
+ * Issue SET FEATURES - SATA FEATURES command to device @dev
+ * on port @ap.
+ *
+ * LOCKING:
+ * PCI/etc. bus probe sem.
+ *
+ * RETURNS:
+ * 0 on success, AC_ERR_* mask otherwise.
+ */


ooh, locking and return value documentation.  Often missed, and nice.



We set high standards in the libata world ;-)

Jeff


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

Re: [patch 1/7] libata: check for AN support

2007-05-09 Thread Andrew Morton 
On Thu, 10 May 2007 01:14:38 -0400 Jeff Garzik [EMAIL PROTECTED] wrote:

 Andrew Morton wrote:
  + *@dev: Device to which command will be sent
  + *
  + *Issue SET FEATURES - SATA FEATURES command to device @dev
  + *on port @ap.
  + *
  + *LOCKING:
  + *PCI/etc. bus probe sem.
  + *
  + *RETURNS:
  + *0 on success, AC_ERR_* mask otherwise.
  + */
  
  ooh, locking and return value documentation.  Often missed, and nice.
 
 
 We set high standards in the libata world ;-)
 

It seems to be working.  This series is perhaps the most idiomatic and
generally kernelly-looking code I've seen in ages.

Who cares if it works? ;)
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[patch 1/7] libata: check for AN support - resend

2007-05-04 Thread Kristen Carlson Accardi 
Check to see if an ATAPI device supports Asynchronous Notification.
If so, enable it.

Changes from last version:
* use parens around id in ata.h

Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED]

Index: 2.6-git/drivers/ata/libata-core.c
===
--- 2.6-git.orig/drivers/ata/libata-core.c
+++ 2.6-git/drivers/ata/libata-core.c
@@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long
 static unsigned int ata_dev_init_params(struct ata_device *dev,
u16 heads, u16 sectors);
 static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
+static unsigned int ata_dev_set_AN(struct ata_device *dev);
 static void ata_dev_xfermask(struct ata_device *dev);
 
 static unsigned int ata_print_id = 1;
@@ -1744,6 +1745,22 @@ int ata_dev_configure(struct ata_device 
}
dev-cdb_len = (unsigned int) rc;
 
+   /*
+* check to see if this ATAPI device supports
+* Asynchronous Notification
+*/
+   if ((ap-flags  ATA_FLAG_AN)  ata_id_has_AN(id)) {
+   int err;
+   /* issue SET feature command to turn this on */
+   err = ata_dev_set_AN(dev);
+   if (err)
+   ata_dev_printk(dev, KERN_ERR,
+   unable to set AN, err %x\n,
+   err);
+   else
+   dev-flags |= ATA_DFLAG_AN;
+   }
+
if (ata_id_cdb_intr(dev-id)) {
dev-flags |= ATA_DFLAG_CDB_INTR;
cdb_intr_string = , CDB intr;
@@ -3525,6 +3542,42 @@ static unsigned int ata_dev_set_xfermode
 }
 
 /**
+ * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
+ *   with sector count set to indicate
+ *   Asynchronous Notification feature
+ * @dev: Device to which command will be sent
+ *
+ * Issue SET FEATURES - SATA FEATURES command to device @dev
+ * on port @ap.
+ *
+ * LOCKING:
+ * PCI/etc. bus probe sem.
+ *
+ * RETURNS:
+ * 0 on success, AC_ERR_* mask otherwise.
+ */
+static unsigned int ata_dev_set_AN(struct ata_device *dev)
+{
+   struct ata_taskfile tf;
+   unsigned int err_mask;
+
+   /* set up set-features taskfile */
+   DPRINTK(set features - SATA features\n);
+
+   ata_tf_init(dev, tf);
+   tf.command = ATA_CMD_SET_FEATURES;
+   tf.feature = SETFEATURES_SATA_ENABLE;
+   tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+   tf.protocol = ATA_PROT_NODATA;
+   tf.nsect = SATA_AN;
+
+   err_mask = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0);
+
+   DPRINTK(EXIT, err_mask=%x\n, err_mask);
+   return err_mask;
+}
+
+/**
  * ata_dev_init_params - Issue INIT DEV PARAMS command
  * @dev: Device to which command will be sent
  * @heads: Number of heads (taskfile parameter)
Index: 2.6-git/include/linux/ata.h
===
--- 2.6-git.orig/include/linux/ata.h
+++ 2.6-git/include/linux/ata.h
@@ -194,6 +194,12 @@ enum {
SETFEATURES_WC_ON   = 0x02, /* Enable write cache */
SETFEATURES_WC_OFF  = 0x82, /* Disable write cache */
 
+   SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */
+   SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */
+
+   /* SETFEATURE Sector counts for SATA features */
+   SATA_AN = 0x05,  /* Asynchronous Notification */
+
/* ATAPI stuff */
ATAPI_PKT_DMA   = (1  0),
ATAPI_DMADIR= (1  2), /* ATAPI data dir:
@@ -299,6 +305,9 @@ struct ata_taskfile {
 #define ata_id_queue_depth(id) (((id)[75]  0x1f) + 1)
 #define ata_id_removeable(id)  ((id)[0]  (1  7))
 #define ata_id_has_dword_io(id)((id)[50]  (1  0))
+#define ata_id_has_AN(id)  \
+   ( (((id)[76] != 0x)  ((id)[76] != 0x))  \
+ ((id)[78]  (1  5)) )
 #define ata_id_iordy_disable(id) ((id)[49]  (1  10))
 #define ata_id_has_iordy(id) ((id)[49]  (1  9))
 #define ata_id_u32(id,n)   \
Index: 2.6-git/include/linux/libata.h
===
--- 2.6-git.orig/include/linux/libata.h
+++ 2.6-git/include/linux/libata.h
@@ -136,6 +136,7 @@ enum {
ATA_DFLAG_CDB_INTR  = (1  2), /* device asserts INTRQ when ready 
for CDB */
ATA_DFLAG_NCQ   = (1  3), /* device supports NCQ */
ATA_DFLAG_FLUSH_EXT = (1  4), /* do FLUSH_EXT instead of FLUSH */
+   ATA_DFLAG_AN= (1  5), /* device supports Async 
notification */
ATA_DFLAG_CFG_MASK  = (1  8) - 1,
 
ATA_DFLAG_PIO   = (1  8), /* device limited to PIO mode */
@@ -174,6 +175,7 @@ enum

Re: [patch 1/7] libata: check for AN support

2007-04-25 Thread Kristen Carlson Accardi 
On Wed, 25 Apr 2007 02:49:46 +0200
Olivier Galibert [EMAIL PROTECTED] wrote:

 On Tue, Apr 24, 2007 at 01:53:27PM -0700, Kristen Carlson Accardi wrote:
  Check to see if an ATAPI device supports Asynchronous Notification.
  If so, enable it.
  
  changes from last version: 
  * fix typo in ata_id_has_AN and make word 76 test more clear
  * If we fail to set the AN feature, just print a warning and continue
   
  Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED]
  
  @@ -299,6 +305,8 @@ struct ata_taskfile {
   #define ata_id_queue_depth(id) (((id)[75]  0x1f) + 1)
   #define ata_id_removeable(id)  ((id)[0]  (1  7))
   #define ata_id_has_dword_io(id)((id)[50]  (1  0))
  +#define ata_id_has_AN(id)  \
  +   (((id[76] != 0x)  (id[76] != 0x))  ((id)[78]  (1  5)))
 
 (id)[76] I guess ?  Sorry for being a pain :/
 
   OG.
 

Ok - I'll fix that.  Thank you for being a pain :), I really appreciate
the time you are taking to review my patches.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 1/7] libata: check for AN support

2007-04-25 Thread Kristen Carlson Accardi 
Check to see if an ATAPI device supports Asynchronous Notification.
If so, enable it.

Changes from last version:
* use parens around id in ata.h

Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED]

Index: 2.6-git/drivers/ata/libata-core.c
===
--- 2.6-git.orig/drivers/ata/libata-core.c
+++ 2.6-git/drivers/ata/libata-core.c
@@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long
 static unsigned int ata_dev_init_params(struct ata_device *dev,
u16 heads, u16 sectors);
 static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
+static unsigned int ata_dev_set_AN(struct ata_device *dev);
 static void ata_dev_xfermask(struct ata_device *dev);
 
 static unsigned int ata_print_id = 1;
@@ -1744,6 +1745,22 @@ int ata_dev_configure(struct ata_device 
}
dev-cdb_len = (unsigned int) rc;
 
+   /*
+* check to see if this ATAPI device supports
+* Asynchronous Notification
+*/
+   if ((ap-flags  ATA_FLAG_AN)  ata_id_has_AN(id)) {
+   int err;
+   /* issue SET feature command to turn this on */
+   err = ata_dev_set_AN(dev);
+   if (err)
+   ata_dev_printk(dev, KERN_ERR,
+   unable to set AN, err %x\n,
+   err);
+   else
+   dev-flags |= ATA_DFLAG_AN;
+   }
+
if (ata_id_cdb_intr(dev-id)) {
dev-flags |= ATA_DFLAG_CDB_INTR;
cdb_intr_string = , CDB intr;
@@ -3525,6 +3542,42 @@ static unsigned int ata_dev_set_xfermode
 }
 
 /**
+ * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
+ *   with sector count set to indicate
+ *   Asynchronous Notification feature
+ * @dev: Device to which command will be sent
+ *
+ * Issue SET FEATURES - SATA FEATURES command to device @dev
+ * on port @ap.
+ *
+ * LOCKING:
+ * PCI/etc. bus probe sem.
+ *
+ * RETURNS:
+ * 0 on success, AC_ERR_* mask otherwise.
+ */
+static unsigned int ata_dev_set_AN(struct ata_device *dev)
+{
+   struct ata_taskfile tf;
+   unsigned int err_mask;
+
+   /* set up set-features taskfile */
+   DPRINTK(set features - SATA features\n);
+
+   ata_tf_init(dev, tf);
+   tf.command = ATA_CMD_SET_FEATURES;
+   tf.feature = SETFEATURES_SATA_ENABLE;
+   tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+   tf.protocol = ATA_PROT_NODATA;
+   tf.nsect = SATA_AN;
+
+   err_mask = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0);
+
+   DPRINTK(EXIT, err_mask=%x\n, err_mask);
+   return err_mask;
+}
+
+/**
  * ata_dev_init_params - Issue INIT DEV PARAMS command
  * @dev: Device to which command will be sent
  * @heads: Number of heads (taskfile parameter)
Index: 2.6-git/include/linux/ata.h
===
--- 2.6-git.orig/include/linux/ata.h
+++ 2.6-git/include/linux/ata.h
@@ -194,6 +194,12 @@ enum {
SETFEATURES_WC_ON   = 0x02, /* Enable write cache */
SETFEATURES_WC_OFF  = 0x82, /* Disable write cache */
 
+   SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */
+   SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */
+
+   /* SETFEATURE Sector counts for SATA features */
+   SATA_AN = 0x05,  /* Asynchronous Notification */
+
/* ATAPI stuff */
ATAPI_PKT_DMA   = (1  0),
ATAPI_DMADIR= (1  2), /* ATAPI data dir:
@@ -299,6 +305,9 @@ struct ata_taskfile {
 #define ata_id_queue_depth(id) (((id)[75]  0x1f) + 1)
 #define ata_id_removeable(id)  ((id)[0]  (1  7))
 #define ata_id_has_dword_io(id)((id)[50]  (1  0))
+#define ata_id_has_AN(id)  \
+   ( (((id)[76] != 0x)  ((id)[76] != 0x))  \
+ ((id)[78]  (1  5)) )
 #define ata_id_iordy_disable(id) ((id)[49]  (1  10))
 #define ata_id_has_iordy(id) ((id)[49]  (1  9))
 #define ata_id_u32(id,n)   \
Index: 2.6-git/include/linux/libata.h
===
--- 2.6-git.orig/include/linux/libata.h
+++ 2.6-git/include/linux/libata.h
@@ -136,6 +136,7 @@ enum {
ATA_DFLAG_CDB_INTR  = (1  2), /* device asserts INTRQ when ready 
for CDB */
ATA_DFLAG_NCQ   = (1  3), /* device supports NCQ */
ATA_DFLAG_FLUSH_EXT = (1  4), /* do FLUSH_EXT instead of FLUSH */
+   ATA_DFLAG_AN= (1  5), /* device supports Async 
notification */
ATA_DFLAG_CFG_MASK  = (1  8) - 1,
 
ATA_DFLAG_PIO   = (1  8), /* device limited to PIO mode */
@@ -174,6 +175,7 @@ enum

Re: [patch 1/7] libata: check for AN support

2007-04-25 Thread Matt Sealey 

Kristen Carlson Accardi wrote:
 Check to see if an ATAPI device supports Asynchronous Notification.
 If so, enable it.
 
 Changes from last version:
 * use parens around id in ata.h
 
 Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED]
 
 Index: 2.6-git/drivers/ata/libata-core.c
 ===
 --- 2.6-git.orig/drivers/ata/libata-core.c
 +++ 2.6-git/drivers/ata/libata-core.c
 @@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long
  static unsigned int ata_dev_init_params(struct ata_device *dev,
   u16 heads, u16 sectors);
  static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
 +static unsigned int ata_dev_set_AN(struct ata_device *dev);
  static void ata_dev_xfermask(struct ata_device *dev);
  
  static unsigned int ata_print_id = 1;
 @@ -1744,6 +1745,22 @@ int ata_dev_configure(struct ata_device 
   }
   dev-cdb_len = (unsigned int) rc;
  
 + /*
 +  * check to see if this ATAPI device supports
 +  * Asynchronous Notification
 +  */
 + if ((ap-flags  ATA_FLAG_AN)  ata_id_has_AN(id)) {
 + int err;
 + /* issue SET feature command to turn this on */
 + err = ata_dev_set_AN(dev);
 + if (err)
 + ata_dev_printk(dev, KERN_ERR,
 + unable to set AN, err %x\n,
 + err);
 + else
 + dev-flags |= ATA_DFLAG_AN;
 + }
 +
   if (ata_id_cdb_intr(dev-id)) {
   dev-flags |= ATA_DFLAG_CDB_INTR;
   cdb_intr_string = , CDB intr;
 @@ -3525,6 +3542,42 @@ static unsigned int ata_dev_set_xfermode
  }
  
  /**
 + *   ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
 + *   with sector count set to indicate
 + *   Asynchronous Notification feature
 + *   @dev: Device to which command will be sent
 + *
 + *   Issue SET FEATURES - SATA FEATURES command to device @dev
 + *   on port @ap.
 + *
 + *   LOCKING:
 + *   PCI/etc. bus probe sem.
 + *
 + *   RETURNS:
 + *   0 on success, AC_ERR_* mask otherwise.
 + */
 +static unsigned int ata_dev_set_AN(struct ata_device *dev)
 +{
 + struct ata_taskfile tf;
 + unsigned int err_mask;
 +
 + /* set up set-features taskfile */
 + DPRINTK(set features - SATA features\n);
 +
 + ata_tf_init(dev, tf);
 + tf.command = ATA_CMD_SET_FEATURES;
 + tf.feature = SETFEATURES_SATA_ENABLE;
 + tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
 + tf.protocol = ATA_PROT_NODATA;
 + tf.nsect = SATA_AN;
 +
 + err_mask = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0);
 +
 + DPRINTK(EXIT, err_mask=%x\n, err_mask);
 + return err_mask;
 +}
 +
 +/**
   *   ata_dev_init_params - Issue INIT DEV PARAMS command
   *   @dev: Device to which command will be sent
   *   @heads: Number of heads (taskfile parameter)
 Index: 2.6-git/include/linux/ata.h
 ===
 --- 2.6-git.orig/include/linux/ata.h
 +++ 2.6-git/include/linux/ata.h
 @@ -194,6 +194,12 @@ enum {
   SETFEATURES_WC_ON   = 0x02, /* Enable write cache */
   SETFEATURES_WC_OFF  = 0x82, /* Disable write cache */
  
 + SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */
 + SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */
 +
 + /* SETFEATURE Sector counts for SATA features */
 + SATA_AN = 0x05,  /* Asynchronous Notification */
 +
   /* ATAPI stuff */
   ATAPI_PKT_DMA   = (1  0),
   ATAPI_DMADIR= (1  2), /* ATAPI data dir:
 @@ -299,6 +305,9 @@ struct ata_taskfile {
  #define ata_id_queue_depth(id)   (((id)[75]  0x1f) + 1)
  #define ata_id_removeable(id)((id)[0]  (1  7))
  #define ata_id_has_dword_io(id)  ((id)[50]  (1  0))
 +#define ata_id_has_AN(id)\
 + ( (((id)[76] != 0x)  ((id)[76] != 0x))  \
 +   ((id)[78]  (1  5)) )

??

 --- 2.6-git.orig/include/linux/libata.h
 +++ 2.6-git/include/linux/libata.h
 @@ -136,6 +136,7 @@ enum {
   ATA_DFLAG_CDB_INTR  = (1  2), /* device asserts INTRQ when ready 
 for CDB */
   ATA_DFLAG_NCQ   = (1  3), /* device supports NCQ */
   ATA_DFLAG_FLUSH_EXT = (1  4), /* do FLUSH_EXT instead of FLUSH */
 + ATA_DFLAG_AN= (1  5), /* device supports Async 
 notification */
   ATA_DFLAG_CFG_MASK  = (1  8) - 1,

Why don't the macros use the enums? It makes the code hard to read without
painful cross-reference doesn't it? Surely (id)[76]  (ATA_DFLAG_AN) is a
lot more readable than 1  5 - even if the flag is obviously that, a lot
of values and registers can have 1  5 as a flag and mean a lot of different

Re: [patch 1/7] libata: check for AN support

2007-04-25 Thread Kristen Carlson Accardi 
On Wed, 25 Apr 2007 20:16:51 +0100
Matt Sealey [EMAIL PROTECTED] wrote:

  +#define ata_id_has_AN(id)  \
  +   ( (((id)[76] != 0x)  ((id)[76] != 0x))  \
  + ((id)[78]  (1  5)) )
 
 ??
 
  --- 2.6-git.orig/include/linux/libata.h
  +++ 2.6-git/include/linux/libata.h
  @@ -136,6 +136,7 @@ enum {
  ATA_DFLAG_CDB_INTR  = (1  2), /* device asserts INTRQ when ready 
  for CDB */
  ATA_DFLAG_NCQ   = (1  3), /* device supports NCQ */
  ATA_DFLAG_FLUSH_EXT = (1  4), /* do FLUSH_EXT instead of FLUSH */
  +   ATA_DFLAG_AN= (1  5), /* device supports Async 
  notification */
  ATA_DFLAG_CFG_MASK  = (1  8) - 1,
 
 Why don't the macros use the enums? It makes the code hard to read without
 painful cross-reference doesn't it? Surely (id)[76]  (ATA_DFLAG_AN) is a
 lot more readable than 1  5 - even if the flag is obviously that, a lot
 of values and registers can have 1  5 as a flag and mean a lot of different
 things.

It's really just a coincidence that the ATA_DFLAG_AN bit is the same as the bit
in the identify device word, so this would not be appropriate.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 1/7] libata: check for AN support

2007-04-25 Thread Matt Sealey 


Kristen Carlson Accardi wrote:
 On Wed, 25 Apr 2007 20:16:51 +0100
 Matt Sealey [EMAIL PROTECTED] wrote:
 
 +#define ata_id_has_AN(id)  \
 +   ( (((id)[76] != 0x)  ((id)[76] != 0x))  \
 + ((id)[78]  (1  5)) )
 ??

 --- 2.6-git.orig/include/linux/libata.h
 +++ 2.6-git/include/linux/libata.h
 @@ -136,6 +136,7 @@ enum {
 ATA_DFLAG_CDB_INTR  = (1  2), /* device asserts INTRQ when ready 
 for CDB */
 ATA_DFLAG_NCQ   = (1  3), /* device supports NCQ */
 ATA_DFLAG_FLUSH_EXT = (1  4), /* do FLUSH_EXT instead of FLUSH */
 +   ATA_DFLAG_AN= (1  5), /* device supports Async 
 notification */
 ATA_DFLAG_CFG_MASK  = (1  8) - 1,
 Why don't the macros use the enums? It makes the code hard to read without
 painful cross-reference doesn't it? Surely (id)[76]  (ATA_DFLAG_AN) is a
 lot more readable than 1  5 - even if the flag is obviously that, a lot
 of values and registers can have 1  5 as a flag and mean a lot of different
 things.
 
 It's really just a coincidence that the ATA_DFLAG_AN bit is the same as the 
 bit
 in the identify device word, so this would not be appropriate.

Okay, that makes sense.. I just had a bad day cross-referencing some terrible
code in another project, was in the mood to nit :D

-- 
Matt Sealey [EMAIL PROTECTED]
Genesi, Manager, Developer Relations
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 1/7] libata: check for AN support

2007-04-24 Thread Alan Cox 
 + /*
 +  * check to see if this ATAPI device supports
 +  * Asynchronous Notification
 +  */
 + if ((ap-flags  ATA_FLAG_AN)  ata_id_has_AN(id))
 + {

Bracketing police ^^^

 + /* issue SET feature command to turn this on */
 + rc = ata_dev_set_AN(dev);
 + if (rc) {
 + ata_dev_printk(dev, KERN_ERR,
 + unable to set AN\n);
 + rc = -EINVAL;
 + goto err_out_nosup;

How fatal is this - do we need to ignore the device at this point or
should we just pretend (possibly correctly) that the device itself does
not support notification. 

 @@ -299,6 +305,8 @@ struct ata_taskfile {
  #define ata_id_queue_depth(id)   (((id)[75]  0x1f) + 1)
  #define ata_id_removeable(id)((id)[0]  (1  7))
  #define ata_id_has_dword_io(id)  ((id)[50]  (1  0))
 +#define ata_id_has_AN(id)\
 + ((id[76]  (~id[76]))  ((id)[78]  (1  5)))

Might be nice to check ATA version as well to be paranoid but this all
looks ok as its a reserved field since way back when.

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

Re: [patch 1/7] libata: check for AN support

2007-04-24 Thread Olivier Galibert 
Sorry for replying to Alan's reply, I missed the original mail.

  +#define ata_id_has_AN(id)  \
  +   ((id[76]  (~id[76]))  ((id)[78]  (1  5)))

(a  ~a)  (b  32)

I don't think that does what you think it does, because at that point
it's a funny way to write 0 ((0 or 1) binary-and (0 or 32)).

I'm not even sure what it is you want.  If for the first part you
wanted (id[76] != 0x00  id[76] != 0xff), please write just that,
thanks :-)

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

Re: [patch 1/7] libata: check for AN support

2007-04-24 Thread Kristen Carlson Accardi 
On Tue, 24 Apr 2007 12:23:04 +0200
Olivier Galibert [EMAIL PROTECTED] wrote:

 Sorry for replying to Alan's reply, I missed the original mail.
 
   +#define ata_id_has_AN(id)\
   + ((id[76]  (~id[76]))  ((id)[78]  (1  5)))
 
 (a  ~a)  (b  32)
 
 I don't think that does what you think it does, because at that point
 it's a funny way to write 0 ((0 or 1) binary-and (0 or 32)).
 
 I'm not even sure what it is you want.  If for the first part you
 wanted (id[76] != 0x00  id[76] != 0xff), please write just that,
 thanks :-)
 
   OG.
 

From the serial ata spec, we have:

13.2.1.18Word 78: Serial ATA features supported
If Word 76 is not h or h, Word 78 reports the optional features 
supported by the device.  Support for this word is optional and if not 
supported the word shall be zero indicating the device has no support for new 
Serial ATA capabilities.

so, basically yes, I'm really testing to make sure that word 76 isn't 0 or all
one then using that value  with value of bit in work 78 to determine AN
support - if you think this is really obfuscated, I've got no problem changing 
it - there's obviously many ways to mess around with bits.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 1/7] libata: check for AN support

2007-04-24 Thread Kristen Carlson Accardi 
On Tue, 24 Apr 2007 17:03:29 +0900
Tejun Heo [EMAIL PROTECTED] wrote:

 Hello,
 
 Kristen Carlson Accardi wrote:
   static unsigned int ata_print_id = 1;
  @@ -1744,6 +1745,23 @@ int ata_dev_configure(struct ata_device 
  }
  dev-cdb_len = (unsigned int) rc;
   
  +   /*
  +* check to see if this ATAPI device supports
  +* Asynchronous Notification
  +*/
  +   if ((ap-flags  ATA_FLAG_AN)  ata_id_has_AN(id))
  +   {
  +   /* issue SET feature command to turn this on */
  +   rc = ata_dev_set_AN(dev);
 
 Please don't store err_mask into int rc.  Please store it to a separate
 err_mask variable and report it when printing error message.
 
  +   if (rc) {
  +   ata_dev_printk(dev, KERN_ERR,
  +   unable to set AN\n);
  +   rc = -EINVAL;
 
 Wouldn't -EIO be more appropriate?

I think Alan is right - and being unable to turn on AN should not be fatal.
I'll just change all this code to just print the err and keep going.

 
  +   goto err_out_nosup;
  +   }
  +   dev-flags |= ATA_DFLAG_AN;
  +   }
  +
 
 Not NACKing.  Just notes for future improvements.  We need to be more
 careful here.  ATA/ATAPI world is filled with braindamaged devices and I
 bet there are devices which advertises it can do AN but chokes when AN
 is enabled.
 
 This should be handled similarly to ACPI failure.  Currently ACPI does
 the following.
 
 1. try once, if fail, record that ACPI failed.  return error to trigger
 retry.
 2. try again, if fail again, ignore error if possible (!FROZEN) and turn
 off ACPI.
 
 This fallback mechanism for optional features can probably be
 generalized and used for both ACPI and AN.

Ok - meanwhile I think it's appropriate here to just do try-once-fail-give-up.
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 1/7] libata: check for AN support

2007-04-24 Thread Olivier Galibert 
On Tue, Apr 24, 2007 at 08:49:04AM -0700, Kristen Carlson Accardi wrote:
 On Tue, 24 Apr 2007 12:23:04 +0200
 Olivier Galibert [EMAIL PROTECTED] wrote:
 
  Sorry for replying to Alan's reply, I missed the original mail.
  
+#define ata_id_has_AN(id)  \
+   ((id[76]  (~id[76]))  ((id)[78]  (1  5)))
  
  (a  ~a)  (b  32)
  
  I don't think that does what you think it does, because at that point
  it's a funny way to write 0 ((0 or 1) binary-and (0 or 32)).
  
  I'm not even sure what it is you want.  If for the first part you
  wanted (id[76] != 0x00  id[76] != 0xff), please write just that,
  thanks :-)
  
OG.
  
 
 From the serial ata spec, we have:
 
 13.2.1.18Word 78: Serial ATA features supported
 If Word 76 is not h or h, Word 78 reports the optional features 
 supported by the device.  Support for this word is optional and if not 
 supported the word shall be zero indicating the device has no support for new 
 Serial ATA capabilities.
 
 so, basically yes, I'm really testing to make sure that word 76 isn't 0 or all
 one then using that value  with value of bit in work 78 to determine AN
 support - if you think this is really obfuscated, I've got no problem 
 changing 
 it - there's obviously many ways to mess around with bits.

 is not , so right now it's really incorrect.  1  32 is 0.

((id)[76] != 0x  (id)[76] != 0x  ((id)[78]  (1  5)))

The implicit typing of id looks dangerous to me, but you're not the
one who has started it.

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

Re: [patch 1/7] libata: check for AN support

2007-04-24 Thread Kristen Carlson Accardi 
On Tue, 24 Apr 2007 20:05:52 +0200
Olivier Galibert [EMAIL PROTECTED] wrote:

 On Tue, Apr 24, 2007 at 08:49:04AM -0700, Kristen Carlson Accardi wrote:
  On Tue, 24 Apr 2007 12:23:04 +0200
  Olivier Galibert [EMAIL PROTECTED] wrote:
  
   Sorry for replying to Alan's reply, I missed the original mail.
   
 +#define ata_id_has_AN(id)\
 + ((id[76]  (~id[76]))  ((id)[78]  (1  5)))
   
   (a  ~a)  (b  32)
   
   I don't think that does what you think it does, because at that point
   it's a funny way to write 0 ((0 or 1) binary-and (0 or 32)).
   
   I'm not even sure what it is you want.  If for the first part you
   wanted (id[76] != 0x00  id[76] != 0xff), please write just that,
   thanks :-)
   
 OG.
   
  
  From the serial ata spec, we have:
  
  13.2.1.18Word 78: Serial ATA features supported
  If Word 76 is not h or h, Word 78 reports the optional features 
  supported by the device.  Support for this word is optional and if not 
  supported the word shall be zero indicating the device has no support for 
  new 
  Serial ATA capabilities.
  
  so, basically yes, I'm really testing to make sure that word 76 isn't 0 or 
  all
  one then using that value  with value of bit in work 78 to determine AN
  support - if you think this is really obfuscated, I've got no problem 
  changing 
  it - there's obviously many ways to mess around with bits.
 
  is not , so right now it's really incorrect.  1  32 is 0.

ah - ok, gotcha, thanks.

 
 ((id)[76] != 0x  (id)[76] != 0x  ((id)[78]  (1  5)))
 
 The implicit typing of id looks dangerous to me, but you're not the
 one who has started it.
 
   OG.
 
-
To unsubscribe from this list: send the line unsubscribe linux-scsi in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 1/7] libata: check for AN support

2007-04-24 Thread Kristen Carlson Accardi 
Check to see if an ATAPI device supports Asynchronous Notification.
If so, enable it.

changes from last version: 
* fix typo in ata_id_has_AN and make word 76 test more clear
* If we fail to set the AN feature, just print a warning and continue
 
Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED]

Index: 2.6-git/drivers/ata/libata-core.c
===
--- 2.6-git.orig/drivers/ata/libata-core.c
+++ 2.6-git/drivers/ata/libata-core.c
@@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long
 static unsigned int ata_dev_init_params(struct ata_device *dev,
u16 heads, u16 sectors);
 static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
+static unsigned int ata_dev_set_AN(struct ata_device *dev);
 static void ata_dev_xfermask(struct ata_device *dev);
 
 static unsigned int ata_print_id = 1;
@@ -1744,6 +1745,22 @@ int ata_dev_configure(struct ata_device 
}
dev-cdb_len = (unsigned int) rc;
 
+   /*
+* check to see if this ATAPI device supports
+* Asynchronous Notification
+*/
+   if ((ap-flags  ATA_FLAG_AN)  ata_id_has_AN(id)) {
+   int err;
+   /* issue SET feature command to turn this on */
+   err = ata_dev_set_AN(dev);
+   if (err)
+   ata_dev_printk(dev, KERN_ERR,
+   unable to set AN, err %x\n,
+   err);
+   else
+   dev-flags |= ATA_DFLAG_AN;
+   }
+
if (ata_id_cdb_intr(dev-id)) {
dev-flags |= ATA_DFLAG_CDB_INTR;
cdb_intr_string = , CDB intr;
@@ -3525,6 +3542,42 @@ static unsigned int ata_dev_set_xfermode
 }
 
 /**
+ * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
+ *   with sector count set to indicate
+ *   Asynchronous Notification feature
+ * @dev: Device to which command will be sent
+ *
+ * Issue SET FEATURES - SATA FEATURES command to device @dev
+ * on port @ap.
+ *
+ * LOCKING:
+ * PCI/etc. bus probe sem.
+ *
+ * RETURNS:
+ * 0 on success, AC_ERR_* mask otherwise.
+ */
+static unsigned int ata_dev_set_AN(struct ata_device *dev)
+{
+   struct ata_taskfile tf;
+   unsigned int err_mask;
+
+   /* set up set-features taskfile */
+   DPRINTK(set features - SATA features\n);
+
+   ata_tf_init(dev, tf);
+   tf.command = ATA_CMD_SET_FEATURES;
+   tf.feature = SETFEATURES_SATA_ENABLE;
+   tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+   tf.protocol = ATA_PROT_NODATA;
+   tf.nsect = SATA_AN;
+
+   err_mask = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0);
+
+   DPRINTK(EXIT, err_mask=%x\n, err_mask);
+   return err_mask;
+}
+
+/**
  * ata_dev_init_params - Issue INIT DEV PARAMS command
  * @dev: Device to which command will be sent
  * @heads: Number of heads (taskfile parameter)
Index: 2.6-git/include/linux/ata.h
===
--- 2.6-git.orig/include/linux/ata.h
+++ 2.6-git/include/linux/ata.h
@@ -194,6 +194,12 @@ enum {
SETFEATURES_WC_ON   = 0x02, /* Enable write cache */
SETFEATURES_WC_OFF  = 0x82, /* Disable write cache */
 
+   SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */
+   SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */
+
+   /* SETFEATURE Sector counts for SATA features */
+   SATA_AN = 0x05,  /* Asynchronous Notification */
+
/* ATAPI stuff */
ATAPI_PKT_DMA   = (1  0),
ATAPI_DMADIR= (1  2), /* ATAPI data dir:
@@ -299,6 +305,8 @@ struct ata_taskfile {
 #define ata_id_queue_depth(id) (((id)[75]  0x1f) + 1)
 #define ata_id_removeable(id)  ((id)[0]  (1  7))
 #define ata_id_has_dword_io(id)((id)[50]  (1  0))
+#define ata_id_has_AN(id)  \
+   (((id[76] != 0x)  (id[76] != 0x))  ((id)[78]  (1  5)))
 #define ata_id_iordy_disable(id) ((id)[49]  (1  10))
 #define ata_id_has_iordy(id) ((id)[49]  (1  9))
 #define ata_id_u32(id,n)   \
Index: 2.6-git/include/linux/libata.h
===
--- 2.6-git.orig/include/linux/libata.h
+++ 2.6-git/include/linux/libata.h
@@ -136,6 +136,7 @@ enum {
ATA_DFLAG_CDB_INTR  = (1  2), /* device asserts INTRQ when ready 
for CDB */
ATA_DFLAG_NCQ   = (1  3), /* device supports NCQ */
ATA_DFLAG_FLUSH_EXT = (1  4), /* do FLUSH_EXT instead of FLUSH */
+   ATA_DFLAG_AN= (1  5), /* device supports Async 
notification */
ATA_DFLAG_CFG_MASK  = (1  8) - 1,

[patch 1/7] libata: check for AN support

2007-04-23 Thread Kristen Carlson Accardi 
Check to see if an ATAPI device supports Asynchronous Notification.
If so, enable it.

Signed-off-by: Kristen Carlson Accardi [EMAIL PROTECTED]

Index: 2.6-git/drivers/ata/libata-core.c
===
--- 2.6-git.orig/drivers/ata/libata-core.c
+++ 2.6-git/drivers/ata/libata-core.c
@@ -70,6 +70,7 @@ const unsigned long sata_deb_timing_long
 static unsigned int ata_dev_init_params(struct ata_device *dev,
u16 heads, u16 sectors);
 static unsigned int ata_dev_set_xfermode(struct ata_device *dev);
+static unsigned int ata_dev_set_AN(struct ata_device *dev);
 static void ata_dev_xfermask(struct ata_device *dev);
 
 static unsigned int ata_print_id = 1;
@@ -1744,6 +1745,23 @@ int ata_dev_configure(struct ata_device 
}
dev-cdb_len = (unsigned int) rc;
 
+   /*
+* check to see if this ATAPI device supports
+* Asynchronous Notification
+*/
+   if ((ap-flags  ATA_FLAG_AN)  ata_id_has_AN(id))
+   {
+   /* issue SET feature command to turn this on */
+   rc = ata_dev_set_AN(dev);
+   if (rc) {
+   ata_dev_printk(dev, KERN_ERR,
+   unable to set AN\n);
+   rc = -EINVAL;
+   goto err_out_nosup;
+   }
+   dev-flags |= ATA_DFLAG_AN;
+   }
+
if (ata_id_cdb_intr(dev-id)) {
dev-flags |= ATA_DFLAG_CDB_INTR;
cdb_intr_string = , CDB intr;
@@ -3525,6 +3543,42 @@ static unsigned int ata_dev_set_xfermode
 }
 
 /**
+ * ata_dev_set_AN - Issue SET FEATURES - SATA FEATURES
+ *   with sector count set to indicate
+ *   Asynchronous Notification feature
+ * @dev: Device to which command will be sent
+ *
+ * Issue SET FEATURES - SATA FEATURES command to device @dev
+ * on port @ap.
+ *
+ * LOCKING:
+ * PCI/etc. bus probe sem.
+ *
+ * RETURNS:
+ * 0 on success, AC_ERR_* mask otherwise.
+ */
+static unsigned int ata_dev_set_AN(struct ata_device *dev)
+{
+   struct ata_taskfile tf;
+   unsigned int err_mask;
+
+   /* set up set-features taskfile */
+   DPRINTK(set features - SATA features\n);
+
+   ata_tf_init(dev, tf);
+   tf.command = ATA_CMD_SET_FEATURES;
+   tf.feature = SETFEATURES_SATA_ENABLE;
+   tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;
+   tf.protocol = ATA_PROT_NODATA;
+   tf.nsect = SATA_AN;
+
+   err_mask = ata_exec_internal(dev, tf, NULL, DMA_NONE, NULL, 0);
+
+   DPRINTK(EXIT, err_mask=%x\n, err_mask);
+   return err_mask;
+}
+
+/**
  * ata_dev_init_params - Issue INIT DEV PARAMS command
  * @dev: Device to which command will be sent
  * @heads: Number of heads (taskfile parameter)
Index: 2.6-git/include/linux/ata.h
===
--- 2.6-git.orig/include/linux/ata.h
+++ 2.6-git/include/linux/ata.h
@@ -194,6 +194,12 @@ enum {
SETFEATURES_WC_ON   = 0x02, /* Enable write cache */
SETFEATURES_WC_OFF  = 0x82, /* Disable write cache */
 
+   SETFEATURES_SATA_ENABLE = 0x10, /* Enable use of SATA feature */
+   SETFEATURES_SATA_DISABLE = 0x90, /* Disable use of SATA feature */
+
+   /* SETFEATURE Sector counts for SATA features */
+   SATA_AN = 0x05,  /* Asynchronous Notification */
+
/* ATAPI stuff */
ATAPI_PKT_DMA   = (1  0),
ATAPI_DMADIR= (1  2), /* ATAPI data dir:
@@ -299,6 +305,8 @@ struct ata_taskfile {
 #define ata_id_queue_depth(id) (((id)[75]  0x1f) + 1)
 #define ata_id_removeable(id)  ((id)[0]  (1  7))
 #define ata_id_has_dword_io(id)((id)[50]  (1  0))
+#define ata_id_has_AN(id)  \
+   ((id[76]  (~id[76]))  ((id)[78]  (1  5)))
 #define ata_id_iordy_disable(id) ((id)[49]  (1  10))
 #define ata_id_has_iordy(id) ((id)[49]  (1  9))
 #define ata_id_u32(id,n)   \
Index: 2.6-git/include/linux/libata.h
===
--- 2.6-git.orig/include/linux/libata.h
+++ 2.6-git/include/linux/libata.h
@@ -136,6 +136,7 @@ enum {
ATA_DFLAG_CDB_INTR  = (1  2), /* device asserts INTRQ when ready 
for CDB */
ATA_DFLAG_NCQ   = (1  3), /* device supports NCQ */
ATA_DFLAG_FLUSH_EXT = (1  4), /* do FLUSH_EXT instead of FLUSH */
+   ATA_DFLAG_AN= (1  5), /* device supports Async 
notification */
ATA_DFLAG_CFG_MASK  = (1  8) - 1,
 
ATA_DFLAG_PIO   = (1  8), /* device limited to PIO mode */
@@ -174,6 +175,7 @@ enum {
ATA_FLAG_SETXFER_POLLING= (1  14), /* use polling for SETXFER */