On Wed, Mar 14, 2012 at 01:02:39PM -0600, Steve Goodrich wrote:
> I've been working with coreboot and SeaBIOS lately to try to get a platform
> working, including having boot capability using an SD-to-USB adapter. I got
> stuck on this last part: the USB/SD adapter would start enumeration, but
> would fail out during TEST_UNIT_READY because the device came back with a
> STALL (this via a LeCroy/CACT USB analyzer). I compared the transactions to
> those done during USB enumeration/configuration in Linux, and I noticed that
> the Direction bit in bmCBWFlags is *cleared* for the TEST_UNIT_READY command
> in Linux, but *set* in coreboot.
If Linux always clears the direction bit for zero length transfers,
then I think it should be safe to do that in SeaBIOS also. How about
the patch below?
-Kevin
>From 1fd9a89082b807a4bb4ab6ce1285df474cb75746 Mon Sep 17 00:00:00 2001
From: Kevin O'Connor <kevin(a)koconnor.net>
Date: Wed, 14 Mar 2012 21:27:45 -0400
Subject: [PATCH] Use OUT mode for all zero byte "scsi" transfers.
Some devices can get confused if asked to "read" data during a zero
byte transfer, so consider these transfers as "writes". (Reported by
Steve Goodrich.)
Also, extract out the code to determine the transfer direction into
cdb_is_read().
Signed-off-by: Kevin O'Connor <kevin(a)koconnor.net>
---
src/blockcmd.c | 7 +++++++
src/blockcmd.h | 1 +
src/usb-msc.c | 4 ++--
src/virtio-scsi.c | 7 ++++---
4 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/src/blockcmd.c b/src/blockcmd.c
index 8d881a5..2dda04d 100644
--- a/src/blockcmd.c
+++ b/src/blockcmd.c
@@ -35,6 +35,13 @@ cdb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize)
}
}
+// Determine if the command is a request to pull data from the device
+int
+cdb_is_read(u8 *cdbcmd, u16 blocksize)
+{
+ return blocksize && cdbcmd[0] != CDB_CMD_WRITE_10;
+}
+
int
scsi_is_ready(struct disk_op_s *op)
{
diff --git a/src/blockcmd.h b/src/blockcmd.h
index ccfeeaf..8459d3e 100644
--- a/src/blockcmd.h
+++ b/src/blockcmd.h
@@ -100,6 +100,7 @@ struct cdbres_mode_sense_geom {
} PACKED;
// blockcmd.c
+int cdb_is_read(u8 *cdbcmd, u16 blocksize);
int cdb_get_inquiry(struct disk_op_s *op, struct cdbres_inquiry *data);
int cdb_get_sense(struct disk_op_s *op, struct cdbres_request_sense *data);
int cdb_test_unit_ready(struct disk_op_s *op);
diff --git a/src/usb-msc.c b/src/usb-msc.c
index dadb9ca..c53a1f5 100644
--- a/src/usb-msc.c
+++ b/src/usb-msc.c
@@ -77,13 +77,13 @@ usb_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize)
cbw.dCBWSignature = CBW_SIGNATURE;
cbw.dCBWTag = 999; // XXX
cbw.dCBWDataTransferLength = bytes;
- cbw.bmCBWFlags = (cbw.CBWCB[0] == CDB_CMD_WRITE_10) ? USB_DIR_OUT : USB_DIR_IN;
+ cbw.bmCBWFlags = cdb_is_read(cdbcmd, blocksize) ? USB_DIR_IN : USB_DIR_OUT;
cbw.bCBWLUN = 0; // XXX
cbw.bCBWCBLength = USB_CDB_SIZE;
// Transfer cbw to device.
int ret = usb_msc_send(udrive_g, USB_DIR_OUT
- , MAKE_FLATPTR(GET_SEG(SS), &cbw), sizeof(cbw));
+ , MAKE_FLATPTR(GET_SEG(SS), &cbw), sizeof(cbw));
if (ret)
goto fail;
diff --git a/src/virtio-scsi.c b/src/virtio-scsi.c
index 3c59438..50339d7 100644
--- a/src/virtio-scsi.c
+++ b/src/virtio-scsi.c
@@ -31,7 +31,7 @@ struct virtio_lun_s {
static int
virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op,
- void *cdbcmd, u16 target, u16 lun, u32 len)
+ void *cdbcmd, u16 target, u16 lun, u16 blocksize)
{
struct virtio_scsi_req_cmd req;
struct virtio_scsi_resp_cmd resp;
@@ -44,7 +44,8 @@ virtio_scsi_cmd(u16 ioaddr, struct vring_virtqueue *vq, struct disk_op_s *op,
req.lun[3] = (lun & 0xff);
memcpy(req.cdb, cdbcmd, 16);
- int datain = (req.cdb[0] != CDB_CMD_WRITE_10);
+ u32 len = op->count * blocksize;
+ int datain = cdb_is_read(cdbcmd, blocksize);
int data_idx = (datain ? 2 : 1);
int out_num = (datain ? 1 : 2);
int in_num = (len ? 3 : 2) - out_num;
@@ -89,7 +90,7 @@ virtio_scsi_cmd_data(struct disk_op_s *op, void *cdbcmd, u16 blocksize)
return virtio_scsi_cmd(GET_GLOBAL(vlun->ioaddr),
GET_GLOBAL(vlun->vq), op, cdbcmd,
GET_GLOBAL(vlun->target), GET_GLOBAL(vlun->lun),
- blocksize * op->count);
+ blocksize);
}
static int
--
1.7.6.5