[PATCH 2/6] usb:gadget:mass-storage: refactoring the SCSI command handling
i.kononenko at yadro.com
Mon Jun 28 20:38:27 AEST 2021
On 28.06.2021 04:06, Alan Stern wrote:
> On Sun, Jun 27, 2021 at 08:14:48PM +0300, i.kononenko wrote:
>> On 27.06.2021 17:23, Alan Stern wrote:
>>> On Sun, Jun 27, 2021 at 12:18:15AM +0300, Igor Kononenko wrote:
>>>> Implements a universal way to define SCSI commands and configure
>>>> precheck handlers.
>>> What is the reason for doing this?
>> I have started implementing a way to specify a backend-file of
>> mass-storage images greater than 2.1Gb for cdrom-like mediums.
>> I notice the implementation of each scsi-command handler uses too
>> many magic-constant, hardcoded indexes and shifts. I decided to
>> define structures that contained appropriate SCSI-defined fields
>> and constant-values to clarify the code.
>> Additionally, I noticed, many kernel subsystems use the 'separate
>> data and logic' approach, making a code more explicit and readable.
>> This looks reasonable to me, and a code looks more clearly, at
>> least - we don't need to examine each magic constant and its purpose.
>>> At first glance, it appears you have added a great deal of complexity
>>> to the driver. The patch replaces a large amount of easily understood
>>> (albeit rather repetitious) code with an approximately equal amount
>>> of rather complicated code. This does not seem like an improvement.
>> The SCSI-commands table is defined as unifying a way to specify the
>> SCSI-command handler, with corresponding required data instead pass
>> it to each repeatedly switch-case block, which makes code more readable
>> to me. If there isn't, I can keep the definition of SCSI-handlers as is,
>> but the SCSI-data structures with their constant-values are still
>> required, in my opinion.
>>> Furthermore, the code you removed is flexible; it easily allows for
>>> small variations as neede by some command handlers. But the code you
>>> added is all table-driven, which does not easily permit arbitrary
>> I don't think that the SCSI-command handlers table is an obstacle to
>> define variation into a specific handler because the current patch has
>> helper macros, which can specify a behavior for each requirement of
>> Anyway, the definition of the scsi-command handlers table may be discarded,
>> because this work done to helping developers who will work the
>> 'usb:gadget:mass-storage' subsystem in the future.
> Can you submit a patch that adds only the data structures without the
> commands table?
Yes, I will remove the mentioned SCSI-handlers table. Thank you!
> Alan Stern
More information about the openbmc