diff mbox series

[15/39] scsi: add get_{status,host}_byte() accessor function

Message ID 20210423113944.42672-16-hare@suse.de
State New
Headers show
Series SCSI result cleanup, part 2 | expand

Commit Message

Hannes Reinecke April 23, 2021, 11:39 a.m. UTC
Add accessor functions for the host and status byte.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 include/scsi/scsi_cmnd.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

Bart Van Assche April 26, 2021, 3:47 a.m. UTC | #1
On 4/23/21 4:39 AM, Hannes Reinecke wrote:
> +static inline unsigned char get_status_byte(struct scsi_cmnd *cmd)

> +{

> +	return cmd->result & 0xff;

> +}

> +

>  static inline void set_msg_byte(struct scsi_cmnd *cmd, char status)

>  {

>  	cmd->result = (cmd->result & 0xffff00ff) | (status << 8);

> @@ -326,6 +331,11 @@ static inline void set_host_byte(struct scsi_cmnd *cmd, char status)

>  	cmd->result = (cmd->result & 0xff00ffff) | (status << 16);

>  }

>  

> +static inline unsigned char get_host_byte(struct scsi_cmnd *cmd)

> +{

> +	return (cmd->result >> 16) & 0xff;

> +}


How about using 'u8' instead of 'unsigned char' to make it more clear
that the returned value is an integer instead of a character? Anyway:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Hannes Reinecke April 26, 2021, 7:05 a.m. UTC | #2
On 4/26/21 5:47 AM, Bart Van Assche wrote:
> On 4/23/21 4:39 AM, Hannes Reinecke wrote:

>> +static inline unsigned char get_status_byte(struct scsi_cmnd *cmd)

>> +{

>> +	return cmd->result & 0xff;

>> +}

>> +

>>  static inline void set_msg_byte(struct scsi_cmnd *cmd, char status)

>>  {

>>  	cmd->result = (cmd->result & 0xffff00ff) | (status << 8);

>> @@ -326,6 +331,11 @@ static inline void set_host_byte(struct scsi_cmnd *cmd, char status)

>>  	cmd->result = (cmd->result & 0xff00ffff) | (status << 16);

>>  }

>>  

>> +static inline unsigned char get_host_byte(struct scsi_cmnd *cmd)

>> +{

>> +	return (cmd->result >> 16) & 0xff;

>> +}

> 

> How about using 'u8' instead of 'unsigned char' to make it more clear

> that the returned value is an integer instead of a character? Anyway:

> 

> Reviewed-by: Bart Van Assche <bvanassche@acm.org>

> 

I like it; 'unsigned char' is more in-line with the overall coding
style, but is quite lengthy and cumbersome.

Will be changing it for the next round.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		        Kernel Storage Architect
hare@suse.de			               +49 911 74053 688
SUSE Software Solutions Germany GmbH, 90409 Nürnberg
GF: F. Imendörffer, HRB 36809 (AG Nürnberg)
Christoph Hellwig April 26, 2021, 3:23 p.m. UTC | #3
On Fri, Apr 23, 2021 at 01:39:20PM +0200, Hannes Reinecke wrote:
> Add accessor functions for the host and status byte.


Looks good,

Reviewed-by: Christoph Hellwig <hch@lst.de>
diff mbox series

Patch

diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index a1eb7732aa1b..0ac18a7d8ac6 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -316,6 +316,11 @@  static inline void set_status_byte(struct scsi_cmnd *cmd, char status)
 	cmd->result = (cmd->result & 0xffffff00) | status;
 }
 
+static inline unsigned char get_status_byte(struct scsi_cmnd *cmd)
+{
+	return cmd->result & 0xff;
+}
+
 static inline void set_msg_byte(struct scsi_cmnd *cmd, char status)
 {
 	cmd->result = (cmd->result & 0xffff00ff) | (status << 8);
@@ -326,6 +331,11 @@  static inline void set_host_byte(struct scsi_cmnd *cmd, char status)
 	cmd->result = (cmd->result & 0xff00ffff) | (status << 16);
 }
 
+static inline unsigned char get_host_byte(struct scsi_cmnd *cmd)
+{
+	return (cmd->result >> 16) & 0xff;
+}
+
 
 static inline unsigned scsi_transfer_length(struct scsi_cmnd *scmd)
 {