From d89bd835326947e6618b97469159d3266016fe0a Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 13 Sep 2013 13:27:11 +0200 Subject: uas: properly reinitialize in uas_eh_bus_reset_handler Signed-off-by: Gerd Hoffmann Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index d966b59f7d7b..fc08ee919439 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -85,6 +85,8 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, struct uas_dev_info *devinfo, gfp_t gfp); static void uas_do_work(struct work_struct *work); static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller); +static void uas_configure_endpoints(struct uas_dev_info *devinfo); +static void uas_free_streams(struct uas_dev_info *devinfo); static DECLARE_WORK(uas_work, uas_do_work); static DEFINE_SPINLOCK(uas_work_lock); @@ -800,7 +802,10 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); + uas_free_streams(devinfo); err = usb_reset_device(udev); + if (!err) + uas_configure_endpoints(devinfo); devinfo->resetting = 0; if (err) { -- cgit v1.2.2 From 1bf8198e6b2da3e30960e95f8d215f44572515ce Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 13 Sep 2013 13:27:12 +0200 Subject: uas: make work list per-device Simplifies locking, we'll protect the list with the device spin lock. Also plugs races which can happen when two devices operate on the global list. While being at it rename the list head from "list" to "work", preparing for the addition of a second list. Signed-off-by: Gerd Hoffmann Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 106 +++++++++++++++++++--------------------------- 1 file changed, 44 insertions(+), 62 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index fc08ee919439..3cf5a5ff3d53 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -51,6 +51,8 @@ struct uas_dev_info { unsigned uas_sense_old:1; struct scsi_cmnd *cmnd; spinlock_t lock; + struct work_struct work; + struct list_head work_list; }; enum { @@ -77,7 +79,7 @@ struct uas_cmd_info { struct urb *cmd_urb; struct urb *data_in_urb; struct urb *data_out_urb; - struct list_head list; + struct list_head work; }; /* I hate forward declarations, but I actually have a loop */ @@ -88,10 +90,6 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller); static void uas_configure_endpoints(struct uas_dev_info *devinfo); static void uas_free_streams(struct uas_dev_info *devinfo); -static DECLARE_WORK(uas_work, uas_do_work); -static DEFINE_SPINLOCK(uas_work_lock); -static LIST_HEAD(uas_work_list); - static void uas_unlink_data_urbs(struct uas_dev_info *devinfo, struct uas_cmd_info *cmdinfo) { @@ -118,75 +116,66 @@ static void uas_unlink_data_urbs(struct uas_dev_info *devinfo, static void uas_do_work(struct work_struct *work) { + struct uas_dev_info *devinfo = + container_of(work, struct uas_dev_info, work); struct uas_cmd_info *cmdinfo; struct uas_cmd_info *temp; - struct list_head list; unsigned long flags; int err; - spin_lock_irq(&uas_work_lock); - list_replace_init(&uas_work_list, &list); - spin_unlock_irq(&uas_work_lock); - - list_for_each_entry_safe(cmdinfo, temp, &list, list) { + spin_lock_irqsave(&devinfo->lock, flags); + list_for_each_entry_safe(cmdinfo, temp, &devinfo->work_list, work) { struct scsi_pointer *scp = (void *)cmdinfo; - struct scsi_cmnd *cmnd = container_of(scp, - struct scsi_cmnd, SCp); - struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata; - spin_lock_irqsave(&devinfo->lock, flags); + struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, + SCp); err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC); - if (!err) + if (!err) { cmdinfo->state &= ~IS_IN_WORK_LIST; - spin_unlock_irqrestore(&devinfo->lock, flags); - if (err) { - list_del(&cmdinfo->list); - spin_lock_irq(&uas_work_lock); - list_add_tail(&cmdinfo->list, &uas_work_list); - spin_unlock_irq(&uas_work_lock); - schedule_work(&uas_work); + list_del(&cmdinfo->work); + } else { + schedule_work(&devinfo->work); } } + spin_unlock_irqrestore(&devinfo->lock, flags); } static void uas_abort_work(struct uas_dev_info *devinfo) { struct uas_cmd_info *cmdinfo; struct uas_cmd_info *temp; - struct list_head list; unsigned long flags; - spin_lock_irq(&uas_work_lock); - list_replace_init(&uas_work_list, &list); - spin_unlock_irq(&uas_work_lock); - spin_lock_irqsave(&devinfo->lock, flags); - list_for_each_entry_safe(cmdinfo, temp, &list, list) { + list_for_each_entry_safe(cmdinfo, temp, &devinfo->work_list, work) { struct scsi_pointer *scp = (void *)cmdinfo; - struct scsi_cmnd *cmnd = container_of(scp, - struct scsi_cmnd, SCp); - struct uas_dev_info *di = (void *)cmnd->device->hostdata; - - if (di == devinfo) { - cmdinfo->state |= COMMAND_ABORTED; - cmdinfo->state &= ~IS_IN_WORK_LIST; - if (devinfo->resetting) { - /* uas_stat_cmplt() will not do that - * when a device reset is in - * progress */ - cmdinfo->state &= ~COMMAND_INFLIGHT; - } - uas_try_complete(cmnd, __func__); - } else { - /* not our uas device, relink into list */ - list_del(&cmdinfo->list); - spin_lock_irq(&uas_work_lock); - list_add_tail(&cmdinfo->list, &uas_work_list); - spin_unlock_irq(&uas_work_lock); + struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, + SCp); + cmdinfo->state |= COMMAND_ABORTED; + cmdinfo->state &= ~IS_IN_WORK_LIST; + if (devinfo->resetting) { + /* uas_stat_cmplt() will not do that + * when a device reset is in + * progress */ + cmdinfo->state &= ~COMMAND_INFLIGHT; } + uas_try_complete(cmnd, __func__); + list_del(&cmdinfo->work); } spin_unlock_irqrestore(&devinfo->lock, flags); } +static void uas_add_work(struct uas_cmd_info *cmdinfo) +{ + struct scsi_pointer *scp = (void *)cmdinfo; + struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); + struct uas_dev_info *devinfo = cmnd->device->hostdata; + + WARN_ON(!spin_is_locked(&devinfo->lock)); + list_add_tail(&cmdinfo->work, &devinfo->work_list); + cmdinfo->state |= IS_IN_WORK_LIST; + schedule_work(&devinfo->work); +} + static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd) { struct sense_iu *sense_iu = urb->transfer_buffer; @@ -288,11 +277,7 @@ static void uas_xfer_data(struct urb *urb, struct scsi_cmnd *cmnd, cmdinfo->state |= direction | SUBMIT_STATUS_URB; err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC); if (err) { - spin_lock(&uas_work_lock); - list_add_tail(&cmdinfo->list, &uas_work_list); - cmdinfo->state |= IS_IN_WORK_LIST; - spin_unlock(&uas_work_lock); - schedule_work(&uas_work); + uas_add_work(cmdinfo); } } @@ -694,11 +679,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, spin_unlock_irqrestore(&devinfo->lock, flags); return SCSI_MLQUEUE_DEVICE_BUSY; } - spin_lock(&uas_work_lock); - list_add_tail(&cmdinfo->list, &uas_work_list); - cmdinfo->state |= IS_IN_WORK_LIST; - spin_unlock(&uas_work_lock); - schedule_work(&uas_work); + uas_add_work(cmdinfo); } spin_unlock_irqrestore(&devinfo->lock, flags); @@ -764,10 +745,8 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) spin_lock_irqsave(&devinfo->lock, flags); cmdinfo->state |= COMMAND_ABORTED; if (cmdinfo->state & IS_IN_WORK_LIST) { - spin_lock(&uas_work_lock); - list_del(&cmdinfo->list); + list_del(&cmdinfo->work); cmdinfo->state &= ~IS_IN_WORK_LIST; - spin_unlock(&uas_work_lock); } if (cmdinfo->state & COMMAND_INFLIGHT) { spin_unlock_irqrestore(&devinfo->lock, flags); @@ -1007,6 +986,8 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) init_usb_anchor(&devinfo->sense_urbs); init_usb_anchor(&devinfo->data_urbs); spin_lock_init(&devinfo->lock); + INIT_WORK(&devinfo->work, uas_do_work); + INIT_LIST_HEAD(&devinfo->work_list); uas_configure_endpoints(devinfo); result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 3); @@ -1050,6 +1031,7 @@ static void uas_disconnect(struct usb_interface *intf) struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; devinfo->resetting = 1; + cancel_work_sync(&devinfo->work); uas_abort_work(devinfo); usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); -- cgit v1.2.2 From 326349f82461918830ed59913a3ccd8ffa9ac46f Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 13 Sep 2013 13:27:13 +0200 Subject: uas: add dead request list This patch adds a new list where all requests which are canceled are added to, so we don't loose them. Then, after killing all inflight urbs on bus reset (and disconnect) we'll walk over the list and clean them up. Without this we can end up with aborted requests lingering around in case of status pipe transfer errors. Signed-off-by: Gerd Hoffmann Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 50 +++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 8 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 3cf5a5ff3d53..f0490382351d 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -53,6 +53,7 @@ struct uas_dev_info { spinlock_t lock; struct work_struct work; struct list_head work_list; + struct list_head dead_list; }; enum { @@ -80,6 +81,7 @@ struct uas_cmd_info { struct urb *data_in_urb; struct urb *data_out_urb; struct list_head work; + struct list_head dead; }; /* I hate forward declarations, but I actually have a loop */ @@ -89,6 +91,7 @@ static void uas_do_work(struct work_struct *work); static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller); static void uas_configure_endpoints(struct uas_dev_info *devinfo); static void uas_free_streams(struct uas_dev_info *devinfo); +static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller); static void uas_unlink_data_urbs(struct uas_dev_info *devinfo, struct uas_cmd_info *cmdinfo) @@ -150,16 +153,12 @@ static void uas_abort_work(struct uas_dev_info *devinfo) struct scsi_pointer *scp = (void *)cmdinfo; struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); + uas_log_cmd_state(cmnd, __func__); + WARN_ON(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; cmdinfo->state &= ~IS_IN_WORK_LIST; - if (devinfo->resetting) { - /* uas_stat_cmplt() will not do that - * when a device reset is in - * progress */ - cmdinfo->state &= ~COMMAND_INFLIGHT; - } - uas_try_complete(cmnd, __func__); list_del(&cmdinfo->work); + list_add_tail(&cmdinfo->dead, &devinfo->dead_list); } spin_unlock_irqrestore(&devinfo->lock, flags); } @@ -176,6 +175,28 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo) schedule_work(&devinfo->work); } +static void uas_zap_dead(struct uas_dev_info *devinfo) +{ + struct uas_cmd_info *cmdinfo; + struct uas_cmd_info *temp; + unsigned long flags; + + spin_lock_irqsave(&devinfo->lock, flags); + list_for_each_entry_safe(cmdinfo, temp, &devinfo->dead_list, dead) { + struct scsi_pointer *scp = (void *)cmdinfo; + struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, + SCp); + uas_log_cmd_state(cmnd, __func__); + WARN_ON(!(cmdinfo->state & COMMAND_ABORTED)); + /* all urbs are killed, clear inflight bits */ + cmdinfo->state &= ~(COMMAND_INFLIGHT | + DATA_IN_URB_INFLIGHT | + DATA_OUT_URB_INFLIGHT); + uas_try_complete(cmnd, __func__); + } + spin_unlock_irqrestore(&devinfo->lock, flags); +} + static void uas_sense(struct urb *urb, struct scsi_cmnd *cmnd) { struct sense_iu *sense_iu = urb->transfer_buffer; @@ -263,6 +284,7 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) if (cmdinfo->state & COMMAND_ABORTED) { scmd_printk(KERN_INFO, cmnd, "abort completed\n"); cmnd->result = DID_ABORT << 16; + list_del(&cmdinfo->dead); } cmnd->scsi_done(cmnd); return 0; @@ -292,7 +314,13 @@ static void uas_stat_cmplt(struct urb *urb) u16 tag; if (urb->status) { - dev_err(&urb->dev->dev, "URB BAD STATUS %d\n", urb->status); + if (urb->status == -ENOENT) { + dev_err(&urb->dev->dev, "stat urb: killed, stream %d\n", + urb->stream_id); + } else { + dev_err(&urb->dev->dev, "stat urb: status %d\n", + urb->status); + } usb_free_urb(urb); return; } @@ -743,7 +771,9 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) uas_log_cmd_state(cmnd, __func__); spin_lock_irqsave(&devinfo->lock, flags); + WARN_ON(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; + list_add_tail(&cmdinfo->dead, &devinfo->dead_list); if (cmdinfo->state & IS_IN_WORK_LIST) { list_del(&cmdinfo->work); cmdinfo->state &= ~IS_IN_WORK_LIST; @@ -776,11 +806,13 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) struct usb_device *udev = devinfo->udev; int err; + shost_printk(KERN_INFO, sdev->host, "%s start\n", __func__); devinfo->resetting = 1; uas_abort_work(devinfo); usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); + uas_zap_dead(devinfo); uas_free_streams(devinfo); err = usb_reset_device(udev); if (!err) @@ -988,6 +1020,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) spin_lock_init(&devinfo->lock); INIT_WORK(&devinfo->work, uas_do_work); INIT_LIST_HEAD(&devinfo->work_list); + INIT_LIST_HEAD(&devinfo->dead_list); uas_configure_endpoints(devinfo); result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 3); @@ -1036,6 +1069,7 @@ static void uas_disconnect(struct usb_interface *intf) usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); + uas_zap_dead(devinfo); scsi_remove_host(shost); uas_free_streams(devinfo); kfree(devinfo); -- cgit v1.2.2 From f491ecbb47d5a709d46401da3cd54ff8ee666ca0 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Fri, 13 Sep 2013 13:27:14 +0200 Subject: uas: replace BUG_ON() + WARN_ON() with WARN_ON_ONCE() Signed-off-by: Gerd Hoffmann Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index f0490382351d..046eedfc3828 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -154,7 +154,7 @@ static void uas_abort_work(struct uas_dev_info *devinfo) struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); uas_log_cmd_state(cmnd, __func__); - WARN_ON(cmdinfo->state & COMMAND_ABORTED); + WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; cmdinfo->state &= ~IS_IN_WORK_LIST; list_del(&cmdinfo->work); @@ -169,7 +169,7 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo) struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); struct uas_dev_info *devinfo = cmnd->device->hostdata; - WARN_ON(!spin_is_locked(&devinfo->lock)); + WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); list_add_tail(&cmdinfo->work, &devinfo->work_list); cmdinfo->state |= IS_IN_WORK_LIST; schedule_work(&devinfo->work); @@ -187,7 +187,7 @@ static void uas_zap_dead(struct uas_dev_info *devinfo) struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); uas_log_cmd_state(cmnd, __func__); - WARN_ON(!(cmdinfo->state & COMMAND_ABORTED)); + WARN_ON_ONCE(!(cmdinfo->state & COMMAND_ABORTED)); /* all urbs are killed, clear inflight bits */ cmdinfo->state &= ~(COMMAND_INFLIGHT | DATA_IN_URB_INFLIGHT | @@ -271,13 +271,13 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; struct uas_dev_info *devinfo = (void *)cmnd->device->hostdata; - WARN_ON(!spin_is_locked(&devinfo->lock)); + WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); if (cmdinfo->state & (COMMAND_INFLIGHT | DATA_IN_URB_INFLIGHT | DATA_OUT_URB_INFLIGHT | UNLINK_DATA_URBS)) return -EBUSY; - BUG_ON(cmdinfo->state & COMMAND_COMPLETED); + WARN_ON_ONCE(cmdinfo->state & COMMAND_COMPLETED); cmdinfo->state |= COMMAND_COMPLETED; usb_free_urb(cmdinfo->data_in_urb); usb_free_urb(cmdinfo->data_out_urb); @@ -398,8 +398,9 @@ static void uas_data_cmplt(struct urb *urb) sdb = scsi_out(cmnd); cmdinfo->state &= ~DATA_OUT_URB_INFLIGHT; } - BUG_ON(sdb == NULL); - if (urb->status) { + if (sdb == NULL) { + WARN_ON_ONCE(1); + } else if (urb->status) { /* error: no data transfered */ sdb->resid = sdb->length; } else { @@ -573,7 +574,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; int err; - WARN_ON(!spin_is_locked(&devinfo->lock)); + WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); if (cmdinfo->state & SUBMIT_STATUS_URB) { err = uas_submit_sense_urb(cmnd->device->host, gfp, cmdinfo->stream); @@ -771,7 +772,7 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) uas_log_cmd_state(cmnd, __func__); spin_lock_irqsave(&devinfo->lock, flags); - WARN_ON(cmdinfo->state & COMMAND_ABORTED); + WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; list_add_tail(&cmdinfo->dead, &devinfo->dead_list); if (cmdinfo->state & IS_IN_WORK_LIST) { -- cgit v1.2.2 From d5f808d3f88e90b13a4839e07b3dc6e715e2ba88 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 17 Oct 2013 19:30:26 +0200 Subject: uas: Urbs must be anchored before submitting them Otherwise they may complete before they get anchored and thus never get unanchored (as the unanchoring is done by the usb core on completion). This commit also remove the usb_get_urb / usb_put_urb around cmd submission + anchoring, since if done in the proper order this is not necessary. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 046eedfc3828..059ce62de4b0 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -531,10 +531,12 @@ static int uas_submit_task_urb(struct scsi_cmnd *cmnd, gfp_t gfp, usb_free_urb, NULL); urb->transfer_flags |= URB_FREE_BUFFER; + usb_anchor_urb(urb, &devinfo->cmd_urbs); err = usb_submit_urb(urb, gfp); - if (err) + if (err) { + usb_unanchor_urb(urb); goto err; - usb_anchor_urb(urb, &devinfo->cmd_urbs); + } return 0; @@ -558,13 +560,14 @@ static int uas_submit_sense_urb(struct Scsi_Host *shost, urb = uas_alloc_sense_urb(devinfo, gfp, shost, stream); if (!urb) return SCSI_MLQUEUE_DEVICE_BUSY; + usb_anchor_urb(urb, &devinfo->sense_urbs); if (usb_submit_urb(urb, gfp)) { + usb_unanchor_urb(urb); shost_printk(KERN_INFO, shost, "sense urb submission failure\n"); usb_free_urb(urb); return SCSI_MLQUEUE_DEVICE_BUSY; } - usb_anchor_urb(urb, &devinfo->sense_urbs); return 0; } @@ -594,14 +597,15 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, } if (cmdinfo->state & SUBMIT_DATA_IN_URB) { + usb_anchor_urb(cmdinfo->data_in_urb, &devinfo->data_urbs); if (usb_submit_urb(cmdinfo->data_in_urb, gfp)) { + usb_unanchor_urb(cmdinfo->data_in_urb); scmd_printk(KERN_INFO, cmnd, "data in urb submission failure\n"); return SCSI_MLQUEUE_DEVICE_BUSY; } cmdinfo->state &= ~SUBMIT_DATA_IN_URB; cmdinfo->state |= DATA_IN_URB_INFLIGHT; - usb_anchor_urb(cmdinfo->data_in_urb, &devinfo->data_urbs); } if (cmdinfo->state & ALLOC_DATA_OUT_URB) { @@ -614,14 +618,15 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, } if (cmdinfo->state & SUBMIT_DATA_OUT_URB) { + usb_anchor_urb(cmdinfo->data_out_urb, &devinfo->data_urbs); if (usb_submit_urb(cmdinfo->data_out_urb, gfp)) { + usb_unanchor_urb(cmdinfo->data_out_urb); scmd_printk(KERN_INFO, cmnd, "data out urb submission failure\n"); return SCSI_MLQUEUE_DEVICE_BUSY; } cmdinfo->state &= ~SUBMIT_DATA_OUT_URB; cmdinfo->state |= DATA_OUT_URB_INFLIGHT; - usb_anchor_urb(cmdinfo->data_out_urb, &devinfo->data_urbs); } if (cmdinfo->state & ALLOC_CMD_URB) { @@ -633,14 +638,13 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, } if (cmdinfo->state & SUBMIT_CMD_URB) { - usb_get_urb(cmdinfo->cmd_urb); + usb_anchor_urb(cmdinfo->cmd_urb, &devinfo->cmd_urbs); if (usb_submit_urb(cmdinfo->cmd_urb, gfp)) { + usb_unanchor_urb(cmdinfo->cmd_urb); scmd_printk(KERN_INFO, cmnd, "cmd urb submission failure\n"); return SCSI_MLQUEUE_DEVICE_BUSY; } - usb_anchor_urb(cmdinfo->cmd_urb, &devinfo->cmd_urbs); - usb_put_urb(cmdinfo->cmd_urb); cmdinfo->cmd_urb = NULL; cmdinfo->state &= ~SUBMIT_CMD_URB; cmdinfo->state |= COMMAND_INFLIGHT; -- cgit v1.2.2 From 6ce8213b3328ae4a1db34339c282144740ac1ec6 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 17 Oct 2013 19:00:45 +0200 Subject: uas: Properly set interface to altsetting 0 on probe failure - Rename labels to properly reflect this - Don't skip free-ing the streams when scsi_init_shared_tag_map fails Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 059ce62de4b0..ec1b22d29501 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -993,8 +993,8 @@ static void uas_free_streams(struct uas_dev_info *devinfo) */ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) { - int result; - struct Scsi_Host *shost; + int result = -ENOMEM; + struct Scsi_Host *shost = NULL; struct uas_dev_info *devinfo; struct usb_device *udev = interface_to_usbdev(intf); @@ -1003,12 +1003,11 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) devinfo = kmalloc(sizeof(struct uas_dev_info), GFP_KERNEL); if (!devinfo) - return -ENOMEM; + goto set_alt0; - result = -ENOMEM; shost = scsi_host_alloc(&uas_host_template, sizeof(void *)); if (!shost) - goto free; + goto set_alt0; shost->max_cmd_len = 16 + 252; shost->max_id = 1; @@ -1030,11 +1029,11 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 3); if (result) - goto free; + goto free_streams; result = scsi_add_host(shost, &intf->dev); if (result) - goto deconfig_eps; + goto free_streams; shost->hostdata[0] = (unsigned long)devinfo; @@ -1042,9 +1041,10 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) usb_set_intfdata(intf, shost); return result; -deconfig_eps: +free_streams: uas_free_streams(devinfo); - free: +set_alt0: + usb_set_interface(udev, intf->altsetting[0].desc.bInterfaceNumber, 0); kfree(devinfo); if (shost) scsi_host_put(shost); -- cgit v1.2.2 From 7e50e0bec45897caeb978e5aecc9184a2dc00df2 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 17 Oct 2013 19:19:04 +0200 Subject: uas: Avoid unnecessary unlock / lock calls around unlink_data_urbs All callers of unlink_data_urbs drop devinfo->lock before calling it, and then immediately take it again after the call. And the first thing unlink_data_urbs does is take the lock again, and the last thing it does is drop it. This commit removes all the unnecessary lock dropping and taking. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index ec1b22d29501..dcaf61197032 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -93,28 +93,26 @@ static void uas_configure_endpoints(struct uas_dev_info *devinfo); static void uas_free_streams(struct uas_dev_info *devinfo); static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller); +/* Must be called with devinfo->lock held, will temporary unlock the lock */ static void uas_unlink_data_urbs(struct uas_dev_info *devinfo, - struct uas_cmd_info *cmdinfo) + struct uas_cmd_info *cmdinfo, + unsigned long *lock_flags) { - unsigned long flags; - /* * The UNLINK_DATA_URBS flag makes sure uas_try_complete * (called by urb completion) doesn't release cmdinfo * underneath us. */ - spin_lock_irqsave(&devinfo->lock, flags); cmdinfo->state |= UNLINK_DATA_URBS; - spin_unlock_irqrestore(&devinfo->lock, flags); + spin_unlock_irqrestore(&devinfo->lock, *lock_flags); if (cmdinfo->data_in_urb) usb_unlink_urb(cmdinfo->data_in_urb); if (cmdinfo->data_out_urb) usb_unlink_urb(cmdinfo->data_out_urb); - spin_lock_irqsave(&devinfo->lock, flags); + spin_lock_irqsave(&devinfo->lock, *lock_flags); cmdinfo->state &= ~UNLINK_DATA_URBS; - spin_unlock_irqrestore(&devinfo->lock, flags); } static void uas_do_work(struct work_struct *work) @@ -361,9 +359,7 @@ static void uas_stat_cmplt(struct urb *urb) uas_sense(urb, cmnd); if (cmnd->result != 0) { /* cancel data transfers on error */ - spin_unlock_irqrestore(&devinfo->lock, flags); - uas_unlink_data_urbs(devinfo, cmdinfo); - spin_lock_irqsave(&devinfo->lock, flags); + uas_unlink_data_urbs(devinfo, cmdinfo, &flags); } cmdinfo->state &= ~COMMAND_INFLIGHT; uas_try_complete(cmnd, __func__); @@ -787,9 +783,7 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) spin_unlock_irqrestore(&devinfo->lock, flags); ret = uas_eh_task_mgmt(cmnd, "ABORT TASK", TMF_ABORT_TASK); } else { - spin_unlock_irqrestore(&devinfo->lock, flags); - uas_unlink_data_urbs(devinfo, cmdinfo); - spin_lock_irqsave(&devinfo->lock, flags); + uas_unlink_data_urbs(devinfo, cmdinfo, &flags); uas_try_complete(cmnd, __func__); spin_unlock_irqrestore(&devinfo->lock, flags); ret = SUCCESS; -- cgit v1.2.2 From a887cd366347c38607f0d9c28ca2baed40cac8fc Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 17 Oct 2013 19:47:28 +0200 Subject: uas: uas_alloc_cmd_urb: drop unused stream_id parameter The cmd endpoint never has streams, so the stream_id parameter is unused. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index dcaf61197032..5eacb8054457 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -454,7 +454,7 @@ static struct urb *uas_alloc_sense_urb(struct uas_dev_info *devinfo, gfp_t gfp, } static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp, - struct scsi_cmnd *cmnd, u16 stream_id) + struct scsi_cmnd *cmnd) { struct usb_device *udev = devinfo->udev; struct scsi_device *sdev = cmnd->device; @@ -626,8 +626,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, } if (cmdinfo->state & ALLOC_CMD_URB) { - cmdinfo->cmd_urb = uas_alloc_cmd_urb(devinfo, gfp, cmnd, - cmdinfo->stream); + cmdinfo->cmd_urb = uas_alloc_cmd_urb(devinfo, gfp, cmnd); if (!cmdinfo->cmd_urb) return SCSI_MLQUEUE_DEVICE_BUSY; cmdinfo->state &= ~ALLOC_CMD_URB; -- cgit v1.2.2 From 6c2334e9019039d7952190e239e6a8f0d10101fe Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 17 Oct 2013 22:20:54 +0200 Subject: uas: Fix uas not working when plugged into an ehci port I thought it would be a good idea to also test uas with usb-2, and it turns out it was, as it did not work. The problem is that the uas driver was passing the bEndpointAddress' direction bit to usb_rcvbulkpipe, the xhci code seems to not care about this, but with the ehci code this causes usb_submit_urb failure. With this fixed the uas code works nicely with an uas device plugged into an ehci port. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 5eacb8054457..6ad5de9639d5 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -948,13 +948,13 @@ static void uas_configure_endpoints(struct uas_dev_info *devinfo) eps[3] = usb_pipe_endpoint(udev, devinfo->data_out_pipe); } else { devinfo->cmd_pipe = usb_sndbulkpipe(udev, - eps[0]->desc.bEndpointAddress); + usb_endpoint_num(&eps[0]->desc)); devinfo->status_pipe = usb_rcvbulkpipe(udev, - eps[1]->desc.bEndpointAddress); + usb_endpoint_num(&eps[1]->desc)); devinfo->data_in_pipe = usb_rcvbulkpipe(udev, - eps[2]->desc.bEndpointAddress); + usb_endpoint_num(&eps[2]->desc)); devinfo->data_out_pipe = usb_sndbulkpipe(udev, - eps[3]->desc.bEndpointAddress); + usb_endpoint_num(&eps[3]->desc)); } devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1, 3, 256, -- cgit v1.2.2 From be326f4c9bdfdff8a85145fb89b0a44c4d20ebc6 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Sun, 22 Sep 2013 16:27:02 +0200 Subject: uas: Fix reset locking Fix the uas_eh_bus_reset_handler not properly taking the usbdev lock before calling usb_device_reset, the usb-core expects this lock to be taken when usb_device_reset is called. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 6ad5de9639d5..36ef82a34131 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -804,6 +804,13 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) struct usb_device *udev = devinfo->udev; int err; + err = usb_lock_device_for_reset(udev, devinfo->intf); + if (err) { + shost_printk(KERN_ERR, sdev->host, + "%s FAILED to get lock err %d\n", __func__, err); + return FAILED; + } + shost_printk(KERN_INFO, sdev->host, "%s start\n", __func__); devinfo->resetting = 1; uas_abort_work(devinfo); @@ -817,6 +824,8 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) uas_configure_endpoints(devinfo); devinfo->resetting = 0; + usb_unlock_device(udev); + if (err) { shost_printk(KERN_INFO, sdev->host, "%s FAILED\n", __func__); return FAILED; -- cgit v1.2.2 From 4de7a3735bdc4219cf57a0d44f92c06d7127a211 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 22 Oct 2013 16:10:44 +0100 Subject: uas: Fix reset handling for externally triggered reset Handle usb-device resets not triggered from uas_eh_bus_reset_handler(), when this happens, disable cmd queuing during the reset, and wait for existing requests to finish in pre_reset. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 36 +++++++++++++++++++++++++++++++----- 1 file changed, 31 insertions(+), 5 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 36ef82a34131..0ee5a05c0a7b 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -18,6 +18,7 @@ #include #include +#include #include #include #include @@ -818,10 +819,7 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); uas_zap_dead(devinfo); - uas_free_streams(devinfo); err = usb_reset_device(udev); - if (!err) - uas_configure_endpoints(devinfo); devinfo->resetting = 0; usb_unlock_device(udev); @@ -1055,13 +1053,41 @@ set_alt0: static int uas_pre_reset(struct usb_interface *intf) { -/* XXX: Need to return 1 if it's not our device in error handling */ + struct Scsi_Host *shost = usb_get_intfdata(intf); + struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + unsigned long flags; + + /* Block new requests */ + spin_lock_irqsave(shost->host_lock, flags); + scsi_block_requests(shost); + spin_unlock_irqrestore(shost->host_lock, flags); + + /* Wait for any pending requests to complete */ + flush_work(&devinfo->work); + if (usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 5000) == 0) { + shost_printk(KERN_ERR, shost, "%s: timed out\n", __func__); + return 1; + } + + uas_free_streams(devinfo); + return 0; } static int uas_post_reset(struct usb_interface *intf) { -/* XXX: Need to return 1 if it's not our device in error handling */ + struct Scsi_Host *shost = usb_get_intfdata(intf); + struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + unsigned long flags; + + uas_configure_endpoints(devinfo); + + spin_lock_irqsave(shost->host_lock, flags); + scsi_report_bus_reset(shost, 0); + spin_unlock_irqrestore(shost->host_lock, flags); + + scsi_unblock_requests(shost); + return 0; } -- cgit v1.2.2 From e52e031498cb51aff4f80a19a56700a127cf2a9a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 23 Oct 2013 14:27:09 +0100 Subject: uas: s/response_ui/response_iu/ Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 0ee5a05c0a7b..33f9dcd68e24 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -46,7 +46,7 @@ struct uas_dev_info { struct usb_anchor sense_urbs; struct usb_anchor data_urbs; int qdepth, resetting; - struct response_ui response; + struct response_iu response; unsigned cmd_pipe, status_pipe, data_in_pipe, data_out_pipe; unsigned use_streams:1; unsigned uas_sense_old:1; -- cgit v1.2.2 From d3f7c1560aee57d0ec293253e0c0e79a84ea3016 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 23 Oct 2013 17:46:17 +0100 Subject: uas: Use all available stream ids If we get ie 16 streams we can use stream-id 1-16, not 1-15. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 33f9dcd68e24..3f021f2fafdf 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -722,7 +722,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, { struct Scsi_Host *shost = cmnd->device->host; struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; - u16 tag = devinfo->qdepth - 1; + u16 tag = devinfo->qdepth; unsigned long flags; spin_lock_irqsave(&devinfo->lock, flags); @@ -843,7 +843,7 @@ static int uas_slave_configure(struct scsi_device *sdev) { struct uas_dev_info *devinfo = sdev->hostdata; scsi_set_tag_type(sdev, MSG_ORDERED_TAG); - scsi_activate_tcq(sdev, devinfo->qdepth - 3); + scsi_activate_tcq(sdev, devinfo->qdepth - 2); return 0; } @@ -1027,7 +1027,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) INIT_LIST_HEAD(&devinfo->dead_list); uas_configure_endpoints(devinfo); - result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 3); + result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 2); if (result) goto free_streams; -- cgit v1.2.2 From e1be067b681054e963dfd01346c0d7fc0f8a63aa Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 21 Oct 2013 08:00:58 +0100 Subject: uas: Add a uas_find_uas_alt_setting helper function This is a preparation patch for teaching usb-storage to not bind to uas devices. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 3f021f2fafdf..54db36541b93 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -892,10 +892,10 @@ static int uas_isnt_supported(struct usb_device *udev) return -ENODEV; } -static int uas_switch_interface(struct usb_device *udev, - struct usb_interface *intf) +static int uas_find_uas_alt_setting(struct usb_interface *intf) { int i; + struct usb_device *udev = interface_to_usbdev(intf); int sg_supported = udev->bus->sg_tablesize != 0; for (i = 0; i < intf->num_altsetting; i++) { @@ -904,15 +904,26 @@ static int uas_switch_interface(struct usb_device *udev, if (uas_is_interface(alt)) { if (!sg_supported) return uas_isnt_supported(udev); - return usb_set_interface(udev, - alt->desc.bInterfaceNumber, - alt->desc.bAlternateSetting); + return alt->desc.bAlternateSetting; } } return -ENODEV; } +static int uas_switch_interface(struct usb_device *udev, + struct usb_interface *intf) +{ + int alt; + + alt = uas_find_uas_alt_setting(intf); + if (alt < 0) + return alt; + + return usb_set_interface(udev, + intf->altsetting[0].desc.bInterfaceNumber, alt); +} + static void uas_configure_endpoints(struct uas_dev_info *devinfo) { struct usb_host_endpoint *eps[4] = { }; -- cgit v1.2.2 From 82aa0387d52dd0db2173b844ad52d114807c189b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 21 Oct 2013 08:53:31 +0100 Subject: uas: Move uas detect code to uas-detect.h This is a preparation patch for teaching usb-storage to not bind to uas devices. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 40 ++-------------------------------------- 1 file changed, 2 insertions(+), 38 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 54db36541b93..6ea892f32f74 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -25,6 +25,8 @@ #include #include +#include "uas-detect.h" + /* * The r00-r01c specs define this version of the SENSE IU data structure. * It's still in use by several different firmware releases. @@ -873,44 +875,6 @@ 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_isnt_supported(struct usb_device *udev) -{ - struct usb_hcd *hcd = bus_to_hcd(udev->bus); - - dev_warn(&udev->dev, "The driver for the USB controller %s does not " - "support scatter-gather which is\n", - hcd->driver->description); - dev_warn(&udev->dev, "required by the UAS driver. Please try an" - "alternative USB controller if you wish to use UAS.\n"); - return -ENODEV; -} - -static int uas_find_uas_alt_setting(struct usb_interface *intf) -{ - int i; - struct usb_device *udev = interface_to_usbdev(intf); - int sg_supported = udev->bus->sg_tablesize != 0; - - for (i = 0; i < intf->num_altsetting; i++) { - struct usb_host_interface *alt = &intf->altsetting[i]; - - if (uas_is_interface(alt)) { - if (!sg_supported) - return uas_isnt_supported(udev); - return alt->desc.bAlternateSetting; - } - } - - return -ENODEV; -} - static int uas_switch_interface(struct usb_device *udev, struct usb_interface *intf) { -- cgit v1.2.2 From 79b4c06112f12c31d03cf22b1ed5ce09423fd887 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 25 Oct 2013 17:04:33 +0100 Subject: uas: Add the posibilty to blacklist uas devices from using the uas driver Once we start supporting uas hardware, and as more and more uas devices become available, we will likely start seeing broken devices. This patch prepares for the inevitable need for blacklisting those devices from using the uas driver (they will use usb-storage instead). Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 6ea892f32f74..e817e72d8673 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -866,7 +867,14 @@ static struct scsi_host_template uas_host_template = { .ordered_tag = 1, }; +#define UNUSUAL_DEV(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax, \ + vendorName, productName, useProtocol, useTransport, \ + initFunction, flags) \ +{ USB_DEVICE_VER(id_vendor, id_product, bcdDeviceMin, bcdDeviceMax), \ + .driver_info = (flags) } + static struct usb_device_id uas_usb_ids[] = { +# include "unusual_uas.h" { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, USB_PR_BULK) }, { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, USB_PR_UAS) }, /* 0xaa is a prototype device I happen to have access to */ @@ -875,6 +883,8 @@ static struct usb_device_id uas_usb_ids[] = { }; MODULE_DEVICE_TABLE(usb, uas_usb_ids); +#undef UNUSUAL_DEV + static int uas_switch_interface(struct usb_device *udev, struct usb_interface *intf) { @@ -973,6 +983,9 @@ 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 (!uas_use_uas_driver(intf, id)) + return -ENODEV; + if (uas_switch_interface(udev, intf)) return -ENODEV; @@ -1083,10 +1096,6 @@ static void uas_disconnect(struct usb_interface *intf) kfree(devinfo); } -/* - * XXX: Should this plug into libusual so we can auto-upgrade devices from - * Bulk-Only to UAS? - */ static struct usb_driver uas_driver = { .name = "uas", .probe = uas_probe, -- cgit v1.2.2 From 34f11e59c3d5d025ad9162d8fd126e0b7ca54f40 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 08:54:48 +0100 Subject: uas: Add uas_find_endpoints() helper function This is a preparation patch for adding better descriptor validation. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index e817e72d8673..1ac66f290fbf 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -898,16 +898,11 @@ static int uas_switch_interface(struct usb_device *udev, intf->altsetting[0].desc.bInterfaceNumber, alt); } -static void uas_configure_endpoints(struct uas_dev_info *devinfo) +static int uas_find_endpoints(struct usb_host_interface *alt, + struct usb_host_endpoint *eps[]) { - struct usb_host_endpoint *eps[4] = { }; - struct usb_interface *intf = devinfo->intf; - struct usb_device *udev = devinfo->udev; - struct usb_host_endpoint *endpoint = intf->cur_altsetting->endpoint; - unsigned i, n_endpoints = intf->cur_altsetting->desc.bNumEndpoints; - - devinfo->uas_sense_old = 0; - devinfo->cmnd = NULL; + struct usb_host_endpoint *endpoint = alt->endpoint; + unsigned i, n_endpoints = alt->desc.bNumEndpoints; for (i = 0; i < n_endpoints; i++) { unsigned char *extra = endpoint[i].extra; @@ -924,12 +919,29 @@ static void uas_configure_endpoints(struct uas_dev_info *devinfo) } } + if (!eps[0] || !eps[1] || !eps[2] || !eps[3]) + return -ENODEV; + + return 0; +} + +static void uas_configure_endpoints(struct uas_dev_info *devinfo) +{ + struct usb_host_endpoint *eps[4] = { }; + struct usb_device *udev = devinfo->udev; + int r; + + devinfo->uas_sense_old = 0; + devinfo->cmnd = NULL; + + r = uas_find_endpoints(devinfo->intf->cur_altsetting, eps); + /* - * Assume that if we didn't find a control pipe descriptor, we're + * Assume that if we didn't find a proper set of descriptors, we're * using a device with old firmware that happens to be set up like * this. */ - if (!eps[0]) { + if (r != 0) { devinfo->cmd_pipe = usb_sndbulkpipe(udev, 1); devinfo->status_pipe = usb_rcvbulkpipe(udev, 1); devinfo->data_in_pipe = usb_rcvbulkpipe(udev, 2); -- cgit v1.2.2 From d495c1baa1b3ba277bb5ae24adeab0600151cba4 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 09:06:54 +0100 Subject: uas: Fix bounds check in uas_find_endpoints The loop uses up to 3 bytes of the endpoint extra data. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 1ac66f290fbf..7662b3e13c4d 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -907,7 +907,7 @@ static int uas_find_endpoints(struct usb_host_interface *alt, for (i = 0; i < n_endpoints; i++) { unsigned char *extra = endpoint[i].extra; int len = endpoint[i].extralen; - while (len > 1) { + while (len >= 3) { if (extra[1] == USB_DT_PIPE_USAGE) { unsigned pipe_id = extra[2]; if (pipe_id > 0 && pipe_id < 5) -- cgit v1.2.2 From d77adc0284beea5783c52b2af49217a37c2114cd Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 10:03:34 +0100 Subject: uas: Move uas_find_endpoints to uas-detect.h No changes, just the move. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 27 --------------------------- 1 file changed, 27 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 7662b3e13c4d..5cedc7f034b6 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -898,33 +898,6 @@ static int uas_switch_interface(struct usb_device *udev, intf->altsetting[0].desc.bInterfaceNumber, alt); } -static int uas_find_endpoints(struct usb_host_interface *alt, - struct usb_host_endpoint *eps[]) -{ - struct usb_host_endpoint *endpoint = alt->endpoint; - unsigned i, n_endpoints = alt->desc.bNumEndpoints; - - for (i = 0; i < n_endpoints; i++) { - unsigned char *extra = endpoint[i].extra; - int len = endpoint[i].extralen; - while (len >= 3) { - if (extra[1] == USB_DT_PIPE_USAGE) { - unsigned pipe_id = extra[2]; - if (pipe_id > 0 && pipe_id < 5) - eps[pipe_id - 1] = &endpoint[i]; - break; - } - len -= extra[0]; - extra += extra[0]; - } - } - - if (!eps[0] || !eps[1] || !eps[2] || !eps[3]) - return -ENODEV; - - return 0; -} - static void uas_configure_endpoints(struct uas_dev_info *devinfo) { struct usb_host_endpoint *eps[4] = { }; -- cgit v1.2.2 From 74d71aec619f33ec1ff5b2090792ab96d840bd3b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 10:10:36 +0100 Subject: uas: Drop fixed endpoint config handling The fixed endpoint config code was only necessary to deal with an early uas prototype which has never been released, so lets drop it and enforce proper uas endpoint descriptors. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 36 +++++++++++------------------------- 1 file changed, 11 insertions(+), 25 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 5cedc7f034b6..754468bbbfdc 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -908,31 +908,17 @@ static void uas_configure_endpoints(struct uas_dev_info *devinfo) devinfo->cmnd = NULL; r = uas_find_endpoints(devinfo->intf->cur_altsetting, eps); - - /* - * Assume that if we didn't find a proper set of descriptors, we're - * using a device with old firmware that happens to be set up like - * this. - */ - if (r != 0) { - devinfo->cmd_pipe = usb_sndbulkpipe(udev, 1); - devinfo->status_pipe = usb_rcvbulkpipe(udev, 1); - devinfo->data_in_pipe = usb_rcvbulkpipe(udev, 2); - devinfo->data_out_pipe = usb_sndbulkpipe(udev, 2); - - eps[1] = usb_pipe_endpoint(udev, devinfo->status_pipe); - eps[2] = usb_pipe_endpoint(udev, devinfo->data_in_pipe); - eps[3] = usb_pipe_endpoint(udev, devinfo->data_out_pipe); - } else { - devinfo->cmd_pipe = usb_sndbulkpipe(udev, - usb_endpoint_num(&eps[0]->desc)); - devinfo->status_pipe = usb_rcvbulkpipe(udev, - usb_endpoint_num(&eps[1]->desc)); - devinfo->data_in_pipe = usb_rcvbulkpipe(udev, - usb_endpoint_num(&eps[2]->desc)); - devinfo->data_out_pipe = usb_sndbulkpipe(udev, - usb_endpoint_num(&eps[3]->desc)); - } + if (r) + return r; + + devinfo->cmd_pipe = usb_sndbulkpipe(udev, + usb_endpoint_num(&eps[0]->desc)); + devinfo->status_pipe = usb_rcvbulkpipe(udev, + usb_endpoint_num(&eps[1]->desc)); + devinfo->data_in_pipe = usb_rcvbulkpipe(udev, + usb_endpoint_num(&eps[2]->desc)); + devinfo->data_out_pipe = usb_sndbulkpipe(udev, + usb_endpoint_num(&eps[3]->desc)); devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1, 3, 256, GFP_KERNEL); -- cgit v1.2.2 From 58d51444cdd066239e9b660d72319d941c758fc3 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 10:23:26 +0100 Subject: uas: Not being able to alloc streams when connected through usb-3 is an error Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 754468bbbfdc..d758faef8664 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -93,7 +93,6 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, struct uas_dev_info *devinfo, gfp_t gfp); static void uas_do_work(struct work_struct *work); static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller); -static void uas_configure_endpoints(struct uas_dev_info *devinfo); static void uas_free_streams(struct uas_dev_info *devinfo); static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller); @@ -898,7 +897,7 @@ static int uas_switch_interface(struct usb_device *udev, intf->altsetting[0].desc.bInterfaceNumber, alt); } -static void uas_configure_endpoints(struct uas_dev_info *devinfo) +static int uas_configure_endpoints(struct uas_dev_info *devinfo) { struct usb_host_endpoint *eps[4] = { }; struct usb_device *udev = devinfo->udev; @@ -920,14 +919,18 @@ static void uas_configure_endpoints(struct uas_dev_info *devinfo) devinfo->data_out_pipe = usb_sndbulkpipe(udev, usb_endpoint_num(&eps[3]->desc)); - devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1, 3, 256, - GFP_KERNEL); - if (devinfo->qdepth < 0) { + if (udev->speed != USB_SPEED_SUPER) { devinfo->qdepth = 256; devinfo->use_streams = 0; } else { + devinfo->qdepth = usb_alloc_streams(devinfo->intf, eps + 1, + 3, 256, GFP_KERNEL); + if (devinfo->qdepth < 0) + return devinfo->qdepth; devinfo->use_streams = 1; } + + return 0; } static void uas_free_streams(struct uas_dev_info *devinfo) @@ -984,7 +987,10 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) INIT_WORK(&devinfo->work, uas_do_work); INIT_LIST_HEAD(&devinfo->work_list); INIT_LIST_HEAD(&devinfo->dead_list); - uas_configure_endpoints(devinfo); + + result = uas_configure_endpoints(devinfo); + if (result) + goto set_alt0; result = scsi_init_shared_tag_map(shost, devinfo->qdepth - 2); if (result) @@ -1039,7 +1045,11 @@ static int uas_post_reset(struct usb_interface *intf) struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; unsigned long flags; - uas_configure_endpoints(devinfo); + if (uas_configure_endpoints(devinfo) != 0) { + shost_printk(KERN_ERR, shost, + "%s: alloc streams error after reset", __func__); + return 1; + } spin_lock_irqsave(shost->host_lock, flags); scsi_report_bus_reset(shost, 0); -- cgit v1.2.2 From 70cf0fba7625987ef16085f458e3869c6e3043c1 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 10:37:23 +0100 Subject: uas: task_mgmt: Kill the sense-urb if we fail to submit the cmd urb Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index d758faef8664..9c6f9f9804fd 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -550,39 +550,38 @@ err: * daft to me. */ -static int uas_submit_sense_urb(struct Scsi_Host *shost, - gfp_t gfp, unsigned int stream) +static struct urb *uas_submit_sense_urb(struct Scsi_Host *shost, + gfp_t gfp, unsigned int stream) { struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; struct urb *urb; urb = uas_alloc_sense_urb(devinfo, gfp, shost, stream); if (!urb) - return SCSI_MLQUEUE_DEVICE_BUSY; + return NULL; usb_anchor_urb(urb, &devinfo->sense_urbs); if (usb_submit_urb(urb, gfp)) { usb_unanchor_urb(urb); shost_printk(KERN_INFO, shost, "sense urb submission failure\n"); usb_free_urb(urb); - return SCSI_MLQUEUE_DEVICE_BUSY; + return NULL; } - return 0; + return urb; } static int uas_submit_urbs(struct scsi_cmnd *cmnd, struct uas_dev_info *devinfo, gfp_t gfp) { struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; - int err; + struct urb *urb; WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); if (cmdinfo->state & SUBMIT_STATUS_URB) { - err = uas_submit_sense_urb(cmnd->device->host, gfp, + urb = uas_submit_sense_urb(cmnd->device->host, gfp, cmdinfo->stream); - if (err) { - return err; - } + if (!urb) + return SCSI_MLQUEUE_DEVICE_BUSY; cmdinfo->state &= ~SUBMIT_STATUS_URB; } @@ -726,10 +725,12 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; u16 tag = devinfo->qdepth; unsigned long flags; + struct urb *sense_urb; spin_lock_irqsave(&devinfo->lock, flags); memset(&devinfo->response, 0, sizeof(devinfo->response)); - if (uas_submit_sense_urb(shost, GFP_ATOMIC, tag)) { + sense_urb = uas_submit_sense_urb(shost, GFP_ATOMIC, tag); + if (!sense_urb) { shost_printk(KERN_INFO, shost, "%s: %s: submit sense urb failed\n", __func__, fname); @@ -741,6 +742,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, "%s: %s: submit task mgmt urb failed\n", __func__, fname); spin_unlock_irqrestore(&devinfo->lock, flags); + usb_kill_urb(sense_urb); return FAILED; } spin_unlock_irqrestore(&devinfo->lock, flags); -- cgit v1.2.2 From b83b86a352280cc8cbbf3760096c703986143b81 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 10:51:00 +0100 Subject: uas: Don't allow more then one task to run at the same time Since we use a fixed tag / stream for tasks we cannot allow more then one to run at the same time. This could happen before this time if a task timed out. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 9c6f9f9804fd..7fc4ad207752 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -53,6 +53,7 @@ struct uas_dev_info { unsigned cmd_pipe, status_pipe, data_in_pipe, data_out_pipe; unsigned use_streams:1; unsigned uas_sense_old:1; + unsigned running_task:1; struct scsi_cmnd *cmnd; spinlock_t lock; struct work_struct work; @@ -195,6 +196,7 @@ static void uas_zap_dead(struct uas_dev_info *devinfo) DATA_OUT_URB_INFLIGHT); uas_try_complete(cmnd, __func__); } + devinfo->running_task = 0; spin_unlock_irqrestore(&devinfo->lock, flags); } @@ -340,6 +342,9 @@ static void uas_stat_cmplt(struct urb *urb) if (!cmnd) { if (iu->iu_id == IU_ID_RESPONSE) { + if (!devinfo->running_task) + dev_warn(&urb->dev->dev, + "stat urb: recv unexpected response iu\n"); /* store results for uas_eh_task_mgmt() */ memcpy(&devinfo->response, iu, sizeof(devinfo->response)); } @@ -726,14 +731,26 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, u16 tag = devinfo->qdepth; unsigned long flags; struct urb *sense_urb; + int result = SUCCESS; spin_lock_irqsave(&devinfo->lock, flags); + + if (devinfo->running_task) { + shost_printk(KERN_INFO, shost, + "%s: %s: error already running a task\n", + __func__, fname); + spin_unlock_irqrestore(&devinfo->lock, flags); + return FAILED; + } + + devinfo->running_task = 1; memset(&devinfo->response, 0, sizeof(devinfo->response)); sense_urb = uas_submit_sense_urb(shost, GFP_ATOMIC, tag); if (!sense_urb) { shost_printk(KERN_INFO, shost, "%s: %s: submit sense urb failed\n", __func__, fname); + devinfo->running_task = 0; spin_unlock_irqrestore(&devinfo->lock, flags); return FAILED; } @@ -741,6 +758,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, shost_printk(KERN_INFO, shost, "%s: %s: submit task mgmt urb failed\n", __func__, fname); + devinfo->running_task = 0; spin_unlock_irqrestore(&devinfo->lock, flags); usb_kill_urb(sense_urb); return FAILED; @@ -748,23 +766,33 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, spin_unlock_irqrestore(&devinfo->lock, flags); if (usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 3000) == 0) { + /* + * Note we deliberately do not clear running_task here. If we + * allow new tasks to be submitted, there is no way to figure + * out if a received response_iu is for the failed task or for + * the new one. A bus-reset will eventually clear running_task. + */ shost_printk(KERN_INFO, shost, "%s: %s timed out\n", __func__, fname); return FAILED; } + + spin_lock_irqsave(&devinfo->lock, flags); + devinfo->running_task = 0; if (be16_to_cpu(devinfo->response.tag) != tag) { shost_printk(KERN_INFO, shost, "%s: %s failed (wrong tag %d/%d)\n", __func__, fname, be16_to_cpu(devinfo->response.tag), tag); - return FAILED; - } - if (devinfo->response.response_code != RC_TMF_COMPLETE) { + result = FAILED; + } else if (devinfo->response.response_code != RC_TMF_COMPLETE) { shost_printk(KERN_INFO, shost, "%s: %s failed (rc 0x%x)\n", __func__, fname, devinfo->response.response_code); - return FAILED; + result = FAILED; } - return SUCCESS; + spin_unlock_irqrestore(&devinfo->lock, flags); + + return result; } static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) @@ -982,6 +1010,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) devinfo->intf = intf; devinfo->udev = udev; devinfo->resetting = 0; + devinfo->running_task = 0; init_usb_anchor(&devinfo->cmd_urbs); init_usb_anchor(&devinfo->sense_urbs); init_usb_anchor(&devinfo->data_urbs); -- cgit v1.2.2 From e36e64930cffd94e1c37fdb82f35989384aa946b Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 7 Nov 2013 08:35:55 +0100 Subject: uas: Use GFP_NOIO rather then GFP_ATOMIC where possible We can sleep in our own workqueue (which is the whole reason for having it), and scsi error handlers are also always called from a context which may sleep. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 7fc4ad207752..8023944f2501 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -133,7 +133,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); - err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_ATOMIC); + err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_NOIO); if (!err) { cmdinfo->state &= ~IS_IN_WORK_LIST; list_del(&cmdinfo->work); @@ -745,7 +745,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, devinfo->running_task = 1; memset(&devinfo->response, 0, sizeof(devinfo->response)); - sense_urb = uas_submit_sense_urb(shost, GFP_ATOMIC, tag); + sense_urb = uas_submit_sense_urb(shost, GFP_NOIO, tag); if (!sense_urb) { shost_printk(KERN_INFO, shost, "%s: %s: submit sense urb failed\n", @@ -754,7 +754,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, spin_unlock_irqrestore(&devinfo->lock, flags); return FAILED; } - if (uas_submit_task_urb(cmnd, GFP_ATOMIC, function, tag)) { + if (uas_submit_task_urb(cmnd, GFP_NOIO, function, tag)) { shost_printk(KERN_INFO, shost, "%s: %s: submit task mgmt urb failed\n", __func__, fname); -- cgit v1.2.2 From 0df1f663f32e5dc28cba68375b09bba5eaad103f Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 7 Nov 2013 08:47:05 +0100 Subject: uas: Add suspend/resume support Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 42 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 8023944f2501..7a16ed8e8aac 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -1091,6 +1091,45 @@ static int uas_post_reset(struct usb_interface *intf) return 0; } +static int uas_suspend(struct usb_interface *intf, pm_message_t message) +{ + struct Scsi_Host *shost = usb_get_intfdata(intf); + struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + + /* Wait for any pending requests to complete */ + flush_work(&devinfo->work); + if (usb_wait_anchor_empty_timeout(&devinfo->sense_urbs, 5000) == 0) { + shost_printk(KERN_ERR, shost, "%s: timed out\n", __func__); + return -ETIME; + } + + return 0; +} + +static int uas_resume(struct usb_interface *intf) +{ + return 0; +} + +static int uas_reset_resume(struct usb_interface *intf) +{ + struct Scsi_Host *shost = usb_get_intfdata(intf); + struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + unsigned long flags; + + if (uas_configure_endpoints(devinfo) != 0) { + shost_printk(KERN_ERR, shost, + "%s: alloc streams error after reset", __func__); + return -EIO; + } + + spin_lock_irqsave(shost->host_lock, flags); + scsi_report_bus_reset(shost, 0); + spin_unlock_irqrestore(shost->host_lock, flags); + + return 0; +} + static void uas_disconnect(struct usb_interface *intf) { struct Scsi_Host *shost = usb_get_intfdata(intf); @@ -1114,6 +1153,9 @@ static struct usb_driver uas_driver = { .disconnect = uas_disconnect, .pre_reset = uas_pre_reset, .post_reset = uas_post_reset, + .suspend = uas_suspend, + .resume = uas_resume, + .reset_resume = uas_reset_resume, .id_table = uas_usb_ids, }; -- cgit v1.2.2 From da65c2bb99542d05f2d8f67efe6627915f4c5ea4 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 11 Nov 2013 11:51:42 +0100 Subject: uas: Reset device on reboot Some BIOS-es will hang on reboot when an uas device is attached and left in uas mode on reboot. This commit adds a shutdown handler which on reboot puts the device back into usb-storage mode, fixing the hang on reboot on these systems. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 7a16ed8e8aac..019f2030ea0c 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -54,6 +54,7 @@ struct uas_dev_info { unsigned use_streams:1; unsigned uas_sense_old:1; unsigned running_task:1; + unsigned shutdown:1; struct scsi_cmnd *cmnd; spinlock_t lock; struct work_struct work; @@ -1011,6 +1012,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) devinfo->udev = udev; devinfo->resetting = 0; devinfo->running_task = 0; + devinfo->shutdown = 0; init_usb_anchor(&devinfo->cmd_urbs); init_usb_anchor(&devinfo->sense_urbs); init_usb_anchor(&devinfo->data_urbs); @@ -1053,6 +1055,9 @@ static int uas_pre_reset(struct usb_interface *intf) struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; unsigned long flags; + if (devinfo->shutdown) + return 0; + /* Block new requests */ spin_lock_irqsave(shost->host_lock, flags); scsi_block_requests(shost); @@ -1076,6 +1081,9 @@ static int uas_post_reset(struct usb_interface *intf) struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; unsigned long flags; + if (devinfo->shutdown) + return 0; + if (uas_configure_endpoints(devinfo) != 0) { shost_printk(KERN_ERR, shost, "%s: alloc streams error after reset", __func__); @@ -1147,6 +1155,27 @@ static void uas_disconnect(struct usb_interface *intf) kfree(devinfo); } +/* + * Put the device back in usb-storage mode on shutdown, as some BIOS-es + * hang on reboot when the device is still in uas mode. Note the reset is + * necessary as some devices won't revert to usb-storage mode without it. + */ +static void uas_shutdown(struct device *dev) +{ + struct usb_interface *intf = to_usb_interface(dev); + struct usb_device *udev = interface_to_usbdev(intf); + struct Scsi_Host *shost = usb_get_intfdata(intf); + struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + + if (system_state != SYSTEM_RESTART) + return; + + devinfo->shutdown = 1; + uas_free_streams(devinfo); + usb_set_interface(udev, intf->altsetting[0].desc.bInterfaceNumber, 0); + usb_reset_device(udev); +} + static struct usb_driver uas_driver = { .name = "uas", .probe = uas_probe, @@ -1156,6 +1185,7 @@ static struct usb_driver uas_driver = { .suspend = uas_suspend, .resume = uas_resume, .reset_resume = uas_reset_resume, + .drvwrap.driver.shutdown = uas_shutdown, .id_table = uas_usb_ids, }; -- cgit v1.2.2 From f323abcda35ea4bae851c9be8f115ee45cc9cf22 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Nov 2013 10:51:33 +0100 Subject: uas: Fix task-management not working when connected over USB-2 For USB-2 connections the stream-id must always be 0. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 019f2030ea0c..10e580e56d4c 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -746,7 +746,8 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, devinfo->running_task = 1; memset(&devinfo->response, 0, sizeof(devinfo->response)); - sense_urb = uas_submit_sense_urb(shost, GFP_NOIO, tag); + sense_urb = uas_submit_sense_urb(shost, GFP_NOIO, + devinfo->use_streams ? tag : 0); if (!sense_urb) { shost_printk(KERN_INFO, shost, "%s: %s: submit sense urb failed\n", -- cgit v1.2.2 From c6d4579d4ba24c494d03daf656cd2ff2a9e683c6 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Nov 2013 10:53:57 +0100 Subject: uas: uas_alloc_data_urb: Remove unnecessary use_streams check uas_alloc_data_urb always gets called with a stream_id value of 0 when not using streams. Removing the check makes it consistent with uas_alloc_sense_urb. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 10e580e56d4c..e06505c8f6f0 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -429,8 +429,7 @@ static struct urb *uas_alloc_data_urb(struct uas_dev_info *devinfo, gfp_t gfp, goto out; usb_fill_bulk_urb(urb, udev, pipe, NULL, sdb->length, uas_data_cmplt, cmnd); - if (devinfo->use_streams) - urb->stream_id = stream_id; + urb->stream_id = stream_id; urb->num_sgs = udev->bus->sg_tablesize ? sdb->table.nents : 0; urb->sg = sdb->table.sgl; out: -- cgit v1.2.2 From 61c09ce510a1eba8595beda6aac194f42571d768 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Nov 2013 13:44:20 +0100 Subject: uas: Properly complete inflight commands on bus-reset or disconnect Before this commit the uas driver would keep track of scsi commands which still need to have some urbs submitted to the device, and complete this with an ABORT result code on bus-reset or disconnect, but in flight scsi commands which have all their urbs submitted, and thus are not part of the work list, would never get their done callback called. The problem is killed sense urbs don't have any tag info, so it is impossible to tell which scsi cmd they belong to, so merely making sure all the urbs have completed one way or the other is not enough. This commit fixes this by changing the work list to an inflight list, which keeps tracks of all inflight scsi cmnds, using the IS_IN_WORK_LIST flag to determine if actual work needs to be done in uas_do_work(), and by moving marking all inflight scsi commands as aborted and moving them to the dead list on bus-reset or disconnect. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index e06505c8f6f0..1a188399e090 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -58,7 +58,7 @@ struct uas_dev_info { struct scsi_cmnd *cmnd; spinlock_t lock; struct work_struct work; - struct list_head work_list; + struct list_head inflight_list; struct list_head dead_list; }; @@ -86,7 +86,7 @@ struct uas_cmd_info { struct urb *cmd_urb; struct urb *data_in_urb; struct urb *data_out_urb; - struct list_head work; + struct list_head inflight; struct list_head dead; }; @@ -125,34 +125,36 @@ static void uas_do_work(struct work_struct *work) struct uas_dev_info *devinfo = container_of(work, struct uas_dev_info, work); struct uas_cmd_info *cmdinfo; - struct uas_cmd_info *temp; unsigned long flags; int err; spin_lock_irqsave(&devinfo->lock, flags); - list_for_each_entry_safe(cmdinfo, temp, &devinfo->work_list, work) { + list_for_each_entry(cmdinfo, &devinfo->inflight_list, inflight) { struct scsi_pointer *scp = (void *)cmdinfo; struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); + + if (!(cmdinfo->state & IS_IN_WORK_LIST)) + continue; + err = uas_submit_urbs(cmnd, cmnd->device->hostdata, GFP_NOIO); - if (!err) { + if (!err) cmdinfo->state &= ~IS_IN_WORK_LIST; - list_del(&cmdinfo->work); - } else { + else schedule_work(&devinfo->work); - } } spin_unlock_irqrestore(&devinfo->lock, flags); } -static void uas_abort_work(struct uas_dev_info *devinfo) +static void uas_abort_inflight(struct uas_dev_info *devinfo) { struct uas_cmd_info *cmdinfo; struct uas_cmd_info *temp; unsigned long flags; spin_lock_irqsave(&devinfo->lock, flags); - list_for_each_entry_safe(cmdinfo, temp, &devinfo->work_list, work) { + list_for_each_entry_safe(cmdinfo, temp, &devinfo->inflight_list, + inflight) { struct scsi_pointer *scp = (void *)cmdinfo; struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); @@ -160,7 +162,7 @@ static void uas_abort_work(struct uas_dev_info *devinfo) WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; cmdinfo->state &= ~IS_IN_WORK_LIST; - list_del(&cmdinfo->work); + list_del(&cmdinfo->inflight); list_add_tail(&cmdinfo->dead, &devinfo->dead_list); } spin_unlock_irqrestore(&devinfo->lock, flags); @@ -173,7 +175,6 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo) struct uas_dev_info *devinfo = cmnd->device->hostdata; WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); - list_add_tail(&cmdinfo->work, &devinfo->work_list); cmdinfo->state |= IS_IN_WORK_LIST; schedule_work(&devinfo->work); } @@ -289,7 +290,8 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) scmd_printk(KERN_INFO, cmnd, "abort completed\n"); cmnd->result = DID_ABORT << 16; list_del(&cmdinfo->dead); - } + } else + list_del(&cmdinfo->inflight); cmnd->scsi_done(cmnd); return 0; } @@ -717,6 +719,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, uas_add_work(cmdinfo); } + list_add_tail(&cmdinfo->inflight, &devinfo->inflight_list); spin_unlock_irqrestore(&devinfo->lock, flags); return 0; } @@ -807,11 +810,9 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) spin_lock_irqsave(&devinfo->lock, flags); WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; + cmdinfo->state &= ~IS_IN_WORK_LIST; + list_del(&cmdinfo->inflight); list_add_tail(&cmdinfo->dead, &devinfo->dead_list); - if (cmdinfo->state & IS_IN_WORK_LIST) { - list_del(&cmdinfo->work); - cmdinfo->state &= ~IS_IN_WORK_LIST; - } if (cmdinfo->state & COMMAND_INFLIGHT) { spin_unlock_irqrestore(&devinfo->lock, flags); ret = uas_eh_task_mgmt(cmnd, "ABORT TASK", TMF_ABORT_TASK); @@ -847,7 +848,7 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) shost_printk(KERN_INFO, sdev->host, "%s start\n", __func__); devinfo->resetting = 1; - uas_abort_work(devinfo); + uas_abort_inflight(devinfo); usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); @@ -1018,7 +1019,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) init_usb_anchor(&devinfo->data_urbs); spin_lock_init(&devinfo->lock); INIT_WORK(&devinfo->work, uas_do_work); - INIT_LIST_HEAD(&devinfo->work_list); + INIT_LIST_HEAD(&devinfo->inflight_list); INIT_LIST_HEAD(&devinfo->dead_list); result = uas_configure_endpoints(devinfo); @@ -1145,7 +1146,7 @@ static void uas_disconnect(struct usb_interface *intf) devinfo->resetting = 1; cancel_work_sync(&devinfo->work); - uas_abort_work(devinfo); + uas_abort_inflight(devinfo); usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); -- cgit v1.2.2 From da3033ea08397fb70279f22789002e6001432f3d Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Nov 2013 13:57:24 +0100 Subject: uas: add uas_mark_cmd_dead helper function Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 35 ++++++++++++++++++----------------- 1 file changed, 18 insertions(+), 17 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 1a188399e090..7810c135a69e 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -146,6 +146,21 @@ static void uas_do_work(struct work_struct *work) spin_unlock_irqrestore(&devinfo->lock, flags); } +static void uas_mark_cmd_dead(struct uas_dev_info *devinfo, + struct uas_cmd_info *cmdinfo, const char *caller) +{ + struct scsi_pointer *scp = (void *)cmdinfo; + struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); + + uas_log_cmd_state(cmnd, caller); + WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); + WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); + cmdinfo->state |= COMMAND_ABORTED; + cmdinfo->state &= ~IS_IN_WORK_LIST; + list_del(&cmdinfo->inflight); + list_add_tail(&cmdinfo->dead, &devinfo->dead_list); +} + static void uas_abort_inflight(struct uas_dev_info *devinfo) { struct uas_cmd_info *cmdinfo; @@ -154,17 +169,8 @@ static void uas_abort_inflight(struct uas_dev_info *devinfo) spin_lock_irqsave(&devinfo->lock, flags); list_for_each_entry_safe(cmdinfo, temp, &devinfo->inflight_list, - inflight) { - struct scsi_pointer *scp = (void *)cmdinfo; - struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, - SCp); - uas_log_cmd_state(cmnd, __func__); - WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); - cmdinfo->state |= COMMAND_ABORTED; - cmdinfo->state &= ~IS_IN_WORK_LIST; - list_del(&cmdinfo->inflight); - list_add_tail(&cmdinfo->dead, &devinfo->dead_list); - } + inflight) + uas_mark_cmd_dead(devinfo, cmdinfo, __func__); spin_unlock_irqrestore(&devinfo->lock, flags); } @@ -806,13 +812,8 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) unsigned long flags; int ret; - uas_log_cmd_state(cmnd, __func__); spin_lock_irqsave(&devinfo->lock, flags); - WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); - cmdinfo->state |= COMMAND_ABORTED; - cmdinfo->state &= ~IS_IN_WORK_LIST; - list_del(&cmdinfo->inflight); - list_add_tail(&cmdinfo->dead, &devinfo->dead_list); + uas_mark_cmd_dead(devinfo, cmdinfo, __func__); if (cmdinfo->state & COMMAND_INFLIGHT) { spin_unlock_irqrestore(&devinfo->lock, flags); ret = uas_eh_task_mgmt(cmnd, "ABORT TASK", TMF_ABORT_TASK); -- cgit v1.2.2 From 040d1a8f11f390f36a8cd7fc04c0c836639b0b6a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 12 Nov 2013 14:02:12 +0100 Subject: uas: cmdinfo: use only one list head cmds are either on the inflight list or on the dead list, never both, so we only need one list head. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 7810c135a69e..cfe0102fcbae 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -86,8 +86,7 @@ struct uas_cmd_info { struct urb *cmd_urb; struct urb *data_in_urb; struct urb *data_out_urb; - struct list_head inflight; - struct list_head dead; + struct list_head list; }; /* I hate forward declarations, but I actually have a loop */ @@ -129,7 +128,7 @@ static void uas_do_work(struct work_struct *work) int err; spin_lock_irqsave(&devinfo->lock, flags); - list_for_each_entry(cmdinfo, &devinfo->inflight_list, inflight) { + list_for_each_entry(cmdinfo, &devinfo->inflight_list, list) { struct scsi_pointer *scp = (void *)cmdinfo; struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); @@ -157,8 +156,7 @@ static void uas_mark_cmd_dead(struct uas_dev_info *devinfo, WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; cmdinfo->state &= ~IS_IN_WORK_LIST; - list_del(&cmdinfo->inflight); - list_add_tail(&cmdinfo->dead, &devinfo->dead_list); + list_move_tail(&cmdinfo->list, &devinfo->dead_list); } static void uas_abort_inflight(struct uas_dev_info *devinfo) @@ -168,8 +166,7 @@ static void uas_abort_inflight(struct uas_dev_info *devinfo) unsigned long flags; spin_lock_irqsave(&devinfo->lock, flags); - list_for_each_entry_safe(cmdinfo, temp, &devinfo->inflight_list, - inflight) + list_for_each_entry_safe(cmdinfo, temp, &devinfo->inflight_list, list) uas_mark_cmd_dead(devinfo, cmdinfo, __func__); spin_unlock_irqrestore(&devinfo->lock, flags); } @@ -192,7 +189,7 @@ static void uas_zap_dead(struct uas_dev_info *devinfo) unsigned long flags; spin_lock_irqsave(&devinfo->lock, flags); - list_for_each_entry_safe(cmdinfo, temp, &devinfo->dead_list, dead) { + list_for_each_entry_safe(cmdinfo, temp, &devinfo->dead_list, list) { struct scsi_pointer *scp = (void *)cmdinfo; struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); @@ -295,9 +292,8 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) if (cmdinfo->state & COMMAND_ABORTED) { scmd_printk(KERN_INFO, cmnd, "abort completed\n"); cmnd->result = DID_ABORT << 16; - list_del(&cmdinfo->dead); - } else - list_del(&cmdinfo->inflight); + } + list_del(&cmdinfo->list); cmnd->scsi_done(cmnd); return 0; } @@ -725,7 +721,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, uas_add_work(cmdinfo); } - list_add_tail(&cmdinfo->inflight, &devinfo->inflight_list); + list_add_tail(&cmdinfo->list, &devinfo->inflight_list); spin_unlock_irqrestore(&devinfo->lock, flags); return 0; } -- cgit v1.2.2 From c6f63207a3ba689025b2120792ea831cf72f9a81 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 13 Nov 2013 09:24:15 +0100 Subject: uas: Fix command / task mgmt submission racing with disconnect Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index cfe0102fcbae..8c685801e267 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -670,13 +670,15 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, BUILD_BUG_ON(sizeof(struct uas_cmd_info) > sizeof(struct scsi_pointer)); + spin_lock_irqsave(&devinfo->lock, flags); + if (devinfo->resetting) { cmnd->result = DID_ERROR << 16; cmnd->scsi_done(cmnd); + spin_unlock_irqrestore(&devinfo->lock, flags); return 0; } - spin_lock_irqsave(&devinfo->lock, flags); if (devinfo->cmnd) { spin_unlock_irqrestore(&devinfo->lock, flags); return SCSI_MLQUEUE_DEVICE_BUSY; @@ -740,6 +742,11 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, spin_lock_irqsave(&devinfo->lock, flags); + if (devinfo->resetting) { + spin_unlock_irqrestore(&devinfo->lock, flags); + return FAILED; + } + if (devinfo->running_task) { shost_printk(KERN_INFO, shost, "%s: %s: error already running a task\n", @@ -809,6 +816,12 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) int ret; spin_lock_irqsave(&devinfo->lock, flags); + + if (devinfo->resetting) { + spin_unlock_irqrestore(&devinfo->lock, flags); + return FAILED; + } + uas_mark_cmd_dead(devinfo, cmdinfo, __func__); if (cmdinfo->state & COMMAND_INFLIGHT) { spin_unlock_irqrestore(&devinfo->lock, flags); -- cgit v1.2.2 From 21fc05b680f6fba868b41e2713ade3fdea4049f9 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Wed, 13 Nov 2013 09:32:22 +0100 Subject: uas: Fix memory management The scsi-host structure is refcounted, scsi_remove_host tears down the scsi-host but does not decrement the refcount, so we need to call scsi_put_host on disconnect to get the underlying memory to be freed. After calling scsi_remove_host, the scsi-core may still hold a reference to the scsi-host, iow we may still get called after uas_disconnect, but we do our own life cycle management of uas_devinfo, freeing it on disconnect, and thus may end up using devinfo after it has been freed. Switch to letting scsi_host_alloc allocate and manage the memory for us. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 33 ++++++++++++++------------------- 1 file changed, 14 insertions(+), 19 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 8c685801e267..d81d041842f4 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -315,7 +315,7 @@ static void uas_stat_cmplt(struct urb *urb) { struct iu *iu = urb->transfer_buffer; struct Scsi_Host *shost = urb->context; - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; struct scsi_cmnd *cmnd; struct uas_cmd_info *cmdinfo; unsigned long flags; @@ -562,7 +562,7 @@ err: static struct urb *uas_submit_sense_urb(struct Scsi_Host *shost, gfp_t gfp, unsigned int stream) { - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; struct urb *urb; urb = uas_alloc_sense_urb(devinfo, gfp, shost, stream); @@ -734,7 +734,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, const char *fname, u8 function) { struct Scsi_Host *shost = cmnd->device->host; - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; u16 tag = devinfo->qdepth; unsigned long flags; struct urb *sense_urb; @@ -879,7 +879,7 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) static int uas_slave_alloc(struct scsi_device *sdev) { - sdev->hostdata = (void *)sdev->host->hostdata[0]; + sdev->hostdata = (void *)sdev->host->hostdata; return 0; } @@ -1005,11 +1005,8 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) if (uas_switch_interface(udev, intf)) return -ENODEV; - devinfo = kmalloc(sizeof(struct uas_dev_info), GFP_KERNEL); - if (!devinfo) - goto set_alt0; - - shost = scsi_host_alloc(&uas_host_template, sizeof(void *)); + shost = scsi_host_alloc(&uas_host_template, + sizeof(struct uas_dev_info)); if (!shost) goto set_alt0; @@ -1019,6 +1016,7 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) shost->max_channel = 0; shost->sg_tablesize = udev->bus->sg_tablesize; + devinfo = (struct uas_dev_info *)shost->hostdata; devinfo->intf = intf; devinfo->udev = udev; devinfo->resetting = 0; @@ -1044,8 +1042,6 @@ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) if (result) goto free_streams; - shost->hostdata[0] = (unsigned long)devinfo; - scsi_scan_host(shost); usb_set_intfdata(intf, shost); return result; @@ -1054,7 +1050,6 @@ free_streams: uas_free_streams(devinfo); set_alt0: usb_set_interface(udev, intf->altsetting[0].desc.bInterfaceNumber, 0); - kfree(devinfo); if (shost) scsi_host_put(shost); return result; @@ -1063,7 +1058,7 @@ set_alt0: static int uas_pre_reset(struct usb_interface *intf) { struct Scsi_Host *shost = usb_get_intfdata(intf); - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; unsigned long flags; if (devinfo->shutdown) @@ -1089,7 +1084,7 @@ static int uas_pre_reset(struct usb_interface *intf) static int uas_post_reset(struct usb_interface *intf) { struct Scsi_Host *shost = usb_get_intfdata(intf); - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; unsigned long flags; if (devinfo->shutdown) @@ -1113,7 +1108,7 @@ static int uas_post_reset(struct usb_interface *intf) static int uas_suspend(struct usb_interface *intf, pm_message_t message) { struct Scsi_Host *shost = usb_get_intfdata(intf); - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; /* Wait for any pending requests to complete */ flush_work(&devinfo->work); @@ -1133,7 +1128,7 @@ static int uas_resume(struct usb_interface *intf) static int uas_reset_resume(struct usb_interface *intf) { struct Scsi_Host *shost = usb_get_intfdata(intf); - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; unsigned long flags; if (uas_configure_endpoints(devinfo) != 0) { @@ -1152,7 +1147,7 @@ static int uas_reset_resume(struct usb_interface *intf) static void uas_disconnect(struct usb_interface *intf) { struct Scsi_Host *shost = usb_get_intfdata(intf); - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; devinfo->resetting = 1; cancel_work_sync(&devinfo->work); @@ -1163,7 +1158,7 @@ static void uas_disconnect(struct usb_interface *intf) uas_zap_dead(devinfo); scsi_remove_host(shost); uas_free_streams(devinfo); - kfree(devinfo); + scsi_host_put(shost); } /* @@ -1176,7 +1171,7 @@ static void uas_shutdown(struct device *dev) struct usb_interface *intf = to_usb_interface(dev); struct usb_device *udev = interface_to_usbdev(intf); struct Scsi_Host *shost = usb_get_intfdata(intf); - struct uas_dev_info *devinfo = (void *)shost->hostdata[0]; + struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; if (system_state != SYSTEM_RESTART) return; -- cgit v1.2.2 From 3a4462e0e2fe8f715f54147d36b5433a7ff5a85a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 14 Nov 2013 11:06:13 +0100 Subject: uas: Clear cmdinfo on command queue-ing The scsi error handling path re-uses previously queued up (and errored-out) cmds. If such a re-used cmd had a data-phase then cmdinfo will have data_in_urb / data_out_urb still set to the free-ed urbs from the errored-out cmd, and they will get free-ed a second time when the error handling cmd completes, corrupting the kernel heap. Clearing cmdinfo on command queue-ing fixes this, and seems like a good idea in general. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index d81d041842f4..fceffccc1be1 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -684,6 +684,8 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd, return SCSI_MLQUEUE_DEVICE_BUSY; } + memset(cmdinfo, 0, sizeof(*cmdinfo)); + if (blk_rq_tagged(cmnd->request)) { cmdinfo->stream = cmnd->request->tag + 2; } else { -- cgit v1.2.2 From 673331c87c492898a9152f3754f3174128e1514a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 14 Nov 2013 14:27:27 +0100 Subject: uas: Use the right error codes for different kinds of errors Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index fceffccc1be1..6ec48c2daf45 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -146,7 +146,8 @@ static void uas_do_work(struct work_struct *work) } static void uas_mark_cmd_dead(struct uas_dev_info *devinfo, - struct uas_cmd_info *cmdinfo, const char *caller) + struct uas_cmd_info *cmdinfo, + int result, const char *caller) { struct scsi_pointer *scp = (void *)cmdinfo; struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd, SCp); @@ -156,10 +157,12 @@ static void uas_mark_cmd_dead(struct uas_dev_info *devinfo, WARN_ON_ONCE(cmdinfo->state & COMMAND_ABORTED); cmdinfo->state |= COMMAND_ABORTED; cmdinfo->state &= ~IS_IN_WORK_LIST; + cmnd->result = result << 16; list_move_tail(&cmdinfo->list, &devinfo->dead_list); } -static void uas_abort_inflight(struct uas_dev_info *devinfo) +static void uas_abort_inflight(struct uas_dev_info *devinfo, int result, + const char *caller) { struct uas_cmd_info *cmdinfo; struct uas_cmd_info *temp; @@ -167,7 +170,7 @@ static void uas_abort_inflight(struct uas_dev_info *devinfo) spin_lock_irqsave(&devinfo->lock, flags); list_for_each_entry_safe(cmdinfo, temp, &devinfo->inflight_list, list) - uas_mark_cmd_dead(devinfo, cmdinfo, __func__); + uas_mark_cmd_dead(devinfo, cmdinfo, result, caller); spin_unlock_irqrestore(&devinfo->lock, flags); } @@ -289,10 +292,8 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller) cmdinfo->state |= COMMAND_COMPLETED; usb_free_urb(cmdinfo->data_in_urb); usb_free_urb(cmdinfo->data_out_urb); - if (cmdinfo->state & COMMAND_ABORTED) { + if (cmdinfo->state & COMMAND_ABORTED) scmd_printk(KERN_INFO, cmnd, "abort completed\n"); - cmnd->result = DID_ABORT << 16; - } list_del(&cmdinfo->list); cmnd->scsi_done(cmnd); return 0; @@ -824,7 +825,7 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd) return FAILED; } - uas_mark_cmd_dead(devinfo, cmdinfo, __func__); + uas_mark_cmd_dead(devinfo, cmdinfo, DID_ABORT, __func__); if (cmdinfo->state & COMMAND_INFLIGHT) { spin_unlock_irqrestore(&devinfo->lock, flags); ret = uas_eh_task_mgmt(cmnd, "ABORT TASK", TMF_ABORT_TASK); @@ -860,7 +861,7 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) shost_printk(KERN_INFO, sdev->host, "%s start\n", __func__); devinfo->resetting = 1; - uas_abort_inflight(devinfo); + uas_abort_inflight(devinfo, DID_RESET, __func__); usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); @@ -1153,7 +1154,7 @@ static void uas_disconnect(struct usb_interface *intf) devinfo->resetting = 1; cancel_work_sync(&devinfo->work); - uas_abort_inflight(devinfo); + uas_abort_inflight(devinfo, DID_NO_CONNECT, __func__); usb_kill_anchored_urbs(&devinfo->cmd_urbs); usb_kill_anchored_urbs(&devinfo->sense_urbs); usb_kill_anchored_urbs(&devinfo->data_urbs); -- cgit v1.2.2 From 876285cc9cf418f626375f28bb0fc5d88012f12d Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Thu, 7 Nov 2013 08:52:42 +0100 Subject: uas: Improve error reporting Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 60 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 46 insertions(+), 14 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 6ec48c2daf45..f09205b162e4 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -411,6 +411,12 @@ static void uas_data_cmplt(struct urb *urb) if (sdb == NULL) { WARN_ON_ONCE(1); } else if (urb->status) { + if (urb->status != -ECONNRESET) { + uas_log_cmd_state(cmnd, __func__); + scmd_printk(KERN_ERR, cmnd, + "data cmplt err %d stream %d\n", + urb->status, urb->stream_id); + } /* error: no data transfered */ sdb->resid = sdb->length; } else { @@ -420,6 +426,17 @@ static void uas_data_cmplt(struct urb *urb) spin_unlock_irqrestore(&devinfo->lock, flags); } +static void uas_cmd_cmplt(struct urb *urb) +{ + struct scsi_cmnd *cmnd = urb->context; + + if (urb->status) { + uas_log_cmd_state(cmnd, __func__); + scmd_printk(KERN_ERR, cmnd, "cmd cmplt err %d\n", urb->status); + } + usb_free_urb(urb); +} + static struct urb *uas_alloc_data_urb(struct uas_dev_info *devinfo, gfp_t gfp, unsigned int pipe, u16 stream_id, struct scsi_cmnd *cmnd, @@ -497,7 +514,7 @@ static struct urb *uas_alloc_cmd_urb(struct uas_dev_info *devinfo, gfp_t gfp, memcpy(iu->cdb, cmnd->cmnd, cmnd->cmd_len); usb_fill_bulk_urb(urb, udev, devinfo->cmd_pipe, iu, sizeof(*iu) + len, - usb_free_urb, NULL); + uas_cmd_cmplt, cmnd); urb->transfer_flags |= URB_FREE_BUFFER; out: return urb; @@ -537,13 +554,15 @@ static int uas_submit_task_urb(struct scsi_cmnd *cmnd, gfp_t gfp, } usb_fill_bulk_urb(urb, udev, devinfo->cmd_pipe, iu, sizeof(*iu), - usb_free_urb, NULL); + uas_cmd_cmplt, cmnd); urb->transfer_flags |= URB_FREE_BUFFER; usb_anchor_urb(urb, &devinfo->cmd_urbs); err = usb_submit_urb(urb, gfp); if (err) { usb_unanchor_urb(urb); + uas_log_cmd_state(cmnd, __func__); + scmd_printk(KERN_ERR, cmnd, "task submission err %d\n", err); goto err; } @@ -560,20 +579,25 @@ err: * daft to me. */ -static struct urb *uas_submit_sense_urb(struct Scsi_Host *shost, +static struct urb *uas_submit_sense_urb(struct scsi_cmnd *cmnd, gfp_t gfp, unsigned int stream) { + struct Scsi_Host *shost = cmnd->device->host; struct uas_dev_info *devinfo = (struct uas_dev_info *)shost->hostdata; struct urb *urb; + int err; urb = uas_alloc_sense_urb(devinfo, gfp, shost, stream); if (!urb) return NULL; usb_anchor_urb(urb, &devinfo->sense_urbs); - if (usb_submit_urb(urb, gfp)) { + err = usb_submit_urb(urb, gfp); + if (err) { usb_unanchor_urb(urb); + uas_log_cmd_state(cmnd, __func__); shost_printk(KERN_INFO, shost, - "sense urb submission failure\n"); + "sense urb submission error %d stream %d\n", + err, stream); usb_free_urb(urb); return NULL; } @@ -585,11 +609,11 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, { struct uas_cmd_info *cmdinfo = (void *)&cmnd->SCp; struct urb *urb; + int err; WARN_ON_ONCE(!spin_is_locked(&devinfo->lock)); if (cmdinfo->state & SUBMIT_STATUS_URB) { - urb = uas_submit_sense_urb(cmnd->device->host, gfp, - cmdinfo->stream); + urb = uas_submit_sense_urb(cmnd, gfp, cmdinfo->stream); if (!urb) return SCSI_MLQUEUE_DEVICE_BUSY; cmdinfo->state &= ~SUBMIT_STATUS_URB; @@ -606,10 +630,13 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, if (cmdinfo->state & SUBMIT_DATA_IN_URB) { usb_anchor_urb(cmdinfo->data_in_urb, &devinfo->data_urbs); - if (usb_submit_urb(cmdinfo->data_in_urb, gfp)) { + err = usb_submit_urb(cmdinfo->data_in_urb, gfp); + if (err) { usb_unanchor_urb(cmdinfo->data_in_urb); + uas_log_cmd_state(cmnd, __func__); scmd_printk(KERN_INFO, cmnd, - "data in urb submission failure\n"); + "data in urb submission error %d stream %d\n", + err, cmdinfo->data_in_urb->stream_id); return SCSI_MLQUEUE_DEVICE_BUSY; } cmdinfo->state &= ~SUBMIT_DATA_IN_URB; @@ -627,10 +654,13 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, if (cmdinfo->state & SUBMIT_DATA_OUT_URB) { usb_anchor_urb(cmdinfo->data_out_urb, &devinfo->data_urbs); - if (usb_submit_urb(cmdinfo->data_out_urb, gfp)) { + err = usb_submit_urb(cmdinfo->data_out_urb, gfp); + if (err) { usb_unanchor_urb(cmdinfo->data_out_urb); + uas_log_cmd_state(cmnd, __func__); scmd_printk(KERN_INFO, cmnd, - "data out urb submission failure\n"); + "data out urb submission error %d stream %d\n", + err, cmdinfo->data_out_urb->stream_id); return SCSI_MLQUEUE_DEVICE_BUSY; } cmdinfo->state &= ~SUBMIT_DATA_OUT_URB; @@ -646,10 +676,12 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd, if (cmdinfo->state & SUBMIT_CMD_URB) { usb_anchor_urb(cmdinfo->cmd_urb, &devinfo->cmd_urbs); - if (usb_submit_urb(cmdinfo->cmd_urb, gfp)) { + err = usb_submit_urb(cmdinfo->cmd_urb, gfp); + if (err) { usb_unanchor_urb(cmdinfo->cmd_urb); + uas_log_cmd_state(cmnd, __func__); scmd_printk(KERN_INFO, cmnd, - "cmd urb submission failure\n"); + "cmd urb submission error %d\n", err); return SCSI_MLQUEUE_DEVICE_BUSY; } cmdinfo->cmd_urb = NULL; @@ -760,7 +792,7 @@ static int uas_eh_task_mgmt(struct scsi_cmnd *cmnd, devinfo->running_task = 1; memset(&devinfo->response, 0, sizeof(devinfo->response)); - sense_urb = uas_submit_sense_urb(shost, GFP_NOIO, + sense_urb = uas_submit_sense_urb(cmnd, GFP_NOIO, devinfo->use_streams ? tag : 0); if (!sense_urb) { shost_printk(KERN_INFO, shost, -- cgit v1.2.2 From 8e453155d7f8dfa53863ba6f8da6c68f7c17ece4 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 15 Nov 2013 10:04:11 +0100 Subject: uas: Add some data in/out ready iu sanity checks Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index f09205b162e4..62086829af14 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -379,9 +379,19 @@ static void uas_stat_cmplt(struct urb *urb) uas_try_complete(cmnd, __func__); break; case IU_ID_READ_READY: + if (!cmdinfo->data_in_urb || + (cmdinfo->state & DATA_IN_URB_INFLIGHT)) { + scmd_printk(KERN_ERR, cmnd, "unexpected read rdy\n"); + break; + } uas_xfer_data(urb, cmnd, SUBMIT_DATA_IN_URB); break; case IU_ID_WRITE_READY: + if (!cmdinfo->data_out_urb || + (cmdinfo->state & DATA_OUT_URB_INFLIGHT)) { + scmd_printk(KERN_ERR, cmnd, "unexpected write rdy\n"); + break; + } uas_xfer_data(urb, cmnd, SUBMIT_DATA_OUT_URB); break; default: -- cgit v1.2.2 From 37599f9603bed3d72becdc1a59c164576df9fbfc Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Fri, 15 Nov 2013 10:04:31 +0100 Subject: uas: Make sure sg elements are properly aligned Copy the sg alignment trick from the usb-storage driver, without this I'm seeing intermittent errors when using uas devices with an ehci controller. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 62086829af14..ad97615b75b1 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -925,6 +925,24 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd) static int uas_slave_alloc(struct scsi_device *sdev) { sdev->hostdata = (void *)sdev->host->hostdata; + + /* USB has unusual DMA-alignment requirements: Although the + * starting address of each scatter-gather element doesn't matter, + * the length of each element except the last must be divisible + * by the Bulk maxpacket value. There's currently no way to + * express this by block-layer constraints, so we'll cop out + * and simply require addresses to be aligned at 512-byte + * boundaries. This is okay since most block I/O involves + * hardware sectors that are multiples of 512 bytes in length, + * and since host controllers up through USB 2.0 have maxpacket + * values no larger than 512. + * + * But it doesn't suffice for Wireless USB, where Bulk maxpacket + * values can be as large as 2048. To make that work properly + * will require changes to the block layer. + */ + blk_queue_update_dma_alignment(sdev->request_queue, (512 - 1)); + return 0; } -- cgit v1.2.2 From f50a4968deb7bf38c46f5baf62db9431a099531a Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Mon, 28 Oct 2013 10:48:04 +0000 Subject: uas: Add Hans de Goede as uas maintainer At the kernel-summit Sarah Sharp asked me if I was willing to become the uas maintainer. I said yes, and here is a patch to make this official. Also remove Matthew Wilcox and Sarah Sharp as maintainers at their request. I've also added myself to the module's author tag, so that if people look there rather then in maintainers they will know they should bug me about uas too. Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index ad97615b75b1..08e9710f193d 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -2,6 +2,7 @@ * USB Attached SCSI * Note that this is not the same as the USB Mass Storage driver * + * Copyright Hans de Goede for Red Hat, Inc. 2013 * Copyright Matthew Wilcox for Intel Corp, 2010 * Copyright Sarah Sharp for Intel Corp, 2010 * @@ -1261,4 +1262,5 @@ static struct usb_driver uas_driver = { module_usb_driver(uas_driver); MODULE_LICENSE("GPL"); -MODULE_AUTHOR("Matthew Wilcox and Sarah Sharp"); +MODULE_AUTHOR( + "Hans de Goede , Matthew Wilcox and Sarah Sharp"); -- cgit v1.2.2 From 7cace978fba5d0ec6eed50509cda40eea85f8e98 Mon Sep 17 00:00:00 2001 From: Hans de Goede Date: Tue, 29 Oct 2013 11:36:43 +0100 Subject: uas: Remove comment about registering a uas scsi controller for each usb bus Although an interesting concept, I don't think that this is a good idea: -This will result in lots of "virtual" scsi controllers confusing users -If we get a scsi-bus-reset we will now need to do a usb-device-reset of all uas devices on the same usb bus, which is something to avoid if possible Signed-off-by: Hans de Goede Signed-off-by: Sarah Sharp --- drivers/usb/storage/uas.c | 6 ------ 1 file changed, 6 deletions(-) (limited to 'drivers/usb/storage/uas.c') diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c index 08e9710f193d..a7ac97cc5949 100644 --- a/drivers/usb/storage/uas.c +++ b/drivers/usb/storage/uas.c @@ -1050,12 +1050,6 @@ static void uas_free_streams(struct uas_dev_info *devinfo) usb_free_streams(devinfo->intf, eps, 3, GFP_KERNEL); } -/* - * XXX: What I'd like to do here is register a SCSI host for each USB host in - * the system. Follow usb-storage's design of registering a SCSI host for - * each USB device for the moment. Can implement this by walking up the - * USB hierarchy until we find a USB host. - */ static int uas_probe(struct usb_interface *intf, const struct usb_device_id *id) { int result = -ENOMEM; -- cgit v1.2.2