diff mbox series

[v4,06/36] hwmon: drivetemp: Convert to scsi_exec_req

Message ID 20221016195946.7613-7-michael.christie@oracle.com
State Superseded
Headers show
Series Allow scsi_execute users to control retries | expand

Commit Message

Mike Christie Oct. 16, 2022, 7:59 p.m. UTC 
scsi_execute* is going to be removed. Convert to scsi_exec_req so
we pass all args in a scsi_exec_args struct.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
---
 drivers/hwmon/drivetemp.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

Comments

Bart Van Assche Oct. 19, 2022, 10:06 p.m. UTC | #1 
On 10/16/22 12:59, Mike Christie wrote:
> scsi_execute* is going to be removed. Convert to scsi_exec_req so
> we pass all args in a scsi_exec_args struct.
> 
> Signed-off-by: Mike Christie <michael.christie@oracle.com>
> Reviewed-by: Martin Wilck <mwilck@suse.com>
> ---
>   drivers/hwmon/drivetemp.c | 11 ++++++++---
>   1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/hwmon/drivetemp.c b/drivers/hwmon/drivetemp.c
> index 5bac2b0fc7bb..ec208cac9c7f 100644
> --- a/drivers/hwmon/drivetemp.c
> +++ b/drivers/hwmon/drivetemp.c
> @@ -192,9 +192,14 @@ static int drivetemp_scsi_command(struct drivetemp_data *st,
>   	scsi_cmd[12] = lba_high;
>   	scsi_cmd[14] = ata_command;
>   
> -	return scsi_execute_req(st->sdev, scsi_cmd, data_dir,
> -				st->smartdata, ATA_SECT_SIZE, NULL, HZ, 5,
> -				NULL);
> +	return scsi_exec_req(((struct scsi_exec_args) {
> +					.sdev = st->sdev,
> +					.cmd = scsi_cmd,
> +					.data_dir = data_dir,
> +					.buf = st->smartdata,
> +					.buf_len = ATA_SECT_SIZE,
> +					.timeout = HZ,
> +					.retries = 5 }));
>   }
>   
>   static int drivetemp_ata_command(struct drivetemp_data *st, u8 feature,

Since Damien Le Moal is the ATA Maintainer, you may want to Cc him for 
ATA patches. Anyway:

Reviewed-by: Bart Van Assche <bvanassche@acm.org>
Damien Le Moal Oct. 19, 2022, 11:23 p.m. UTC | #2 
On 10/20/22 07:06, Bart Van Assche wrote:
> On 10/16/22 12:59, Mike Christie wrote:
>> scsi_execute* is going to be removed. Convert to scsi_exec_req so
>> we pass all args in a scsi_exec_args struct.
>>
>> Signed-off-by: Mike Christie <michael.christie@oracle.com>
>> Reviewed-by: Martin Wilck <mwilck@suse.com>
>> ---
>>   drivers/hwmon/drivetemp.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/hwmon/drivetemp.c b/drivers/hwmon/drivetemp.c
>> index 5bac2b0fc7bb..ec208cac9c7f 100644
>> --- a/drivers/hwmon/drivetemp.c
>> +++ b/drivers/hwmon/drivetemp.c
>> @@ -192,9 +192,14 @@ static int drivetemp_scsi_command(struct drivetemp_data *st,
>>   	scsi_cmd[12] = lba_high;
>>   	scsi_cmd[14] = ata_command;
>>   
>> -	return scsi_execute_req(st->sdev, scsi_cmd, data_dir,
>> -				st->smartdata, ATA_SECT_SIZE, NULL, HZ, 5,
>> -				NULL);
>> +	return scsi_exec_req(((struct scsi_exec_args) {
>> +					.sdev = st->sdev,
>> +					.cmd = scsi_cmd,
>> +					.data_dir = data_dir,
>> +					.buf = st->smartdata,
>> +					.buf_len = ATA_SECT_SIZE,
>> +					.timeout = HZ,
>> +					.retries = 5 }));
>>   }
>>   
>>   static int drivetemp_ata_command(struct drivetemp_data *st, u8 feature,
> 
> Since Damien Le Moal is the ATA Maintainer, you may want to Cc him for 
> ATA patches. Anyway:
> 
> Reviewed-by: Bart Van Assche <bvanassche@acm.org>

This is ATA related, but not ata code :)

Anyway, while not being a fan of the function call + struct initialization
all together, this looks correct to me.

Reviewed-by: Damien Le Moal <damien.lemoal@opensource.wdc.com>
Bart Van Assche Oct. 20, 2022, 5:52 p.m. UTC | #3 
On 10/19/22 16:23, Damien Le Moal wrote:
> Anyway, while not being a fan of the function call + struct initialization
> all together, this looks correct to me.

Hi Damien,

I asked to make this change for scsi_execute() because Mike's patch 
series adds an argument to that macro and because I would like to add 
another argument to that macro. Adding a member to a struct is much 
easier than adding an additional argument to a macro and updating all 
callers.

Thanks,

Bart.
Damien Le Moal Oct. 20, 2022, 9:22 p.m. UTC | #4 
On 10/21/22 02:52, Bart Van Assche wrote:
> On 10/19/22 16:23, Damien Le Moal wrote:
>> Anyway, while not being a fan of the function call + struct initialization
>> all together, this looks correct to me.
> 
> Hi Damien,
> 
> I asked to make this change for scsi_execute() because Mike's patch 
> series adds an argument to that macro and because I would like to add 
> another argument to that macro. Adding a member to a struct is much 
> easier than adding an additional argument to a macro and updating all 
> callers.

I like that the arguments are passed as a struct. This is fine. It is the
struct being initialized inside the "( )" for the function call that I am
not a fan of. But as said, not a big deal.

> 
> Thanks,
> 
> Bart.
diff mbox series

Patch

diff --git a/drivers/hwmon/drivetemp.c b/drivers/hwmon/drivetemp.c
index 5bac2b0fc7bb..ec208cac9c7f 100644
--- a/drivers/hwmon/drivetemp.c
+++ b/drivers/hwmon/drivetemp.c
@@ -192,9 +192,14 @@  static int drivetemp_scsi_command(struct drivetemp_data *st,
 	scsi_cmd[12] = lba_high;
 	scsi_cmd[14] = ata_command;
 
-	return scsi_execute_req(st->sdev, scsi_cmd, data_dir,
-				st->smartdata, ATA_SECT_SIZE, NULL, HZ, 5,
-				NULL);
+	return scsi_exec_req(((struct scsi_exec_args) {
+					.sdev = st->sdev,
+					.cmd = scsi_cmd,
+					.data_dir = data_dir,
+					.buf = st->smartdata,
+					.buf_len = ATA_SECT_SIZE,
+					.timeout = HZ,
+					.retries = 5 }));
 }
 
 static int drivetemp_ata_command(struct drivetemp_data *st, u8 feature,