Re: [PATCH 21/24] scsi: consolidate the START STOP UNIT handling

On Thu, Jul 22, 2021 at 11:44:33AM -0700, Bart Van Assche wrote:

On 7/11/21 10:48 PM, Christoph Hellwig wrote:

quoted

+	char cdb[MAX_COMMAND_SIZE] = { };

How about using 'u8' instead of 'char'?

Sure.

Additionally, MAX_COMMAND_SIZE equals 16. According to SBC-4 6 bytes is 
enough for the START STOP UNIT command.

This just keeps the existing logic.

quoted

+	cdb[0] = START_STOP;
+	cdb[4] = data;

Please combine the above two statements with the cdb[] declaration into a 
single line.

Just keeping the style of the existing code, which seems fine.  Even
when initializing at declaration time this would have to be multiple
lines to stay readable.

Additionally, please split data into two arguments to make calls of this 
function easier to read. This is what I found in SBC-4:
* bit 1 of byte 4 has the name LOEJ (load eject).
* bit 0 of byte 4 has the name START (start unit).

Not sure how that really helps anyone.  And again I'm not trying to
do a grand rewrite, just consoidating the existing code a bit.