diff mbox

[VERY,EARLY,RFC,01/13] scsi: use host_byte() accessor

Message ID 20180418150116.18807-2-jthumshirn@suse.de (mailing list archive)
State Changes Requested
Headers show

Commit Message

Johannes Thumshirn April 18, 2018, 3:01 p.m. UTC
Use the host_byte() accessor for getting the SCSI host byte.

Signed-off-by: Johannes Thumshirn <jthumshirn@suse.de>
---
 drivers/scsi/aic7xxx/aic79xx_osm.h | 2 +-
 drivers/scsi/aic7xxx/aic7xxx_osm.h | 2 +-
 drivers/scsi/dc395x.c              | 2 +-
 drivers/scsi/imm.c                 | 4 ++--
 drivers/scsi/ppa.c                 | 4 ++--
 5 files changed, 7 insertions(+), 7 deletions(-)

Comments

Finn Thain April 18, 2018, 11:46 p.m. UTC | #1
On Wed, 18 Apr 2018, Johannes Thumshirn wrote:

> --- a/drivers/scsi/dc395x.c
> +++ b/drivers/scsi/dc395x.c
> @@ -3473,7 +3473,7 @@ static void srb_done(struct AdapterCtlBlk *acb, struct DeviceCtlBlk *dcb,
>  
>  	/*if( srb->cmd->cmnd[0] == INQUIRY && */
>  	/*  (host_byte(cmd->result) == DID_OK || status_byte(cmd->result) & CHECK_CONDITION) ) */
> -		if ((cmd->result == (DID_OK << 16)
> +		if ((host_byte(cmd->result) == DID_OK
>  		     || status_byte(cmd->result) &
>  		     CHECK_CONDITION)) {
>  			if (!dcb->init_tcq_flag) {

That's not quite the same. The old test is effectively cmd->result == 0. 
Maybe this should be a separate patch?

--
Hannes Reinecke April 19, 2018, 6:01 a.m. UTC | #2
On Thu, 19 Apr 2018 09:46:30 +1000 (AEST)
Finn Thain <fthain@telegraphics.com.au> wrote:

> On Wed, 18 Apr 2018, Johannes Thumshirn wrote:
> 
> > --- a/drivers/scsi/dc395x.c
> > +++ b/drivers/scsi/dc395x.c
> > @@ -3473,7 +3473,7 @@ static void srb_done(struct AdapterCtlBlk
> > *acb, struct DeviceCtlBlk *dcb, 
> >  	/*if( srb->cmd->cmnd[0] == INQUIRY && */
> >  	/*  (host_byte(cmd->result) == DID_OK ||
> > status_byte(cmd->result) & CHECK_CONDITION) ) */
> > -		if ((cmd->result == (DID_OK << 16)
> > +		if ((host_byte(cmd->result) == DID_OK
> >  		     || status_byte(cmd->result) &
> >  		     CHECK_CONDITION)) {
> >  			if (!dcb->init_tcq_flag) {  
> 
> That's not quite the same. The old test is effectively cmd->result ==
> 0. Maybe this should be a separate patch?
> 

Indeed, as it's actually wrong.
The status byte is not a bitfield, but rather a value, and as such
the '&' check is wrong; it should be '=='.

Cheers,

Hannes
diff mbox

Patch

diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.h b/drivers/scsi/aic7xxx/aic79xx_osm.h
index 8a8b7ae7aed3..0a84a846ca88 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm.h
+++ b/drivers/scsi/aic7xxx/aic79xx_osm.h
@@ -550,7 +550,7 @@  void ahd_set_scsi_status(struct scb *scb, uint32_t status)
 static inline
 uint32_t ahd_cmd_get_transaction_status(struct scsi_cmnd *cmd)
 {
-	return ((cmd->result >> 16) & CAM_STATUS_MASK);
+	return host_byte(cmd->result) & CAM_STATUS_MASK;
 }
 
 static inline
diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm.h b/drivers/scsi/aic7xxx/aic7xxx_osm.h
index f8489078f003..108b30e92c75 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_osm.h
+++ b/drivers/scsi/aic7xxx/aic7xxx_osm.h
@@ -568,7 +568,7 @@  void ahc_set_scsi_status(struct scb *scb, uint32_t status)
 static inline
 uint32_t ahc_cmd_get_transaction_status(struct scsi_cmnd *cmd)
 {
-	return ((cmd->result >> 16) & CAM_STATUS_MASK);
+	return host_byte(cmd->result) & CAM_STATUS_MASK;
 }
 
 static inline
diff --git a/drivers/scsi/dc395x.c b/drivers/scsi/dc395x.c
index 60ef8df42b95..da28f08ae185 100644
--- a/drivers/scsi/dc395x.c
+++ b/drivers/scsi/dc395x.c
@@ -3473,7 +3473,7 @@  static void srb_done(struct AdapterCtlBlk *acb, struct DeviceCtlBlk *dcb,
 
 	/*if( srb->cmd->cmnd[0] == INQUIRY && */
 	/*  (host_byte(cmd->result) == DID_OK || status_byte(cmd->result) & CHECK_CONDITION) ) */
-		if ((cmd->result == (DID_OK << 16)
+		if ((host_byte(cmd->result) == DID_OK
 		     || status_byte(cmd->result) &
 		     CHECK_CONDITION)) {
 			if (!dcb->init_tcq_flag) {
diff --git a/drivers/scsi/imm.c b/drivers/scsi/imm.c
index 87c94191033b..690c0e25ea51 100644
--- a/drivers/scsi/imm.c
+++ b/drivers/scsi/imm.c
@@ -728,7 +728,7 @@  static void imm_interrupt(struct work_struct *work)
 	}
 	/* Command must of completed hence it is safe to let go... */
 #if IMM_DEBUG > 0
-	switch ((cmd->result >> 16) & 0xff) {
+	switch (host_byte(cmd->result)) {
 	case DID_OK:
 		break;
 	case DID_NO_CONNECT:
@@ -757,7 +757,7 @@  static void imm_interrupt(struct work_struct *work)
 		break;
 	default:
 		printk("imm: bad return code (%02x)\n",
-		       (cmd->result >> 16) & 0xff);
+		       host_byte(cmd->result));
 	}
 #endif
 
diff --git a/drivers/scsi/ppa.c b/drivers/scsi/ppa.c
index ee86a0c62dbf..f22fe4939a74 100644
--- a/drivers/scsi/ppa.c
+++ b/drivers/scsi/ppa.c
@@ -625,7 +625,7 @@  static void ppa_interrupt(struct work_struct *work)
 	}
 	/* Command must of completed hence it is safe to let go... */
 #if PPA_DEBUG > 0
-	switch ((cmd->result >> 16) & 0xff) {
+	switch (host_byte(cmd->result)) {
 	case DID_OK:
 		break;
 	case DID_NO_CONNECT:
@@ -654,7 +654,7 @@  static void ppa_interrupt(struct work_struct *work)
 		break;
 	default:
 		printk(KERN_WARNING "ppa: bad return code (%02x)\n",
-		       (cmd->result >> 16) & 0xff);
+		       host_byte(cmd->result));
 	}
 #endif