Re: [PATCH v2 08/10] usb/msd: Rename mode to cbw_state, and tweak names

["Nicholas Piggin" <npiggin@gmail.com>]

On Fri Apr 11, 2025 at 8:37 PM AEST, Philippe Mathieu-Daudé wrote:
> On 11/4/25 10:04, Nicholas Piggin wrote:
>> This reflects a little better what it does, particularly with a
>> subsequent change to relax the order packets are seen in. This
>> field is not the general state of the MSD state machine, rather
>> it follows packets that are completed as part of a CBW command.
>> 
>> The difference is a bit subtle, so for a concrete example, the
>> next change will permit the host to send a CSW packet before it
>> sends the associated CBW packet. In that case the CSW packet
>> will be tracked and the MSD state machine will move, but this
>> mode / cbw_state field would remain unchanged (in the "expecting
>> CBW" state), until the CBW packet arrives.
>> 
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>   include/hw/usb/msd.h | 12 +++++------
>>   hw/usb/dev-storage.c | 50 +++++++++++++++++++++++---------------------
>>   2 files changed, 32 insertions(+), 30 deletions(-)
>> 
>> diff --git a/include/hw/usb/msd.h b/include/hw/usb/msd.h
>> index a40d15f5def..c109544f632 100644
>> --- a/include/hw/usb/msd.h
>> +++ b/include/hw/usb/msd.h
>> @@ -10,11 +10,11 @@
>>   #include "hw/usb.h"
>>   #include "hw/scsi/scsi.h"
>>   
>> -enum USBMSDMode {
>> -    USB_MSDM_CBW, /* Command Block.  */
>> -    USB_MSDM_DATAOUT, /* Transfer data to device.  */
>> -    USB_MSDM_DATAIN, /* Transfer data from device.  */
>> -    USB_MSDM_CSW /* Command Status.  */
>
> Since modifying this, please add
>
>    typedef

Done.

Thanks,
Nick

>
>> +enum USBMSDCBWState {
>> +    USB_MSD_CBW_NONE,    /* Ready, waiting for CBW packet. */
>> +    USB_MSD_CBW_DATAOUT, /* Expecting DATA-OUT (to device) packet */
>> +    USB_MSD_CBW_DATAIN,  /* Expecting DATA-IN (from device) packet */
>> +    USB_MSD_CBW_CSW      /* No more data, expecting CSW packet.  */
>>   }
>
>        USBMSDCBWState;
>
>>   
>>   struct QEMU_PACKED usb_msd_csw {
>> @@ -26,7 +26,7 @@ struct QEMU_PACKED usb_msd_csw {
>>   
>>   struct MSDState {
>>       USBDevice dev;
>> -    enum USBMSDMode mode;
>> +    enum USBMSDCBWState cbw_state;
>
>         USBMSDCBWState cbw_state;
>
>>       uint32_t scsi_off;
>>       uint32_t scsi_len;
>>       uint32_t data_len;