* scsi.h: please rename the status_byte() macro
@ 2010-09-15 20:39 Douglas Gilbert
2010-09-16 20:46 ` Douglas Gilbert
0 siblings, 1 reply; 3+ messages in thread
From: Douglas Gilbert @ 2010-09-15 20:39 UTC (permalink / raw)
To: linux-scsi
... to something like:
scsi_status_byte_div2()
or:
scsi_broken_status_byte()
Doug Gilbert
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: scsi.h: please rename the status_byte() macro
2010-09-15 20:39 scsi.h: please rename the status_byte() macro Douglas Gilbert
@ 2010-09-16 20:46 ` Douglas Gilbert
2010-09-25 1:17 ` [RFC/PATCH] " Randy Dunlap
0 siblings, 1 reply; 3+ messages in thread
From: Douglas Gilbert @ 2010-09-16 20:46 UTC (permalink / raw)
To: SCSI development list
On 10-09-15 04:39 PM, Douglas Gilbert wrote:
> ... to something like:
> scsi_status_byte_div2()
> or:
> scsi_broken_status_byte()
The bug in the bsg driver with its SCSI status byte
brought this to my attention. If memory serves about
10 years ago some of us tried to kill the status_byte()
macro.
But it is still there in the kernel's scsi/scsi.h
ready to trip up another generation of programmers:
#define status_byte(result) (((result) >> 1) & 0x7f)
with no explanation that it does NOT yield the SCSI
status byte but the right shifted (once) equivalent.
The correct macro (or inline function) for obtaining the
SCSI status byte from the 32 bit "result" from a LLD would
be:
#define scsi_status_byte(result) ((result) & 0xff)
[Long ago (20 years) something silly was put in bit 0
so masking with 0xfe would work as well.]
Another thought, these macros are associated with the
mid-level or a ULD deciding whether a SCSI command has
succeeded; so scsi_cmnd.h would be a more appropriate
header.
Happily the version of scsi.h shown to the user space
(typically /usr/include/scsi/scsi.h) does not include
these defective macros. So any API breakage is limited
to the kernel.
Doug Gilbert
^ permalink raw reply [flat|nested] 3+ messages in thread
* [RFC/PATCH] Re: scsi.h: please rename the status_byte() macro
2010-09-16 20:46 ` Douglas Gilbert
@ 2010-09-25 1:17 ` Randy Dunlap
0 siblings, 0 replies; 3+ messages in thread
From: Randy Dunlap @ 2010-09-25 1:17 UTC (permalink / raw)
To: dgilbert; +Cc: SCSI development list
On Thu, 16 Sep 2010 16:46:21 -0400 Douglas Gilbert wrote:
> On 10-09-15 04:39 PM, Douglas Gilbert wrote:
> > ... to something like:
> > scsi_status_byte_div2()
> > or:
> > scsi_broken_status_byte()
There are many names that could be chosen for the new macro.
I don't particularly like the ones above. How about something
simple, like status_value() or status_bits()?
> The bug in the bsg driver with its SCSI status byte
> brought this to my attention. If memory serves about
> 10 years ago some of us tried to kill the status_byte()
> macro.
>
> But it is still there in the kernel's scsi/scsi.h
> ready to trip up another generation of programmers:
> #define status_byte(result) (((result) >> 1) & 0x7f)
>
> with no explanation that it does NOT yield the SCSI
> status byte but the right shifted (once) equivalent.
>
> The correct macro (or inline function) for obtaining the
> SCSI status byte from the 32 bit "result" from a LLD would
> be:
> #define scsi_status_byte(result) ((result) & 0xff)
>
> [Long ago (20 years) something silly was put in bit 0
> so masking with 0xfe would work as well.]
>
> Another thought, these macros are associated with the
> mid-level or a ULD deciding whether a SCSI command has
> succeeded; so scsi_cmnd.h would be a more appropriate
> header.
It makes sense to me to leave it where it is, along with
msg_byte(), host_byte(), and driver_byte().
> Happily the version of scsi.h shown to the user space
> (typically /usr/include/scsi/scsi.h) does not include
> these defective macros. So any API breakage is limited
> to the kernel.
Anyway, here is a cut at it. (patch is against linux-next)
---
From: Randy Dunlap <randy.dunlap@oracle.com>
Rename the misnamed status_byte() macro to status_bits()
and convert all users of it.
Retain the former macro as status_full_byte(), although it
has no users for now, so it could just go away.
Signed-off-by: Randy Dunlap <randy.dunlap@oracle.com>
---
block/bsg.c | 2 +-
block/scsi_ioctl.c | 2 +-
drivers/scsi/53c700.c | 6 +++---
drivers/scsi/NCR5380.c | 4 ++--
drivers/scsi/advansys.c | 4 ++--
drivers/scsi/aic7xxx_old.c | 2 +-
drivers/scsi/arm/acornscsi.c | 2 +-
drivers/scsi/arm/fas216.c | 6 +++---
drivers/scsi/atari_NCR5380.c | 6 +++---
drivers/scsi/dc395x.c | 8 ++++----
drivers/scsi/eata.c | 2 +-
drivers/scsi/scsi.c | 2 +-
drivers/scsi/scsi_error.c | 10 +++++-----
drivers/scsi/sg.c | 2 +-
drivers/scsi/sun3_NCR5380.c | 6 +++---
drivers/scsi/u14-34f.c | 2 +-
include/scsi/scsi.h | 3 ++-
17 files changed, 35 insertions(+), 34 deletions(-)
--- linux-next-20100924.orig/include/scsi/scsi.h
+++ linux-next-20100924/include/scsi/scsi.h
@@ -451,7 +451,8 @@ static inline int scsi_is_wlun(unsigned
* host_byte = set by low-level driver to indicate status.
* driver_byte = set by mid-level.
*/
-#define status_byte(result) (((result) >> 1) & 0x7f)
+#define status_full_byte(result) ((result) & 0xff)
+#define status_bits(result) (((result) >> 1) & 0x7f)
#define msg_byte(result) (((result) >> 8) & 0xff)
#define host_byte(result) (((result) >> 16) & 0xff)
#define driver_byte(result) (((result) >> 24) & 0xff)
--- linux-next-20100924.orig/block/bsg.c
+++ linux-next-20100924/block/bsg.c
@@ -425,7 +425,7 @@ static int blk_complete_sgv4_hdr_rq(stru
/*
* fill in all the output members
*/
- hdr->device_status = status_byte(rq->errors);
+ hdr->device_status = status_bits(rq->errors);
hdr->transport_status = host_byte(rq->errors);
hdr->driver_status = driver_byte(rq->errors);
hdr->info = 0;
--- linux-next-20100924.orig/block/scsi_ioctl.c
+++ linux-next-20100924/block/scsi_ioctl.c
@@ -253,7 +253,7 @@ static int blk_complete_sghdr_rq(struct
* fill in all the output members
*/
hdr->status = rq->errors & 0xff;
- hdr->masked_status = status_byte(rq->errors);
+ hdr->masked_status = status_bits(rq->errors);
hdr->msg_status = msg_byte(rq->errors);
hdr->host_status = host_byte(rq->errors);
hdr->driver_status = driver_byte(rq->errors);
--- linux-next-20100924.orig/drivers/scsi/arm/acornscsi.c
+++ linux-next-20100924/drivers/scsi/arm/acornscsi.c
@@ -852,7 +852,7 @@ static void acornscsi_done(AS_Host *host
xfer_warn = 0;
if (xfer_warn) {
- switch (status_byte(SCpnt->result)) {
+ switch (status_bits(SCpnt->result)) {
case CHECK_CONDITION:
case COMMAND_TERMINATED:
case BUSY:
--- linux-next-20100924.orig/drivers/scsi/arm/fas216.c
+++ linux-next-20100924/drivers/scsi/arm/fas216.c
@@ -2052,15 +2052,15 @@ fas216_std_done(FAS216_Info *info, struc
* If the command returned CHECK_CONDITION or COMMAND_TERMINATED
* status, request the sense information.
*/
- if (status_byte(SCpnt->result) == CHECK_CONDITION ||
- status_byte(SCpnt->result) == COMMAND_TERMINATED)
+ if (status_bits(SCpnt->result) == CHECK_CONDITION ||
+ status_bits(SCpnt->result) == COMMAND_TERMINATED)
goto request_sense;
/*
* If the command did not complete with GOOD status,
* we are all done here.
*/
- if (status_byte(SCpnt->result) != GOOD)
+ if (status_bits(SCpnt->result) != GOOD)
goto done;
/*
--- linux-next-20100924.orig/drivers/scsi/atari_NCR5380.c
+++ linux-next-20100924/drivers/scsi/atari_NCR5380.c
@@ -2188,7 +2188,7 @@ static void NCR5380_information_transfer
"completed\n", HOSTNO, cmd->device->id, cmd->device->lun);
#ifdef SUPPORT_TAGS
cmd_free_tag(cmd);
- if (status_byte(cmd->SCp.Status) == QUEUE_FULL) {
+ if (status_bits(cmd->SCp.Status) == QUEUE_FULL) {
/* Turn a QUEUE FULL status into BUSY, I think the
* mid level cannot handle QUEUE FULL :-( (The
* command is retried after BUSY). Also update our
@@ -2229,7 +2229,7 @@ static void NCR5380_information_transfer
if (cmd->cmnd[0] != REQUEST_SENSE)
cmd->result = cmd->SCp.Status | (cmd->SCp.Message << 8);
- else if (status_byte(cmd->SCp.Status) != GOOD)
+ else if (status_bits(cmd->SCp.Status) != GOOD)
cmd->result = (cmd->result & 0x00ffff) | (DID_ERROR << 16);
#ifdef AUTOSENSE
@@ -2240,7 +2240,7 @@ static void NCR5380_information_transfer
}
if ((cmd->cmnd[0] != REQUEST_SENSE) &&
- (status_byte(cmd->SCp.Status) == CHECK_CONDITION)) {
+ (status_bits(cmd->SCp.Status) == CHECK_CONDITION)) {
scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
ASEN_PRINTK("scsi%d: performing request sense\n", HOSTNO);
--- linux-next-20100924.orig/drivers/scsi/sun3_NCR5380.c
+++ linux-next-20100924/drivers/scsi/sun3_NCR5380.c
@@ -2209,7 +2209,7 @@ static void NCR5380_information_transfer
"completed\n", HOSTNO, cmd->device->id, cmd->device->lun);
#ifdef SUPPORT_TAGS
cmd_free_tag( cmd );
- if (status_byte(cmd->SCp.Status) == QUEUE_FULL) {
+ if (status_bits(cmd->SCp.Status) == QUEUE_FULL) {
/* Turn a QUEUE FULL status into BUSY, I think the
* mid level cannot handle QUEUE FULL :-( (The
* command is retried after BUSY). Also update our
@@ -2250,7 +2250,7 @@ static void NCR5380_information_transfer
if (cmd->cmnd[0] != REQUEST_SENSE)
cmd->result = cmd->SCp.Status | (cmd->SCp.Message << 8);
- else if (status_byte(cmd->SCp.Status) != GOOD)
+ else if (status_bits(cmd->SCp.Status) != GOOD)
cmd->result = (cmd->result & 0x00ffff) | (DID_ERROR << 16);
#ifdef AUTOSENSE
@@ -2261,7 +2261,7 @@ static void NCR5380_information_transfer
}
if ((cmd->cmnd[0] != REQUEST_SENSE) &&
- (status_byte(cmd->SCp.Status) == CHECK_CONDITION)) {
+ (status_bits(cmd->SCp.Status) == CHECK_CONDITION)) {
scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
ASEN_PRINTK("scsi%d: performing request sense\n",
HOSTNO);
--- linux-next-20100924.orig/drivers/scsi/dc395x.c
+++ linux-next-20100924/drivers/scsi/dc395x.c
@@ -3403,10 +3403,10 @@ static void srb_done(struct AdapterCtlBl
/*
* target status..........................
*/
- if (status_byte(status) == CHECK_CONDITION) {
+ if (status_bits(status) == CHECK_CONDITION) {
request_sense(acb, dcb, srb);
return;
- } else if (status_byte(status) == QUEUE_FULL) {
+ } else if (status_bits(status) == QUEUE_FULL) {
tempcnt = (u8)list_size(&dcb->srb_going_list);
dprintkl(KERN_INFO, "QUEUE_FULL for dev <%02i-%i> with %i cmnds\n",
dcb->target_id, dcb->target_lun, tempcnt);
@@ -3475,9 +3475,9 @@ static void srb_done(struct AdapterCtlBl
dcb->inquiry7 = ptr->Flags;
/*if( srb->cmd->cmnd[0] == INQUIRY && */
- /* (host_byte(cmd->result) == DID_OK || status_byte(cmd->result) & CHECK_CONDITION) ) */
+ /* (host_byte(cmd->result) == DID_OK || status_bits(cmd->result) & CHECK_CONDITION) ) */
if ((cmd->result == (DID_OK << 16)
- || status_byte(cmd->result) &
+ || status_bits(cmd->result) &
CHECK_CONDITION)) {
if (!dcb->init_tcq_flag) {
add_dev(acb, dcb, ptr);
--- linux-next-20100924.orig/drivers/scsi/NCR5380.c
+++ linux-next-20100924/drivers/scsi/NCR5380.c
@@ -2275,7 +2275,7 @@ static void NCR5380_information_transfer
if (cmd->cmnd[0] != REQUEST_SENSE)
cmd->result = cmd->SCp.Status | (cmd->SCp.Message << 8);
- else if (status_byte(cmd->SCp.Status) != GOOD)
+ else if (status_bits(cmd->SCp.Status) != GOOD)
cmd->result = (cmd->result & 0x00ffff) | (DID_ERROR << 16);
#ifdef AUTOSENSE
@@ -2285,7 +2285,7 @@ static void NCR5380_information_transfer
hostdata->ses.cmd_len = 0 ;
}
- if ((cmd->cmnd[0] != REQUEST_SENSE) && (status_byte(cmd->SCp.Status) == CHECK_CONDITION)) {
+ if ((cmd->cmnd[0] != REQUEST_SENSE) && (status_bits(cmd->SCp.Status) == CHECK_CONDITION)) {
scsi_eh_prep_cmnd(cmd, &hostdata->ses, NULL, 0, ~0);
dprintk(NDEBUG_AUTOSENSE, ("scsi%d : performing request sense\n", instance->host_no));
--- linux-next-20100924.orig/drivers/scsi/aic7xxx_old.c
+++ linux-next-20100924/drivers/scsi/aic7xxx_old.c
@@ -4241,7 +4241,7 @@ aic7xxx_handle_seqint(struct aic7xxx_hos
cmd->result = hscb->target_status;
- switch (status_byte(hscb->target_status))
+ switch (status_bits(hscb->target_status))
{
case GOOD:
if (aic7xxx_verbose & VERBOSE_SEQINT)
--- linux-next-20100924.orig/drivers/scsi/advansys.c
+++ linux-next-20100924/drivers/scsi/advansys.c
@@ -6725,7 +6725,7 @@ static void adv_isr_callback(ADV_DVC_VAR
ASC_DBG_PRT_SENSE(2, scp->sense_buffer,
SCSI_SENSE_BUFFERSIZE);
/*
- * Note: The 'status_byte()' macro used by
+ * Note: The 'status_bits()' macro used by
* target drivers defined in scsi.h shifts the
* status byte returned by host drivers right
* by 1 bit. This is why target drivers also
@@ -7658,7 +7658,7 @@ static void asc_isr_callback(ASC_DVC_VAR
ASC_DBG_PRT_SENSE(2, scp->sense_buffer,
SCSI_SENSE_BUFFERSIZE);
/*
- * Note: The 'status_byte()' macro used by
+ * Note: The 'status_bits()' macro used by
* target drivers defined in scsi.h shifts the
* status byte returned by host drivers right
* by 1 bit. This is why target drivers also
--- linux-next-20100924.orig/drivers/scsi/53c700.c
+++ linux-next-20100924/drivers/scsi/53c700.c
@@ -975,8 +975,8 @@ process_script_interrupt(__u32 dsps, __u
NCR_700_FINISHED_TAG_NEGOTIATION);
/* check for contingent allegiance contitions */
- if(status_byte(hostdata->status[0]) == CHECK_CONDITION ||
- status_byte(hostdata->status[0]) == COMMAND_TERMINATED) {
+ if(status_bits(hostdata->status[0]) == CHECK_CONDITION ||
+ status_bits(hostdata->status[0]) == COMMAND_TERMINATED) {
struct NCR_700_command_slot *slot =
(struct NCR_700_command_slot *)SCp->host_scribble;
if(slot->flags == NCR_700_FLAG_AUTOSENSE) {
@@ -1042,7 +1042,7 @@ process_script_interrupt(__u32 dsps, __u
// Currently rely on the mid layer evaluation
// of the tag queuing capability
//
- //if(status_byte(hostdata->status[0]) == GOOD &&
+ //if(status_bits(hostdata->status[0]) == GOOD &&
// SCp->cmnd[0] == INQUIRY && SCp->use_sg == 0) {
// /* Piggy back the tag queueing support
// * on this command */
--- linux-next-20100924.orig/drivers/scsi/eata.c
+++ linux-next-20100924/drivers/scsi/eata.c
@@ -2411,7 +2411,7 @@ static irqreturn_t ihdlr(struct Scsi_Hos
&& TLDEV(SCpnt->device->type))
flush_dev(SCpnt->device, blk_rq_pos(SCpnt->request), ha, 1);
- tstatus = status_byte(spp->target_status);
+ tstatus = status_bits(spp->target_status);
#if defined(DEBUG_GENERATE_ERRORS)
if ((ha->iocount > 500) && ((ha->iocount % 200) < 2))
--- linux-next-20100924.orig/drivers/scsi/scsi.c
+++ linux-next-20100924/drivers/scsi/scsi.c
@@ -614,7 +614,7 @@ void scsi_log_completion(struct scsi_cmn
}
scsi_print_result(cmd);
scsi_print_command(cmd);
- if (status_byte(cmd->result) & CHECK_CONDITION)
+ if (status_bits(cmd->result) & CHECK_CONDITION)
scsi_print_sense("", cmd);
if (level > 3)
scmd_printk(KERN_INFO, cmd,
--- linux-next-20100924.orig/drivers/scsi/sg.c
+++ linux-next-20100924/drivers/scsi/sg.c
@@ -1294,7 +1294,7 @@ static void sg_rq_end_io(struct request
struct scsi_sense_hdr sshdr;
srp->header.status = 0xff & result;
- srp->header.masked_status = status_byte(result);
+ srp->header.masked_status = status_bits(result);
srp->header.msg_status = msg_byte(result);
srp->header.host_status = host_byte(result);
srp->header.driver_status = driver_byte(result);
--- linux-next-20100924.orig/drivers/scsi/u14-34f.c
+++ linux-next-20100924/drivers/scsi/u14-34f.c
@@ -1804,7 +1804,7 @@ static irqreturn_t ihdlr(unsigned int j)
&& TLDEV(SCpnt->device->type))
flush_dev(SCpnt->device, blk_rq_pos(SCpnt->request), j, TRUE);
- tstatus = status_byte(spp->target_status);
+ tstatus = status_bits(spp->target_status);
#if defined(DEBUG_GENERATE_ERRORS)
if ((HD(j)->iocount > 500) && ((HD(j)->iocount % 200) < 2))
--- linux-next-20100924.orig/drivers/scsi/scsi_error.c
+++ linux-next-20100924/drivers/scsi/scsi_error.c
@@ -458,7 +458,7 @@ static int scsi_eh_completed_normally(st
* now, check the status byte to see if this indicates
* anything special.
*/
- switch (status_byte(scmd->result)) {
+ switch (status_bits(scmd->result)) {
case GOOD:
scsi_handle_queue_ramp_up(scmd->device);
case COMMAND_TERMINATED:
@@ -1339,14 +1339,14 @@ int scsi_noretry_cmd(struct scsi_cmnd *s
return (scmd->request->cmd_flags & REQ_FAILFAST_DEV);
case DID_ERROR:
if (msg_byte(scmd->result) == COMMAND_COMPLETE &&
- status_byte(scmd->result) == RESERVATION_CONFLICT)
+ status_bits(scmd->result) == RESERVATION_CONFLICT)
return 0;
/* fall through */
case DID_SOFT_ERROR:
return (scmd->request->cmd_flags & REQ_FAILFAST_DRIVER);
}
- switch (status_byte(scmd->result)) {
+ switch (status_bits(scmd->result)) {
case CHECK_CONDITION:
/*
* assume caller has checked sense and determinted
@@ -1449,7 +1449,7 @@ int scsi_decide_disposition(struct scsi_
return SUCCESS;
case DID_ERROR:
if (msg_byte(scmd->result) == COMMAND_COMPLETE &&
- status_byte(scmd->result) == RESERVATION_CONFLICT)
+ status_bits(scmd->result) == RESERVATION_CONFLICT)
/*
* execute reservation conflict processing code
* lower down
@@ -1487,7 +1487,7 @@ int scsi_decide_disposition(struct scsi_
/*
* check the status byte to see if this indicates anything special.
*/
- switch (status_byte(scmd->result)) {
+ switch (status_bits(scmd->result)) {
case QUEUE_FULL:
scsi_handle_queue_full(scmd->device);
/*
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2010-09-25 1:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-15 20:39 scsi.h: please rename the status_byte() macro Douglas Gilbert
2010-09-16 20:46 ` Douglas Gilbert
2010-09-25 1:17 ` [RFC/PATCH] " Randy Dunlap
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.