From 4400ef311e10666a2e5acf97d040df89cb880cb2 Mon Sep 17 00:00:00 2001 From: Matthew Wilcox Date: Wed, 15 Dec 2010 15:44:02 -0500 Subject: USB: uas: Fix up the Sense IU 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 Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 2054b1e25a65..3c7a24433784 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]; }; -- cgit v1.2.3 From ac563cfd528033ee6e3bb4801b5c73468d0145c8 Mon Sep 17 00:00:00 2001 From: Matthew Wilcox Date: Wed, 15 Dec 2010 15:44:03 -0500 Subject: USB: uas: Use kzalloc instead of kmalloc 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 Signed-off-by: Matthew Wilcox Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 3c7a24433784..721fe376be18 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; -- cgit v1.2.3 From 92a3f767f5cd079351ae04a337c40266e9c6048f Mon Sep 17 00:00:00 2001 From: Matthew Wilcox Date: Wed, 15 Dec 2010 15:44:04 -0500 Subject: USB: uas: Rename sense pipe and sense urb to status pipe and status urb 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 Signed-off-by: Matthew Wilcox Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 721fe376be18..06409a6ca2f4 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); @@ -363,21 +363,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) { @@ -446,7 +446,7 @@ static int uas_queuecommand(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)) { @@ -458,7 +458,7 @@ static int uas_queuecommand(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) { @@ -481,8 +481,8 @@ static int uas_queuecommand(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); -- cgit v1.2.3 From 89dc29051b626756e69db12f3ffb22e49a817bfe Mon Sep 17 00:00:00 2001 From: Matthew Wilcox Date: Wed, 15 Dec 2010 15:44:05 -0500 Subject: USB: uas: Ensure we only bind to a UAS interface 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 Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 37 ++++++++++++++++++++++++++++++------- 1 file changed, 30 insertions(+), 7 deletions(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 06409a6ca2f4..52e5ec1b2e8a 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -582,6 +582,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] = { }; @@ -655,13 +683,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) -- cgit v1.2.3 From 0b83ae960cd7d4a5ee02786ecf41ab45688999bf Mon Sep 17 00:00:00 2001 From: Matthew Wilcox Date: Wed, 15 Dec 2010 15:44:06 -0500 Subject: USB: uas: Use GFP_NOIO instead of GFP_KERNEL in I/O submission path 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 Signed-off-by: Matthew Wilcox Signed-off-by: Greg Kroah-Hartman --- drivers/usb/storage/uas.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/storage') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 52e5ec1b2e8a..8d58c6316111 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); } } -- cgit v1.2.3