Hi Greg,
The following changes since commit 0fcdcfbbc98f70f559e4b36773a69972489a6d8f:
Merge branch 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/tj/wq (2010-12-14 18:50:10 -0800)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/willy/uas.git master
Matthew Wilcox (5):
uas: Fix up the Sense IU
uas: Use kzalloc instead of kmalloc
uas: Rename sense pipe and sense urb to status pipe and status urb
uas: Ensure we only bind to a UAS interface
uas: Use GFP_NOIO instead of GFP_KERNEL in I/O submission path
drivers/usb/storage/uas.c | 82 +++++++++++++++++++++++++++++---------------
1 files changed, 54 insertions(+), 28 deletions(-)
I'll send the patches as replies to this mail.
--
Matthew Wilcox Intel Open Source Technology Centre
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."
From: Matthew Wilcox <[email protected]>
The spec calls this the status pipe. While it is used to receive sense IUs,
it is also used to receive other IUs, so this can be confusing.
Reported-by: Luben Tuikov <[email protected]>
Signed-off-by: Matthew Wilcox <[email protected]>
---
drivers/usb/storage/uas.c | 32 ++++++++++++++++----------------
1 files changed, 16 insertions(+), 16 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 981fdef..f82787f 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -100,8 +100,8 @@ struct uas_dev_info {
};
enum {
- ALLOC_SENSE_URB = (1 << 0),
- SUBMIT_SENSE_URB = (1 << 1),
+ ALLOC_STATUS_URB = (1 << 0),
+ SUBMIT_STATUS_URB = (1 << 1),
ALLOC_DATA_IN_URB = (1 << 2),
SUBMIT_DATA_IN_URB = (1 << 3),
ALLOC_DATA_OUT_URB = (1 << 4),
@@ -115,7 +115,7 @@ struct uas_cmd_info {
unsigned int state;
unsigned int stream;
struct urb *cmd_urb;
- struct urb *sense_urb;
+ struct urb *status_urb;
struct urb *data_in_urb;
struct urb *data_out_urb;
struct list_head list;
@@ -207,7 +207,7 @@ static void uas_xfer_data(struct urb *urb, struct scsi_cmnd *cmnd,
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
int err;
- cmdinfo->state = direction | SUBMIT_SENSE_URB;
+ cmdinfo->state = direction | SUBMIT_STATUS_URB;
err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC);
if (err) {
spin_lock(&uas_work_lock);
@@ -360,21 +360,21 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,
{
struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp;
- if (cmdinfo->state & ALLOC_SENSE_URB) {
- cmdinfo->sense_urb = uas_alloc_sense_urb(devinfo, gfp, cmnd,
- cmdinfo->stream);
- if (!cmdinfo->sense_urb)
+ if (cmdinfo->state & ALLOC_STATUS_URB) {
+ cmdinfo->status_urb = uas_alloc_sense_urb(devinfo, gfp, cmnd,
+ cmdinfo->stream);
+ if (!cmdinfo->status_urb)
return SCSI_MLQUEUE_DEVICE_BUSY;
- cmdinfo->state &= ~ALLOC_SENSE_URB;
+ cmdinfo->state &= ~ALLOC_STATUS_URB;
}
- if (cmdinfo->state & SUBMIT_SENSE_URB) {
- if (usb_submit_urb(cmdinfo->sense_urb, gfp)) {
+ if (cmdinfo->state & SUBMIT_STATUS_URB) {
+ if (usb_submit_urb(cmdinfo->status_urb, gfp)) {
scmd_printk(KERN_INFO, cmnd,
"sense urb submission failure\n");
return SCSI_MLQUEUE_DEVICE_BUSY;
}
- cmdinfo->state &= ~SUBMIT_SENSE_URB;
+ cmdinfo->state &= ~SUBMIT_STATUS_URB;
}
if (cmdinfo->state & ALLOC_DATA_IN_URB) {
@@ -443,7 +443,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
BUILD_BUG_ON(sizeof(struct uas_cmd_info) > sizeof(struct scsi_pointer));
- if (!cmdinfo->sense_urb && sdev->current_cmnd)
+ if (!cmdinfo->status_urb && sdev->current_cmnd)
return SCSI_MLQUEUE_DEVICE_BUSY;
if (blk_rq_tagged(cmnd->request)) {
@@ -455,7 +455,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
cmnd->scsi_done = done;
- cmdinfo->state = ALLOC_SENSE_URB | SUBMIT_SENSE_URB |
+ cmdinfo->state = ALLOC_STATUS_URB | SUBMIT_STATUS_URB |
ALLOC_CMD_URB | SUBMIT_CMD_URB;
switch (cmnd->sc_data_direction) {
@@ -478,8 +478,8 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
err = uas_submit_urbs(cmnd, devinfo, GFP_ATOMIC);
if (err) {
/* If we did nothing, give up now */
- if (cmdinfo->state & SUBMIT_SENSE_URB) {
- usb_free_urb(cmdinfo->sense_urb);
+ if (cmdinfo->state & SUBMIT_STATUS_URB) {
+ usb_free_urb(cmdinfo->status_urb);
return SCSI_MLQUEUE_DEVICE_BUSY;
}
spin_lock(&uas_work_lock);
--
1.7.2.3
From: Matthew Wilcox <[email protected]>
The IUs are not being fully initialised by the driver (due to the reserved
space). Since we should be zeroing reserved fields, use kzalloc to do
it for us.
Reported-by: Luben Tuikov <[email protected]>
Signed-off-by: Matthew Wilcox <[email protected]>
---
drivers/usb/storage/uas.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index ca277c2..981fdef 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -297,7 +297,7 @@ static struct urb *uas_alloc_sense_urb(struct uas_dev_info *devinfo, gfp_t gfp,
if (!urb)
goto out;
- iu = kmalloc(sizeof(*iu), gfp);
+ iu = kzalloc(sizeof(*iu), gfp);
if (!iu)
goto free;
@@ -328,7 +328,7 @@ static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp,
if (len < 0)
len = 0;
len = ALIGN(len, 4);
- iu = kmalloc(sizeof(*iu) + len, gfp);
+ iu = kzalloc(sizeof(*iu) + len, gfp);
if (!iu)
goto free;
--
1.7.2.3
From: Matthew Wilcox <[email protected]>
If swap is on a UAS device, we could recurse into the driver by using
GFP_KERNEL. Using GFP_NOIO ensures we won't.
Reported-by: James Bottomley <[email protected]>
Signed-off-by: Matthew Wilcox <[email protected]>
---
drivers/usb/storage/uas.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 0ebe7f6..23f0dd9 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -141,7 +141,7 @@ static void uas_do_work(struct work_struct *work)
struct scsi_pointer *scp = (void *)cmdinfo;
struct scsi_cmnd *cmnd = container_of(scp,
struct scsi_cmnd, SCp);
- uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_KERNEL);
+ uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_NOIO);
}
}
--
1.7.2.3
From: Matthew Wilcox <[email protected]>
While all existing UAS devices use alternate interface 1, this is not
guaranteed, and it has caused confusion with people trying to bind the
uas driver to non-uas devices.
Signed-off-by: Matthew Wilcox <[email protected]>
---
drivers/usb/storage/uas.c | 37 ++++++++++++++++++++++++++++++-------
1 files changed, 30 insertions(+), 7 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index f82787f..0ebe7f6 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -581,6 +581,34 @@ static struct usb_device_id uas_usb_ids[] = {
};
MODULE_DEVICE_TABLE(usb, uas_usb_ids);
+static int uas_is_interface(struct usb_host_interface *intf)
+{
+ return (intf->desc.bInterfaceClass == USB_CLASS_MASS_STORAGE &&
+ intf->desc.bInterfaceSubClass == USB_SC_SCSI &&
+ intf->desc.bInterfaceProtocol == USB_PR_UAS);
+}
+
+static int uas_switch_interface(struct usb_device *udev,
+ struct usb_interface *intf)
+{
+ int i;
+
+ if (uas_is_interface(intf->cur_altsetting))
+ return 0;
+
+ for (i = 0; i < intf->num_altsetting; i++) {
+ struct usb_host_interface *alt = &intf->altsetting[i];
+ if (alt == intf->cur_altsetting)
+ continue;
+ if (uas_is_interface(alt))
+ return usb_set_interface(udev,
+ alt->desc.bInterfaceNumber,
+ alt->desc.bAlternateSetting);
+ }
+
+ return -ENODEV;
+}
+
static void uas_configure_endpoints(struct uas_dev_info *devinfo)
{
struct usb_host_endpoint *eps[4] = { };
@@ -654,13 +682,8 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id)
struct uas_dev_info *devinfo;
struct usb_device *udev = interface_to_usbdev(intf);
- if (id->bInterfaceProtocol == 0x50) {
- int ifnum = intf->cur_altsetting->desc.bInterfaceNumber;
-/* XXX: Shouldn't assume that 1 is the alternative we want */
- int ret = usb_set_interface(udev, ifnum, 1);
- if (ret)
- return -ENODEV;
- }
+ if (uas_switch_interface(udev, intf))
+ return -ENODEV;
devinfo = kmalloc(sizeof(struct uas_dev_info), GFP_KERNEL);
if (!devinfo)
--
1.7.2.3
From: Matthew Wilcox <[email protected]>
Add a comment to the Sense IU data structure that it's also used for Read
Ready and Write Ready. Remove the 'service response' element since it's
gone from the current draft (04).
Signed-off-by: Matthew Wilcox <[email protected]>
---
drivers/usb/storage/uas.c | 7 +++++--
1 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 339fac3..ca277c2 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -49,14 +49,17 @@ struct command_iu {
__u8 cdb[16]; /* XXX: Overflow-checking tools may misunderstand */
};
+/*
+ * Also used for the Read Ready and Write Ready IUs since they have the
+ * same first four bytes
+ */
struct sense_iu {
__u8 iu_id;
__u8 rsvd1;
__be16 tag;
__be16 status_qual;
__u8 status;
- __u8 service_response;
- __u8 rsvd8[6];
+ __u8 rsvd7[7];
__be16 len;
__u8 sense[SCSI_SENSE_BUFFERSIZE];
};
--
1.7.2.3