[PATCH 4/24][RFC] firewire & ieee1394: Simple convert to new scsi_eh_cpy_sense.

[PATCH 4/24][RFC] firewire & ieee1394: Simple convert to new scsi_eh_cpy_sense.

[Boaz Harrosh]

  Abstract away scsi_cmnd->sense_buffer for later removal.

Signed-off-by: Boaz Harrosh <bharrosh@panasas.com>
---
 drivers/firewire/fw-sbp2.c |    7 +++++--
 drivers/ieee1394/sbp2.c    |    9 +++++++--
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/firewire/fw-sbp2.c b/drivers/firewire/fw-sbp2.c
index 1d9602b..0404650 100644
--- a/drivers/firewire/fw-sbp2.c
+++ b/drivers/firewire/fw-sbp2.c
@@ -46,6 +46,7 @@
 #include <scsi/scsi_cmnd.h>
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_host.h>
+#include <scsi/scsi_eh.h>
 
 #include "fw-transaction.h"
 #include "fw-topology.h"
@@ -1016,8 +1017,9 @@ static struct fw_driver sbp2_driver = {
 };
 
 static unsigned int
-sbp2_status_to_sense_data(u8 *sbp2_status, u8 *sense_data)
+sbp2_status_to_sense_data(u8 *sbp2_status, struct scsi_cmnd *srb)
 {
+	u8 sense_data[16];
 	int sam_status;
 
 	sense_data[0] = 0x70;
@@ -1036,6 +1038,7 @@ sbp2_status_to_sense_data(u8 *sbp2_status, u8 *sense_data)
 	sense_data[13] = sbp2_status[3];
 	sense_data[14] = sbp2_status[12];
 	sense_data[15] = sbp2_status[13];
+	scsi_eh_cpy_sense(srb, sense_data, sizeof(sense_data));
 
 	sam_status = sbp2_status[0] & 0x3f;
 
@@ -1081,7 +1084,7 @@ complete_command_orb(struct sbp2_orb *base_orb, struct sbp2_status *status)
 
 		if (result == DID_OK << 16 && STATUS_GET_LEN(*status) > 1)
 			result = sbp2_status_to_sense_data(STATUS_GET_DATA(*status),
-							   orb->cmd->sense_buffer);
+							   orb->cmd);
 	} else {
 		/*
 		 * If the orb completes with status == NULL, something
diff --git a/drivers/ieee1394/sbp2.c b/drivers/ieee1394/sbp2.c
index 2b889d9..ed54c54 100644
--- a/drivers/ieee1394/sbp2.c
+++ b/drivers/ieee1394/sbp2.c
@@ -89,6 +89,7 @@
 #include <scsi/scsi_dbg.h>
 #include <scsi/scsi_device.h>
 #include <scsi/scsi_host.h>
+#include <scsi/scsi_eh.h>
 
 #include "csr1212.h"
 #include "highlevel.h"
@@ -1672,8 +1673,11 @@ static int sbp2_send_command(struct sbp2_lu *lu, struct scsi_cmnd *SCpnt,
  * Translates SBP-2 status into SCSI sense data for check conditions
  */
 static unsigned int sbp2_status_to_sense_data(unchar *sbp2_status,
-					      unchar *sense_data)
+					      struct scsi_cmnd *SCpnt)
 {
+	u8 sense_data[16];
+
+	memset(sense_data, 0, sizeof(sense_data));
 	/* OK, it's pretty ugly... ;-) */
 	sense_data[0] = 0x70;
 	sense_data[1] = 0x0;
@@ -1691,6 +1695,7 @@ static unsigned int sbp2_status_to_sense_data(unchar *sbp2_status,
 	sense_data[13] = sbp2_status[11];
 	sense_data[14] = sbp2_status[20];
 	sense_data[15] = sbp2_status[21];
+	scsi_eh_cpy_sense(SCpnt, sense_data, sizeof(sense_data));
 
 	return sbp2_status[8] & 0x3f;
 }
@@ -1784,7 +1789,7 @@ static int sbp2_handle_status_write(struct hpsb_host *host, int nodeid,
 
 			if (STATUS_GET_LEN(h) > 1)
 				scsi_status = sbp2_status_to_sense_data(
-					(unchar *)sb, SCpnt->sense_buffer);
+					(unchar *)sb, SCpnt);
 
 			if (STATUS_TEST_DEAD(h))
                                 sbp2_agent_reset(lu, 0);
-- 
1.5.3.3

Re: [PATCH 4/24][RFC] firewire & ieee1394: Simple convert to new scsi_eh_cpy_sense.

[Stefan Richter]

Boaz Harrosh wrote:
> --- a/drivers/ieee1394/sbp2.c
> +++ b/drivers/ieee1394/sbp2.c
> @@ -1672,8 +1673,11 @@ static int sbp2_send_command(struct sbp2_lu *lu, struct scsi_cmnd *SCpnt,
>   * Translates SBP-2 status into SCSI sense data for check conditions
>   */
>  static unsigned int sbp2_status_to_sense_data(unchar *sbp2_status,
> -					      unchar *sense_data)
> +					      struct scsi_cmnd *SCpnt)
>  {
> +	u8 sense_data[16];
> +
> +	memset(sense_data, 0, sizeof(sense_data));
>  	/* OK, it's pretty ugly... ;-) */
>  	sense_data[0] = 0x70;
>  	sense_data[1] = 0x0;
> @@ -1691,6 +1695,7 @@ static unsigned int sbp2_status_to_sense_data(unchar *sbp2_status,
>  	sense_data[13] = sbp2_status[11];
>  	sense_data[14] = sbp2_status[20];
>  	sense_data[15] = sbp2_status[21];
> +	scsi_eh_cpy_sense(SCpnt, sense_data, sizeof(sense_data));
>  
>  	return sbp2_status[8] & 0x3f;
>  }

You don't need the memset.

Also, here and in drivers/firewire/fw-sbp2.c, the SCSI sense data could
AFAICS be rewritten in-place in sbp2_status.  But I don't know if this
is a worthwhile optimization; it would reduce readability.
-- 
Stefan Richter
-=====-==--- --=- --=--
http://arcgraph.de/sr/

Re: [PATCH 4/24][RFC] firewire & ieee1394: Simple convert to new scsi_eh_cpy_sense.

[Boaz Harrosh]

On Mon, Feb 04 2008 at 18:37 +0200, Stefan Richter <stefanr@s5r6.in-berlin.de> wrote:
> Boaz Harrosh wrote:
>> --- a/drivers/ieee1394/sbp2.c
>> +++ b/drivers/ieee1394/sbp2.c
>> @@ -1672,8 +1673,11 @@ static int sbp2_send_command(struct sbp2_lu *lu, struct scsi_cmnd *SCpnt,
>>   * Translates SBP-2 status into SCSI sense data for check conditions
>>   */
>>  static unsigned int sbp2_status_to_sense_data(unchar *sbp2_status,
>> -					      unchar *sense_data)
>> +					      struct scsi_cmnd *SCpnt)
>>  {
>> +	u8 sense_data[16];
>> +
>> +	memset(sense_data, 0, sizeof(sense_data));
>>  	/* OK, it's pretty ugly... ;-) */
>>  	sense_data[0] = 0x70;
>>  	sense_data[1] = 0x0;
>> @@ -1691,6 +1695,7 @@ static unsigned int sbp2_status_to_sense_data(unchar *sbp2_status,
>>  	sense_data[13] = sbp2_status[11];
>>  	sense_data[14] = sbp2_status[20];
>>  	sense_data[15] = sbp2_status[21];
>> +	scsi_eh_cpy_sense(SCpnt, sense_data, sizeof(sense_data));
>>  
>>  	return sbp2_status[8] & 0x3f;
>>  }
> 
> You don't need the memset.
> 
OK I see what you mean now, they are all used. Thanks.

> Also, here and in drivers/firewire/fw-sbp2.c, the SCSI sense data could
> AFAICS be rewritten in-place in sbp2_status.  But I don't know if this
> is a worthwhile optimization; it would reduce readability.
Right, it's a very unlikely code path. readability is more important
here.

I will fix both places. Maybe I can use also here what Jeff suggested in the
other mail. We'll see how it goes.

Boaz