usb: device_next: new USB Mass Storage implementation #54776
Conversation
tmon-nordic
requested review from
jfischer-no,
nashif,
carlescufi,
galak and
MaureenHelm
as code owners
tmon-nordic
force-pushed
the
usbd-msc
branch
2 times, most recently
from
67ce80c to
a17e18b
Compare
tmon-nordic
force-pushed
the
usbd-msc
branch
2 times, most recently
from
9593cd6 to
3cbe450
Compare
tmon-nordic
force-pushed
the
usbd-msc
branch
from
3cbe450 to
72d4424
Compare
| * to allow generating boilerplate handling code. | ||
| */ | ||
| #define SCSI_CMD_STRUCT(opcode) struct scsi_##opcode##_cmd | ||
| #define SCSI_CMD_HANDLER(opcode) \ |
There was a problem hiding this comment.
I would not obscure it for about 10 definitions.
There was a problem hiding this comment.
I disagree. This macro enforces common calling scheme for all commands.
| #define SCSI_CMD_HANDLER(opcode) \ | ||
| static int scsi_##opcode(struct scsi_ctx *ctx, \ | ||
| struct scsi_##opcode##_cmd *cmd, \ | ||
| uint8_t data_in_buf[CONFIG_USBD_MSC_SCSI_BUFFER_SIZE]) |
There was a problem hiding this comment.
Looks like there is not data_out_buf, I suggest to name just data, or buf.
| uint8_t data_in_buf[CONFIG_USBD_MSC_SCSI_BUFFER_SIZE]) | |
| uint8_t *const data) |
or one that generates a warning
| uint8_t data_in_buf[CONFIG_USBD_MSC_SCSI_BUFFER_SIZE]) | |
| uint8_t data[static CONFIG_USBD_MSC_SCSI_BUFFER_SIZE]) |
There was a problem hiding this comment.
There is no data_out_buf passed to handler directly because the data out is not available at this time. If the command is not supported (or command wants to process more data than indicated in CBW, or wants to send data instead of receive), we want to STALL OUT endpoint immediately to not waste time receiving any Data Out. Besides that, having both data_in_buf and data_out_buf would require allocating two buffers but considering how USB Bulk-Only Transport SCSI transport works it is perfectly fine to have common shared buffer.
"Data-In Buffer" and "Data-Out Buffer" are the terms used in SAM-6 (5.4.2.2 Send SCSI Command SCSI transport protocol service request) so I refuse to change the name.
There was a problem hiding this comment.
"Data-In Buffer" and "Data-Out Buffer" are the terms used in SAM-6 (5.4.2.2 Send SCSI Command SCSI transport protocol service request) so I refuse to change the name.
okay, but why not uint8_t data_in_buf[static CONFIG_USBD_MSC_SCSI_BUFFER_SIZE]), otherwise I do not see a reason to use sized array parameter.
There was a problem hiding this comment.
I had to look it up what static means in this context and it means what I really want. Added static to all declarations.
tmon-nordic
force-pushed
the
usbd-msc
branch
3 times, most recently
from
0072307 to
780a709
Compare
Introduce new USB Mass Storage Bulk-Only Transport implementation written from scratch. Main goal behind new implementation was to separate USB and SCSI parts as clearly as possible. Limited set of SCSI disk commands is implemented in separate source code file that is internal to USB device stack. While it should be possible to use the SCSI implementation by other components there is currently no other user besides USB MSC and therefore SCSI header is kept private. Signed-off-by: Tomasz Moń <tomasz.mon@nordicsemi.no>
tmon-nordic
force-pushed
the
usbd-msc
branch
from
780a709 to
9dc1c95
Compare
Introduce new USB Mass Storage Bulk-Only Transport implementation written from scratch. Main goal behind new implementation was to separate USB and SCSI parts as clearly as possible.
Limited set of SCSI disk commands is implemented in separate source code file that is internal to USB device stack. While it should be possible to use the SCSI implementation by other components there is currently no other user besides USB MSC and therefore SCSI header is kept private.
Signed-off-by: Tomasz Moń tomasz.mon@nordicsemi.no