[PATCH 0/5] sweep up functions to build sense data

[PATCH 0/5] sweep up functions to build sense data

[FUJITA Tomonori]

We have several similar functions to build sense data. This patchset
just adds a simple helper function to do the same job and sweeps them
up.

[PATCH 1/5] add scsi_build_sense_buffer helper function

[FUJITA Tomonori]

This adds scsi_build_sense_buffer, a simple helper function to build
sense data in a buffer.

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/scsi/scsi_error.c |   30 ++++++++++++++++++++++++++++++
 include/scsi/scsi_eh.h    |    5 ++++-
 2 files changed, 34 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 1221d2c..85add5b 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1993,3 +1993,33 @@ int scsi_get_sense_info_fld(const u8 * sense_buffer, int sb_len,
 	}
 }
 EXPORT_SYMBOL(scsi_get_sense_info_fld);
+
+/**
+ * scsi_build_sense_buffer - build sense data in a buffer
+ * @desc:	Sense format (non zero == descriptor format,
+ * 		0 == fixed format
+ * @buf:	Where to build sense data
+ * @key:	Sense key
+ * @asc:	Additional sense code
+ * @ascq:	Additional sense code qualifier
+ * @addlen:	Additional sense length
+ *
+ **/
+void scsi_build_sense_buffer(int desc, u8 *buf, u8 key, u8 asc, u8 ascq,
+			     int addlen)
+{
+	if (desc) {
+		buf[0] = 0x72;	/* descriptor, current */
+		buf[1] = key;
+		buf[2] = asc;
+		buf[3] = ascq;
+		buf[7] = addlen;
+	} else {
+		buf[0] = 0x70;	/* fixed, current */
+		buf[2] = key;
+		buf[7] = addlen;
+		buf[12] = asc;
+		buf[13] = ascq;
+	}
+}
+EXPORT_SYMBOL(scsi_build_sense_buffer);
diff --git a/include/scsi/scsi_eh.h b/include/scsi/scsi_eh.h
index 37a7614..0d0db78 100644
--- a/include/scsi/scsi_eh.h
+++ b/include/scsi/scsi_eh.h
@@ -57,7 +57,10 @@ extern const u8 * scsi_sense_desc_find(const u8 * sense_buffer, int sb_len,
 
 extern int scsi_get_sense_info_fld(const u8 * sense_buffer, int sb_len,
 				   u64 * info_out);
- 
+
+extern void scsi_build_sense_buffer(int desc, u8 *buf, u8 key, u8 asc, u8 ascq,
+				    int addlen);
+
 /*
  * Reset request from external source
  */
-- 
1.5.3.7

[PATCH 2/5] scsi_debug: use scsi_build_sense_buffer

[FUJITA Tomonori]

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Douglas Gilbert <dougg@torque.net>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/scsi/scsi_debug.c |   19 ++++++-------------
 1 files changed, 6 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 4f4c5b7..68bd097 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -46,6 +46,7 @@
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_host.h>
 #include <scsi/scsicam.h>
+#include <scsi/scsi_eh.h>
 
 #include <linux/stat.h>
 
@@ -1808,22 +1809,14 @@ static struct sdebug_dev_info * devInfoReg(struct scsi_device * sdev)
 static void mk_sense_buffer(struct sdebug_dev_info * devip, int key,
 			    int asc, int asq)
 {
-	unsigned char * sbuff;
+	unsigned char *sbuff;
 
 	sbuff = devip->sense_buff;
 	memset(sbuff, 0, SDEBUG_SENSE_LEN);
-	if (scsi_debug_dsense) {
-		sbuff[0] = 0x72;  /* descriptor, current */
-		sbuff[1] = key;
-		sbuff[2] = asc;
-		sbuff[3] = asq;
-	} else {
-		sbuff[0] = 0x70;  /* fixed, current */
-		sbuff[2] = key;
-		sbuff[7] = 0xa;	  /* implies 18 byte sense buffer */
-		sbuff[12] = asc;
-		sbuff[13] = asq;
-	}
+
+	scsi_build_sense_buffer(scsi_debug_dsense, sbuff, key, asc, asq,
+				scsi_debug_dsense ? 0 : 0xa);
+
 	if (SCSI_DEBUG_OPT_NOISE & scsi_debug_opts)
 		printk(KERN_INFO "scsi_debug:    [sense_key,asc,ascq]: "
 		      "[0x%x,0x%x,0x%x]\n", key, asc, asq);
-- 
1.5.3.7

[PATCH 3/5] libata: use scsi_build_sense_buffer

[FUJITA Tomonori]

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Jeff Garzik <jeff@garzik.org>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/ata/libata-scsi.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 8f0e8f2..5917a5d 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -2324,11 +2324,7 @@ void ata_scsi_set_sense(struct scsi_cmnd *cmd, u8 sk, u8 asc, u8 ascq)
 {
 	cmd->result = (DRIVER_SENSE << 24) | SAM_STAT_CHECK_CONDITION;
 
-	cmd->sense_buffer[0] = 0x70;	/* fixed format, current */
-	cmd->sense_buffer[2] = sk;
-	cmd->sense_buffer[7] = 18 - 8;	/* additional sense length */
-	cmd->sense_buffer[12] = asc;
-	cmd->sense_buffer[13] = ascq;
+	scsi_build_sense_buffer(0, cmd->sense_buffer, sk, asc, ascq, 10);
 }
 
 /**
-- 
1.5.3.7

[PATCH 4/5] stex: use scsi_build_sense_buffer

[FUJITA Tomonori]

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Ed Lin <ed.lin@promise.com>
Cc: James Bottomley <James.Bottomley@SteelEye.com>
---
 drivers/scsi/stex.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c
index 8c7b183..9ef47fe 100644
--- a/drivers/scsi/stex.c
+++ b/drivers/scsi/stex.c
@@ -33,6 +33,7 @@
 #include <scsi/scsi_host.h>
 #include <scsi/scsi_tcq.h>
 #include <scsi/scsi_dbg.h>
+#include <scsi/scsi_eh.h>
 
 #define DRV_NAME "stex"
 #define ST_DRIVER_VERSION "3.6.0000.1"
@@ -366,11 +367,7 @@ static void stex_set_sense(struct scsi_cmnd *cmd, u8 sk, u8 asc, u8 ascq)
 {
 	cmd->result = (DRIVER_SENSE << 24) | SAM_STAT_CHECK_CONDITION;
 
-	cmd->sense_buffer[0] = 0x70;    /* fixed format, current */
-	cmd->sense_buffer[2] = sk;
-	cmd->sense_buffer[7] = 18 - 8;  /* additional sense length */
-	cmd->sense_buffer[12] = asc;
-	cmd->sense_buffer[13] = ascq;
+	scsi_build_sense_buffer(0, cmd->sense_buffer, sk, asc, ascq, 10);
 }
 
 static void stex_invalid_field(struct scsi_cmnd *cmd,
-- 
1.5.3.7

[PATCH 5/5] ps3rom: use scsi_build_sense_buffer

[FUJITA Tomonori]

Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
Cc: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>
Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
---
 drivers/scsi/ps3rom.c |    7 ++-----
 1 files changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/ps3rom.c b/drivers/scsi/ps3rom.c
index d1e7845..150e5cd 100644
--- a/drivers/scsi/ps3rom.c
+++ b/drivers/scsi/ps3rom.c
@@ -26,6 +26,7 @@
 #include <scsi/scsi_dbg.h>
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_host.h>
+#include <scsi/scsi_eh.h>
 
 #include <asm/lv1call.h>
 #include <asm/ps3stor.h>
@@ -330,11 +331,7 @@ static irqreturn_t ps3rom_interrupt(int irq, void *data)
 		goto done;
 	}
 
-	cmd->sense_buffer[0]  = 0x70;
-	cmd->sense_buffer[2]  = sense_key;
-	cmd->sense_buffer[7]  = 16 - 6;
-	cmd->sense_buffer[12] = asc;
-	cmd->sense_buffer[13] = ascq;
+	scsi_build_sense_buffer(0, cmd->sense_buffer, sense_key, asc, ascq, 10);
 	cmd->result = SAM_STAT_CHECK_CONDITION;
 
 done:
-- 
1.5.3.7

Re: [PATCH 1/5] add scsi_build_sense_buffer helper function

[James Bottomley]

On Tue, 2008-03-25 at 01:54 +0900, FUJITA Tomonori wrote:
> This adds scsi_build_sense_buffer, a simple helper function to build
> sense data in a buffer.
> 
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> ---
>  drivers/scsi/scsi_error.c |   30 ++++++++++++++++++++++++++++++
>  include/scsi/scsi_eh.h    |    5 ++++-
>  2 files changed, 34 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 1221d2c..85add5b 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -1993,3 +1993,33 @@ int scsi_get_sense_info_fld(const u8 * sense_buffer, int sb_len,
>  	}
>  }
>  EXPORT_SYMBOL(scsi_get_sense_info_fld);
> +
> +/**
> + * scsi_build_sense_buffer - build sense data in a buffer
> + * @desc:	Sense format (non zero == descriptor format,
> + * 		0 == fixed format
> + * @buf:	Where to build sense data
> + * @key:	Sense key
> + * @asc:	Additional sense code
> + * @ascq:	Additional sense code qualifier
> + * @addlen:	Additional sense length
> + *
> + **/
> +void scsi_build_sense_buffer(int desc, u8 *buf, u8 key, u8 asc, u8 ascq,
> +			     int addlen)
> +{
> +	if (desc) {
> +		buf[0] = 0x72;	/* descriptor, current */
> +		buf[1] = key;
> +		buf[2] = asc;
> +		buf[3] = ascq;
> +		buf[7] = addlen;
> +	} else {
> +		buf[0] = 0x70;	/* fixed, current */
> +		buf[2] = key;
> +		buf[7] = addlen;
> +		buf[12] = asc;
> +		buf[13] = ascq;
> +	}

This doesn't look quite right ... if you're using this call to
manufacture sense data, you're always doing it at a fixed size, aren't
you (i.e. addlen is always 0 for descriptor format and 10 for fixed
format, isn't it)?  So we should just hard code that rather than
requiring it to be passed in.

Also, the libata piece needs to go to linux-ide@vger.kernel.org as well.

James

Re: [PATCH 1/5] add scsi_build_sense_buffer helper function

[FUJITA Tomonori]

On Mon, 24 Mar 2008 12:10:09 -0500
James Bottomley <James.Bottomley@HansenPartnership.com> wrote:

> On Tue, 2008-03-25 at 01:54 +0900, FUJITA Tomonori wrote:
> > This adds scsi_build_sense_buffer, a simple helper function to build
> > sense data in a buffer.
> > 
> > Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> > Cc: James Bottomley <James.Bottomley@HansenPartnership.com>
> > ---
> >  drivers/scsi/scsi_error.c |   30 ++++++++++++++++++++++++++++++
> >  include/scsi/scsi_eh.h    |    5 ++++-
> >  2 files changed, 34 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> > index 1221d2c..85add5b 100644
> > --- a/drivers/scsi/scsi_error.c
> > +++ b/drivers/scsi/scsi_error.c
> > @@ -1993,3 +1993,33 @@ int scsi_get_sense_info_fld(const u8 * sense_buffer, int sb_len,
> >  	}
> >  }
> >  EXPORT_SYMBOL(scsi_get_sense_info_fld);
> > +
> > +/**
> > + * scsi_build_sense_buffer - build sense data in a buffer
> > + * @desc:	Sense format (non zero == descriptor format,
> > + * 		0 == fixed format
> > + * @buf:	Where to build sense data
> > + * @key:	Sense key
> > + * @asc:	Additional sense code
> > + * @ascq:	Additional sense code qualifier
> > + * @addlen:	Additional sense length
> > + *
> > + **/
> > +void scsi_build_sense_buffer(int desc, u8 *buf, u8 key, u8 asc, u8 ascq,
> > +			     int addlen)
> > +{
> > +	if (desc) {
> > +		buf[0] = 0x72;	/* descriptor, current */
> > +		buf[1] = key;
> > +		buf[2] = asc;
> > +		buf[3] = ascq;
> > +		buf[7] = addlen;
> > +	} else {
> > +		buf[0] = 0x70;	/* fixed, current */
> > +		buf[2] = key;
> > +		buf[7] = addlen;
> > +		buf[12] = asc;
> > +		buf[13] = ascq;
> > +	}
> 
> This doesn't look quite right ... if you're using this call to
> manufacture sense data, you're always doing it at a fixed size, aren't
> you (i.e. addlen is always 0 for descriptor format and 10 for fixed
> format, isn't it)?  So we should just hard code that rather than
> requiring it to be passed in.

Fixed. I just thought that someone (likely scsi_debug) would build
more complicated sense data but probably nobody will do that (and LLDs
can overwrite the value if necessary).


> Also, the libata piece needs to go to linux-ide@vger.kernel.org as well.

I CC'ed this time.