Re: [PATCH 10/24] scsi: introduce set_status_byte()

2019-10-21 Thread Hannes Reinecke 
On 10/22/19 12:12 AM, Finn Thain wrote:
> On Mon, 21 Oct 2019, Hannes Reinecke wrote:
> 
>> To be in-line with the other set_XX_byte() functions.
>>
>> Signed-off-by: Hannes Reinecke 
>> ---
>>  include/scsi/scsi_cmnd.h | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
>> index 91bd749a02f7..6932d91472d5 100644
>> --- a/include/scsi/scsi_cmnd.h
>> +++ b/include/scsi/scsi_cmnd.h
>> @@ -307,6 +307,11 @@ static inline struct scsi_data_buffer *scsi_prot(struct 
>> scsi_cmnd *cmd)
>>  #define scsi_for_each_prot_sg(cmd, sg, nseg, __i)   \
>>  for_each_sg(scsi_prot_sglist(cmd), sg, nseg, __i)
>>  
>> +static inline void set_status_byte(struct scsi_cmnd *cmd, char status)
>> +{
>> +cmd->result = (cmd->result & 0xff00) | status;
> 
> Is sign-extension desirable here? Do callers need it?
> 
It'll be a theoretical issue, as a status value with the top bit set
would be invalid anyway.
But for consistencies sake I'll make it an unsigned char in the next
iteration.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke   Teamlead Storage & Networking
h...@suse.de  +49 911 74053 688
SUSE Software Solutions Germany GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 247165 (AG München), GF: Felix Imendörffer

Re: [PATCH 10/24] scsi: introduce set_status_byte()

2019-10-21 Thread Finn Thain 
On Mon, 21 Oct 2019, Hannes Reinecke wrote:

> To be in-line with the other set_XX_byte() functions.
> 
> Signed-off-by: Hannes Reinecke 
> ---
>  include/scsi/scsi_cmnd.h | 5 +
>  1 file changed, 5 insertions(+)
> 
> diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
> index 91bd749a02f7..6932d91472d5 100644
> --- a/include/scsi/scsi_cmnd.h
> +++ b/include/scsi/scsi_cmnd.h
> @@ -307,6 +307,11 @@ static inline struct scsi_data_buffer *scsi_prot(struct 
> scsi_cmnd *cmd)
>  #define scsi_for_each_prot_sg(cmd, sg, nseg, __i)\
>   for_each_sg(scsi_prot_sglist(cmd), sg, nseg, __i)
>  
> +static inline void set_status_byte(struct scsi_cmnd *cmd, char status)
> +{
> + cmd->result = (cmd->result & 0xff00) | status;

Is sign-extension desirable here? Do callers need it?

-- 

> +}
> +
>  static inline void set_msg_byte(struct scsi_cmnd *cmd, char status)
>  {
>   cmd->result = (cmd->result & 0x00ff) | (status << 8);
>

[PATCH 10/24] scsi: introduce set_status_byte()

2019-10-21 Thread Hannes Reinecke 
To be in-line with the other set_XX_byte() functions.

Signed-off-by: Hannes Reinecke 
---
 include/scsi/scsi_cmnd.h | 5 +
 1 file changed, 5 insertions(+)

diff --git a/include/scsi/scsi_cmnd.h b/include/scsi/scsi_cmnd.h
index 91bd749a02f7..6932d91472d5 100644
--- a/include/scsi/scsi_cmnd.h
+++ b/include/scsi/scsi_cmnd.h
@@ -307,6 +307,11 @@ static inline struct scsi_data_buffer *scsi_prot(struct 
scsi_cmnd *cmd)
 #define scsi_for_each_prot_sg(cmd, sg, nseg, __i)  \
for_each_sg(scsi_prot_sglist(cmd), sg, nseg, __i)
 
+static inline void set_status_byte(struct scsi_cmnd *cmd, char status)
+{
+   cmd->result = (cmd->result & 0xff00) | status;
+}
+
 static inline void set_msg_byte(struct scsi_cmnd *cmd, char status)
 {
cmd->result = (cmd->result & 0x00ff) | (status << 8);
-- 
2.16.4