On 11/27/21 12:12 AM, Bart Van Assche wrote:
On 11/25/21 07:10, Hannes Reinecke wrote:
+struct scsi_cmnd *scsi_get_internal_cmd(struct scsi_device *sdev,
+ int data_direction, bool nowait)
Please use enum dma_data_direction instead of 'int' for the data direction.
I have oriented myself at __scsi_execute(), which also has
'data_direction' as an integer.
Presumably to avoid header clutter.
Martin?
+{
+ struct request *rq;
+ struct scsi_cmnd *scmd;
+ blk_mq_req_flags_t flags = 0;
+ int op;
The name 'op' is confusing since that variable is a bitfield that
includes the operation and operation flags. Consider one of the names
'opf', 'op_and_flags' or 'cmd_and_flags'. Please also change the data
type from 'int' into 'unsigned int' or 'u32'.
'op' is the name the variable has in blk_mq_alloc_request(), so I prefer
to stay with that name. Agree with the 'unsigned int', though.
+ op = (data_direction == DMA_TO_DEVICE) ?
+ REQ_OP_DRV_OUT : REQ_OP_DRV_IN;
Please leave out the parentheses from the above assignment.
Okay.
Cheers,
Hannes
--
Dr. Hannes Reinecke Kernel Storage Architect
hare@xxxxxxx +49 911 74053 688
SUSE Software Solutions GmbH, Maxfeldstr. 5, 90409 Nürnberg
HRB 36809 (AG Nürnberg), Geschäftsführer: Felix Imendörffer
