[PATCH v2 1/5] add scsi_build_sense_buffer helper function

[PATCH v2 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 |   28 ++++++++++++++++++++++++++++
 include/scsi/scsi_eh.h    |    4 +++-
 2 files changed, 31 insertions(+), 1 deletions(-)

diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
index 1221d2c..221f31e 100644
--- a/drivers/scsi/scsi_error.c
+++ b/drivers/scsi/scsi_error.c
@@ -1993,3 +1993,31 @@ 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
+ *
+ **/
+void scsi_build_sense_buffer(int desc, u8 *buf, u8 key, u8 asc, u8 ascq)
+{
+	if (desc) {
+		buf[0] = 0x72;	/* descriptor, current */
+		buf[1] = key;
+		buf[2] = asc;
+		buf[3] = ascq;
+		buf[7] = 0;
+	} else {
+		buf[0] = 0x70;	/* fixed, current */
+		buf[2] = key;
+		buf[7] = 0xa;
+		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..d3a133b 100644
--- a/include/scsi/scsi_eh.h
+++ b/include/scsi/scsi_eh.h
@@ -57,7 +57,9 @@ 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);
+
 /*
  * Reset request from external source
  */
-- 
1.5.3.7

[PATCH v2 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 |   18 +++++-------------
 1 files changed, 5 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index 4f4c5b7..5f348e7 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,13 @@ 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);
+
 	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 v2 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..90a809a 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);
 }
 
 /**
-- 
1.5.3.7

[PATCH v2 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 |   17 +++++------------
 1 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c
index 8c7b183..f308a03 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"
@@ -362,22 +363,14 @@ static struct status_msg *stex_get_status(struct st_hba *hba)
 	return status;
 }
 
-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;
-}
-
 static void stex_invalid_field(struct scsi_cmnd *cmd,
 			       void (*done)(struct scsi_cmnd *))
 {
+	cmd->result = (DRIVER_SENSE << 24) | SAM_STAT_CHECK_CONDITION;
+
 	/* "Invalid field in cbd" */
-	stex_set_sense(cmd, ILLEGAL_REQUEST, 0x24, 0x0);
+	scsi_build_sense_buffer(0, cmd->sense_buffer, ILLEGAL_REQUEST, 0x24,
+				0x0);
 	done(cmd);
 }
 
-- 
1.5.3.7

[PATCH v2 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..ce48e2d 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);
 	cmd->result = SAM_STAT_CHECK_CONDITION;
 
 done:
-- 
1.5.3.7

Re: [PATCH v2 5/5] ps3rom: use scsi_build_sense_buffer

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1769 bytes --]

On Tue, 25 Mar 2008, FUJITA Tomonori wrote:
> Signed-off-by: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
> Cc: Geert Uytterhoeven <Geert.Uytterhoeven@sonycom.com>

Acked-by: 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..ce48e2d 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);
>  	cmd->result = SAM_STAT_CHECK_CONDITION;
>  
>  done:
> -- 
> 1.5.3.7

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

RE: [PATCH v2 4/5] stex: use scsi_build_sense_buffer

[Ed Lin]

>-----Original Message-----
>From: fujita [mailto:tomof@acm.org] On Behalf Of FUJITA Tomonori
>Sent: Monday, March 24, 2008 5:27 PM
>To: linux-scsi@vger.kernel.org
>Cc: tomof@acm.org; FUJITA Tomonori; Ed Lin; James Bottomley
>Subject: [PATCH v2 4/5] stex: use scsi_build_sense_buffer
>
>
>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 |   17 +++++------------
> 1 files changed, 5 insertions(+), 12 deletions(-)
>
>diff --git a/drivers/scsi/stex.c b/drivers/scsi/stex.c
>index 8c7b183..f308a03 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"
>@@ -362,22 +363,14 @@ static struct status_msg 
>*stex_get_status(struct st_hba *hba)
> 	return status;
> }
> 
>-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;
>-}
>-
> static void stex_invalid_field(struct scsi_cmnd *cmd,
> 			       void (*done)(struct scsi_cmnd *))
> {
>+	cmd->result = (DRIVER_SENSE << 24) | SAM_STAT_CHECK_CONDITION;
>+
> 	/* "Invalid field in cbd" */
>-	stex_set_sense(cmd, ILLEGAL_REQUEST, 0x24, 0x0);
>+	scsi_build_sense_buffer(0, cmd->sense_buffer, 
>ILLEGAL_REQUEST, 0x24,
>+				0x0);
> 	done(cmd);
> }
> 
>-- 
>1.5.3.7
>
>

ACK patch 4.

Ed Lin

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

[Boaz Harrosh]

On Tue, Mar 25 2008 at 2:26 +0200, FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> 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 |   28 ++++++++++++++++++++++++++++
>  include/scsi/scsi_eh.h    |    4 +++-
>  2 files changed, 31 insertions(+), 1 deletions(-)
> 
> diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> index 1221d2c..221f31e 100644
> --- a/drivers/scsi/scsi_error.c
> +++ b/drivers/scsi/scsi_error.c
> @@ -1993,3 +1993,31 @@ 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
> + *
> + **/
> +void scsi_build_sense_buffer(int desc, u8 *buf, u8 key, u8 asc, u8 ascq)
> +{
> +	if (desc) {
> +		buf[0] = 0x72;	/* descriptor, current */
> +		buf[1] = key;
> +		buf[2] = asc;
> +		buf[3] = ascq;
> +		buf[7] = 0;
> +	} else {
> +		buf[0] = 0x70;	/* fixed, current */
> +		buf[2] = key;
> +		buf[7] = 0xa;
> +		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..d3a133b 100644
> --- a/include/scsi/scsi_eh.h
> +++ b/include/scsi/scsi_eh.h
> @@ -57,7 +57,9 @@ 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);
> +
>  /*
>   * Reset request from external source
>   */
Tomo Hi.

Thanks for doing this. It was requested before by few people.

I have one small request please? This is to help me  with my "sense" effort.
(See here: http://git.open-osd.org/gitweb.cgi?p=open-osd.git;a=shortlog;h=boaz_sense_effort)
Could you change it that a struct scsi_cmnd * is passed to the new API, instead of the
u8 * directly. This is because I'm removing the buffer at scsi_cmnd. Something like:

extern void scsi_build_sense_buffer(struct scsi_cmnd *, int desc, u8 key, u8 asc, u8 ascq);

If you do this then my patches get that much smaller.
Thanks
Boaz
  

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

[FUJITA Tomonori]

On Tue, 25 Mar 2008 11:45:27 +0200
Boaz Harrosh <bharrosh@panasas.com> wrote:

> On Tue, Mar 25 2008 at 2:26 +0200, FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> 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 |   28 ++++++++++++++++++++++++++++
> >  include/scsi/scsi_eh.h    |    4 +++-
> >  2 files changed, 31 insertions(+), 1 deletions(-)
> > 
> > diff --git a/drivers/scsi/scsi_error.c b/drivers/scsi/scsi_error.c
> > index 1221d2c..221f31e 100644
> > --- a/drivers/scsi/scsi_error.c
> > +++ b/drivers/scsi/scsi_error.c
> > @@ -1993,3 +1993,31 @@ 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
> > + *
> > + **/
> > +void scsi_build_sense_buffer(int desc, u8 *buf, u8 key, u8 asc, u8 ascq)
> > +{
> > +	if (desc) {
> > +		buf[0] = 0x72;	/* descriptor, current */
> > +		buf[1] = key;
> > +		buf[2] = asc;
> > +		buf[3] = ascq;
> > +		buf[7] = 0;
> > +	} else {
> > +		buf[0] = 0x70;	/* fixed, current */
> > +		buf[2] = key;
> > +		buf[7] = 0xa;
> > +		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..d3a133b 100644
> > --- a/include/scsi/scsi_eh.h
> > +++ b/include/scsi/scsi_eh.h
> > @@ -57,7 +57,9 @@ 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);
> > +
> >  /*
> >   * Reset request from external source
> >   */
> Tomo Hi.
> 
> Thanks for doing this. It was requested before by few people.
> 
> I have one small request please? This is to help me  with my "sense" effort.
> (See here: http://git.open-osd.org/gitweb.cgi?p=open-osd.git;a=shortlog;h=boaz_sense_effort)
> Could you change it that a struct scsi_cmnd * is passed to the new API, instead of the
> u8 * directly. This is because I'm removing the buffer at scsi_cmnd. Something like:
> 
> extern void scsi_build_sense_buffer(struct scsi_cmnd *, int desc, u8 key, u8 asc, u8 ascq);

Unfortunately, it doesn't work for scsi_debug (doesn't build sense
data in scsi_cmnd).

I guess, later on you could change something like:

extern void __scsi_build_sense_buffer(int desc, u8 *buf, u8 key, u8 asc, u8 ascq);

void scsi_build_sense_buffer(struct scsi_cmnd *cmd, int desc, u8 key, u8 asc, u8 ascq)
{
	__scsi_build_sense_buffer(desc, cmd->sense_buffer, desc, key, asc, ascq);
}