On Thu, Jul 19, 2012 at 06:15:02PM +0200, Gerd Hoffmann wrote:
> This patch adds support for booting from UAS (usb
> attached scsi) devices.
>
> For now only usb 2.0 support is there. On usb 3.0 the
> UAS protocol uses streams, so changes will be required
> to make usb 3.0 devices fly once we have a xhci host
> controller driver.
>
> So far the driver has been tested on qemu-emulated
> virtual hardware only. In theory should just work on
> bare metal too.
Thanks. Looks good to me. I have a few minor comments below.
[...]
> --- a/src/block.c
> +++ b/src/block.c
> @@ -280,7 +280,7 @@ map_floppy_drive(struct drive_s *drive_g)
> static int
> process_scsi_op(struct disk_op_s *op)
> {
> - if (!CONFIG_VIRTIO_SCSI && !CONFIG_USB_MSC)
> + if (!CONFIG_VIRTIO_SCSI && !CONFIG_USB_MSC && !CONFIG_USB_UAS)
I think I'll submit a patch to drop this config check - it's not
really needed.
[...]
> --- a/src/blockcmd.c
> +++ b/src/blockcmd.c
[...]
> int
> scsi_init_drive(struct drive_s *drive, const char *s, int prio)
> {
> - if (!CONFIG_USB_MSC && !CONFIG_VIRTIO_SCSI)
> + if (!CONFIG_USB_UAS && !CONFIG_USB_MSC && !CONFIG_VIRTIO_SCSI)
> return 0;
Same here.
[...]
> +int
> +usb_uas_init(struct usbdevice_s *usbdev)
> +{
> + if (!CONFIG_USB_UAS)
> + return -1;
> +
> + // Verify right kind of device
> + struct usb_interface_descriptor *iface = usbdev->iface;
> + if (iface->bInterfaceSubClass != 0x06 ||
> + iface->bInterfaceProtocol != 0x62) {
It would be preferable to #define constants for these numbers.
[...]
> + while (desc) {
> + desc += desc[0];
> + switch (desc[1]) {
> + case USB_DT_ENDPOINT:
> + ep = (void*)desc;
> + break;
> + case 0x24:
Same here.
[...]
> --- /dev/null
> +++ b/src/usb-uas.h
> @@ -0,0 +1,2 @@
> +int uas_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize);
> +int usb_uas_init(struct usbdevice_s *usbdev);
Due to SeaBIOS' use of -fcombine during the build, all headers really
should have #ifdef guards on them.
[...]
> + else if (iface->bInterfaceClass == USB_CLASS_MASS_STORAGE) {
> + if (iface->bInterfaceProtocol == 0x50)
> + ret = usb_msc_init(usbdev);
> + if (iface->bInterfaceProtocol == 0x62)
> + ret = usb_uas_init(usbdev);
Would prefer #define names.
-Kevin
