RE: [PATCH v4 7/9] usb: rename transceiver and phy to usb_phy in ChipIdea

[Peter Chen]

 
 On Thu, Sep 11, 2014 at 08:28:59AM +0800, Peter Chen wrote:
  On Wed, Sep 03, 2014 at 09:40:38AM +0200, Antoine Tenart wrote:
 
  Again, rebase my next-tree, and modify the msm part.
 
 git://github.com/hzpeterchen/linux-usb.git ci-for-usb-next ?
 

Yes.

 I can do that. But that would be easier if you rebased this branch on top of 
 at
 least v3.17-rc1 so that other series depending on this one could be tested 
 easily
 (like my other series introducing the USB support for Marvell Berlin).
 
 Antoine
 
Done 

Peter
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [Patch v9 3/3] phy: Add Qualcomm DWC3 HS/SS PHY driver

[Kishon Vijay Abraham I]

Hi,

On Saturday 13 September 2014 12:58 AM, Andy Gross wrote:
 This patch adds a new driver for the Qualcomm USB 3.0 PHY that exists on some
 Qualcomm platforms.  This driver uses the generic PHY framework and will
 interact with the DWC3 controller.

Do you have dt documentation for this driver?
 
 Signed-off-by: Andy Gross agr...@codeaurora.org
 ---
  drivers/phy/Kconfig |   11 +
  drivers/phy/Makefile|1 +
  drivers/phy/phy-qcom-dwc3.c |  483 
 +++
  3 files changed, 495 insertions(+)
  create mode 100644 drivers/phy/phy-qcom-dwc3.c
 
 diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
 index 0dd7427..5d56161 100644
 --- a/drivers/phy/Kconfig
 +++ b/drivers/phy/Kconfig
 @@ -230,4 +230,15 @@ config PHY_XGENE
   help
 This option enables support for APM X-Gene SoC multi-purpose PHY.
  
 +config PHY_QCOM_DWC3
 + tristate QCOM DWC3 USB PHY support
 + depends on ARCH_QCOM
 + depends on HAS_IOMEM
 + depends on OF
 + select GENERIC_PHY
 + help
 +   This option enables support for the Synopsis PHYs present inside the
 +   Qualcomm USB3.0 DWC3 controller.  This driver supports both HS and SS
 +   PHY controllers.
 +
  endmenu
 diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
 index 95c69ed..aa16f30 100644
 --- a/drivers/phy/Makefile
 +++ b/drivers/phy/Makefile
 @@ -28,3 +28,4 @@ obj-$(CONFIG_PHY_QCOM_IPQ806X_SATA) += 
 phy-qcom-ipq806x-sata.o
  obj-$(CONFIG_PHY_ST_SPEAR1310_MIPHY) += phy-spear1310-miphy.o
  obj-$(CONFIG_PHY_ST_SPEAR1340_MIPHY) += phy-spear1340-miphy.o
  obj-$(CONFIG_PHY_XGENE)  += phy-xgene.o
 +obj-$(CONFIG_PHY_QCOM_DWC3)  += phy-qcom-dwc3.o
 diff --git a/drivers/phy/phy-qcom-dwc3.c b/drivers/phy/phy-qcom-dwc3.c
 new file mode 100644
 index 000..2c7b316
 --- /dev/null
 +++ b/drivers/phy/phy-qcom-dwc3.c
 @@ -0,0 +1,483 @@
 +/* Copyright (c) 2013-2014, Code Aurora Forum. All rights reserved.
 + *
 + * This program is free software; you can redistribute it and/or modify
 + * it under the terms of the GNU General Public License version 2 and
 + * only version 2 as published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope that it will be useful,
 + * but WITHOUT ANY WARRANTY; without even the implied warranty of
 + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 + * GNU General Public License for more details.
 + */
 +
 +#include linux/clk.h
 +#include linux/err.h
 +#include linux/io.h
 +#include linux/module.h
 +#include linux/of.h
 +#include linux/phy/phy.h
 +#include linux/platform_device.h
 +#include linux/delay.h
 +
 +/**
 + *  USB QSCRATCH Hardware registers
 + */
 +#define QSCRATCH_GENERAL_CFG (0x08)
 +#define HSUSB_PHY_CTRL_REG   (0x10)
 +
 +/* PHY_CTRL_REG */
 +#define HSUSB_CTRL_DMSEHV_CLAMP  BIT(24)
 +#define HSUSB_CTRL_USB2_SUSPEND  BIT(23)
 +#define HSUSB_CTRL_UTMI_CLK_EN   BIT(21)
 +#define  HSUSB_CTRL_UTMI_OTG_VBUS_VALID  BIT(20)
  
alignment went wrong here..

 +#define HSUSB_CTRL_USE_CLKCORE   BIT(18)
 +#define HSUSB_CTRL_DPSEHV_CLAMP  BIT(17)
 +#define HSUSB_CTRL_COMMONONN BIT(11)
 +#define HSUSB_CTRL_ID_HV_CLAMP   BIT(9)
 +#define HSUSB_CTRL_OTGSESSVLD_CLAMP  BIT(8)
 +#define HSUSB_CTRL_CLAMP_EN  BIT(7)
 +#define HSUSB_CTRL_RETENABLENBIT(1)
 +#define HSUSB_CTRL_POR   BIT(0)
 +
 +/* QSCRATCH_GENERAL_CFG */
 +#define HSUSB_GCFG_XHCI_REV  BIT(2)
 +
 +/**
 + *  USB QSCRATCH Hardware registers
 + */
 +#define SSUSB_PHY_CTRL_REG   (0x00)
 +#define SSUSB_PHY_PARAM_CTRL_1   (0x04)
 +#define SSUSB_PHY_PARAM_CTRL_2   (0x08)
 +#define CR_PROTOCOL_DATA_IN_REG  (0x0c)
 +#define CR_PROTOCOL_DATA_OUT_REG (0x10)
 +#define CR_PROTOCOL_CAP_ADDR_REG (0x14)
 +#define CR_PROTOCOL_CAP_DATA_REG (0x18)
 +#define CR_PROTOCOL_READ_REG (0x1c)
 +#define CR_PROTOCOL_WRITE_REG(0x20)
 +
 +/* PHY_CTRL_REG */
 +#define SSUSB_CTRL_REF_USE_PAD   BIT(28)
 +#define SSUSB_CTRL_TEST_POWERDOWNBIT(27)
 +#define SSUSB_CTRL_LANE0_PWR_PRESENT BIT(24)
 +#define SSUSB_CTRL_SS_PHY_EN BIT(8)
 +#define SSUSB_CTRL_SS_PHY_RESET  BIT(7)
 +
 +/* SSPHY control registers */
 +#define SSPHY_CTRL_RX_OVRD_IN_HI(lane)   (0x1006 + 0x100 * lane)
 +#define SSPHY_CTRL_TX_OVRD_DRV_LO(lane)  (0x1002 + 0x100 * lane)
 +
 +/* RX OVRD IN HI bits */
 +#define RX_OVRD_IN_HI_RX_RESET_OVRD  BIT(13)
 +#define RX_OVRD_IN_HI_RX_RX_RESETBIT(12)
 +#define RX_OVRD_IN_HI_RX_EQ_OVRD BIT(11)
 +#define RX_OVRD_IN_HI_RX_EQ_MASK 0x0700
 +#define RX_OVRD_IN_HI_RX_EQ_SHIFT8
 +#define RX_OVRD_IN_HI_RX_EQ_EN_OVRD  BIT(7)
 

[PATCH fix for 3.17] uas: Add a quirk for rejecting ATA_12 and ATA_16 commands

[Hans de Goede]

And set this quirk for the Seagate Expansion Desk (0bc2:2312), as that one
seems to hang upon receiving an ATA_12 or ATA_16 command.

https://bugzilla.kernel.org/show_bug.cgi?id=79511

Cc: sta...@vger.kernel.org # 3.16
Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 13 +
 drivers/usb/storage/unusual_uas.h | 16 ++--
 drivers/usb/storage/usb.c |  6 +-
 include/linux/usb_usual.h |  2 ++
 4 files changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 3f42785..75d2ccd 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -28,6 +28,7 @@
 #include scsi/scsi_tcq.h
 
 #include uas-detect.h
+#include scsiglue.h
 
 /*
  * The r00-r01c specs define this version of the SENSE IU data structure.
@@ -49,6 +50,7 @@ struct uas_dev_info {
struct usb_anchor cmd_urbs;
struct usb_anchor sense_urbs;
struct usb_anchor data_urbs;
+   unsigned long flags;
int qdepth, resetting;
struct response_iu response;
unsigned cmd_pipe, status_pipe, data_in_pipe, data_out_pipe;
@@ -714,6 +716,15 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
 
BUILD_BUG_ON(sizeof(struct uas_cmd_info)  sizeof(struct scsi_pointer));
 
+   if ((devinfo-flags  US_FL_NO_ATA_1X) 
+   (cmnd-cmnd[0] == ATA_12 || cmnd-cmnd[0] == ATA_16)) {
+   memcpy(cmnd-sense_buffer, usb_stor_sense_invalidCDB,
+  sizeof(usb_stor_sense_invalidCDB));
+   cmnd-result = SAM_STAT_CHECK_CONDITION;
+   cmnd-scsi_done(cmnd);
+   return 0;
+   }
+
spin_lock_irqsave(devinfo-lock, flags);
 
if (devinfo-resetting) {
@@ -1080,6 +1091,8 @@ static int uas_probe(struct usb_interface *intf, const 
struct usb_device_id *id)
devinfo-resetting = 0;
devinfo-running_task = 0;
devinfo-shutdown = 0;
+   devinfo-flags = id-driver_info;
+   usb_stor_adjust_quirks(udev, devinfo-flags);
init_usb_anchor(devinfo-cmd_urbs);
init_usb_anchor(devinfo-sense_urbs);
init_usb_anchor(devinfo-data_urbs);
diff --git a/drivers/usb/storage/unusual_uas.h 
b/drivers/usb/storage/unusual_uas.h
index 724..52f7012 100644
--- a/drivers/usb/storage/unusual_uas.h
+++ b/drivers/usb/storage/unusual_uas.h
@@ -40,13 +40,9 @@
  * and don't forget to CC: the USB development list linux-usb@vger.kernel.org
  */
 
-/*
- * This is an example entry for the US_FL_IGNORE_UAS flag. Once we have an
- * actual entry using US_FL_IGNORE_UAS this entry should be removed.
- *
- * UNUSUAL_DEV(  0xabcd, 0x1234, 0x0100, 0x0100,
- * Example,
- * Storage with broken UAS,
- * USB_SC_DEVICE, USB_PR_DEVICE, NULL,
- * US_FL_IGNORE_UAS),
- */
+/* https://bugzilla.kernel.org/show_bug.cgi?id=79511 */
+UNUSUAL_DEV(0x0bc2, 0x2312, 0x, 0x,
+   Seagate,
+   Expansion Desk,
+   USB_SC_DEVICE, USB_PR_DEVICE, NULL,
+   US_FL_NO_ATA_1X),
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index cedb292..b9d1b93 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -478,7 +478,8 @@ void usb_stor_adjust_quirks(struct usb_device *udev, 
unsigned long *fflags)
US_FL_CAPACITY_OK | US_FL_IGNORE_RESIDUE |
US_FL_SINGLE_LUN | US_FL_NO_WP_DETECT |
US_FL_NO_READ_DISC_INFO | US_FL_NO_READ_CAPACITY_16 |
-   US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE);
+   US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE |
+   US_FL_NO_ATA_1X);
 
p = quirks;
while (*p) {
@@ -543,6 +544,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev, 
unsigned long *fflags)
case 's':
f |= US_FL_SINGLE_LUN;
break;
+   case 't':
+   f |= US_FL_NO_ATA_1X;
+   break;
case 'u':
f |= US_FL_IGNORE_UAS;
break;
diff --git a/include/linux/usb_usual.h b/include/linux/usb_usual.h
index 9b7de1b..d271f88 100644
--- a/include/linux/usb_usual.h
+++ b/include/linux/usb_usual.h
@@ -73,6 +73,8 @@
/* Device advertises UAS but it is broken */\
US_FLAG(BROKEN_FUA, 0x0100) \
/* Cannot handle FUA in WRITE or READ CDBs */   \
+   US_FLAG(NO_ATA_1X,  0x0200) \
+   /* Cannot handle ATA_12 or ATA_16 CDBs */   \
 
 #define US_FLAG(name, value)   US_FL_##name = value ,
 enum { US_DO_ALL_FLAGS };
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  

[PATCH fix for 3.17 0/1] uas: Add a quirk for rejecting ATA_12 and ATA_16

[Hans de Goede]

Hi Greg,

As the subject indicates, if possible I would like to still get this into
3.17.

For now this impacts only a single usb-id, but looking at
https://bbs.archlinux.org/viewtopic.php?id=183190

It seems more seagate uas capable enclosures are having issues, I'm waiting
for feedback from their users. I hope those can be fixed by adding the same
quirk for there usb-ids.

Regards,

Hans
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 00/24] uas: rewrite error handling for robustness + misc cleanups

[Hans de Goede]

Hi Greg, et al,

Here is v2 of my uas error handling rewrite + cleanups series.

Note this has been rebased on top of the

[PATCH fix for 3.17] uas: Add a quirk for rejecting ATA_12 and ATA_16 commands

patch which I've just send, so if that does not make 3.17, you should still
apply that one first to avoid conflicts.

Changes since v1:
-Rebased on top of:
 uas: Add a quirk for rejecting ATA_12 and ATA_16 commands
-uas: zap_pending: data urbs should have completed at this time:
 Modified WARN_ON check to not have side effects
-Added 3 new patches:
 uas: Cleanup uas_log_cmd_state usage
 uas: Log error codes when logging errors
 uas: Add response iu handling

Regards,

Hans
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 24/24] uas: Add response iu handling

[Hans de Goede]

If something goes wrong in our communication with an uas device we may get
a response iu in reaction to a cmnd, rather then a status iu. In this case
propagate an error upwards, rather then logging a bogus iu message.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 10 ++
 1 file changed, 10 insertions(+)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 1dcaeee..bebf861 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -328,6 +328,16 @@ static void uas_stat_cmplt(struct urb *urb)
}
uas_xfer_data(urb, cmnd, SUBMIT_DATA_OUT_URB);
break;
+   case IU_ID_RESPONSE:
+   uas_log_cmd_state(cmnd, unexpected response iu,
+ ((struct response_iu *)iu)-response_code);
+   /* Error, cancel data transfers */
+   data_in_urb = usb_get_urb(cmdinfo-data_in_urb);
+   data_out_urb = usb_get_urb(cmdinfo-data_out_urb);
+   cmdinfo-state = ~COMMAND_INFLIGHT;
+   cmnd-result = DID_ERROR  16;
+   uas_try_complete(cmnd, __func__);
+   break;
default:
uas_log_cmd_state(cmnd, bogus IU, iu-iu_id);
}
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 15/24] uas: pre_reset and suspend: Fix a few races

[Hans de Goede]

The purpose of uas_pre_reset is to:

1) Stop any new commands from being submitted while an externally triggered
   usb-device-reset is running
2) Wait for any pending commands to finish before allowing the usb-device-reset
   to continue

The purpose of uas_suspend is to:
2) Wait for any pending commands to finish before suspending

This commit fixes races in both paths:

1) For 1) we use scsi_block_requests, but the scsi midlayer calls queuecommand
   without holding any locks, so a queuecommand may already past the midlayer
   scsi_block_requests checks when we call it, add a check to uas_queuecommand
   to fix this

2) For 2) we were waiting for all sense-urbs to complete, there are 2 problems
   with this approach:
a) data-urbs may complete after the sense urb, so we need to check for those
   too
b) if a sense-urb completes with a iu id of READ/WRITE_READY a command is not
   yet done. We submit a new sense-urb immediately in this case, but that
   submit may fail (in which case it will get retried by uas_do_work), if this
   happens the sense_urbs anchor may become empty while the cmnd is not yet
   done

Also unblock requests on timeout, to avoid things getting stuck in that case.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 61 ++-
 1 file changed, 55 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index fca8775..f4fe816 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -667,6 +667,10 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
 
BUILD_BUG_ON(sizeof(struct uas_cmd_info)  sizeof(struct scsi_pointer));
 
+   /* Re-check scsi_block_requests now that we've the host-lock */
+   if (cmnd-device-host-host_self_blocked)
+   return SCSI_MLQUEUE_DEVICE_BUSY;
+
if ((devinfo-flags  US_FL_NO_ATA_1X) 
(cmnd-cmnd[0] == ATA_12 || cmnd-cmnd[0] == ATA_16)) {
memcpy(cmnd-sense_buffer, usb_stor_sense_invalidCDB,
@@ -1005,6 +1009,54 @@ set_alt0:
return result;
 }
 
+static int uas_cmnd_list_empty(struct uas_dev_info *devinfo)
+{
+   unsigned long flags;
+   int i, r = 1;
+
+   spin_lock_irqsave(devinfo-lock, flags);
+
+   for (i = 0; i  devinfo-qdepth; i++) {
+   if (devinfo-cmnd[i]) {
+   r = 0; /* Not empty */
+   break;
+   }
+   }
+
+   spin_unlock_irqrestore(devinfo-lock, flags);
+
+   return r;
+}
+
+/*
+ * Wait for any pending cmnds to complete, on usb-2 sense_urbs may temporarily
+ * get empty while there still is more work to do due to sense-urbs completing
+ * with a READ/WRITE_READY iu code, so keep waiting until the list gets empty.
+ */
+static int uas_wait_for_pending_cmnds(struct uas_dev_info *devinfo)
+{
+   unsigned long start_time;
+   int r;
+
+   start_time = jiffies;
+   do {
+   flush_work(devinfo-work);
+
+   r = usb_wait_anchor_empty_timeout(devinfo-sense_urbs, 5000);
+   if (r == 0)
+   return -ETIME;
+
+   r = usb_wait_anchor_empty_timeout(devinfo-data_urbs, 500);
+   if (r == 0)
+   return -ETIME;
+
+   if (time_after(jiffies, start_time + 5 * HZ))
+   return -ETIME;
+   } while (!uas_cmnd_list_empty(devinfo));
+
+   return 0;
+}
+
 static int uas_pre_reset(struct usb_interface *intf)
 {
struct Scsi_Host *shost = usb_get_intfdata(intf);
@@ -1019,10 +1071,9 @@ static int uas_pre_reset(struct usb_interface *intf)
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) {
+   if (uas_wait_for_pending_cmnds(devinfo) != 0) {
shost_printk(KERN_ERR, shost, %s: timed out\n, __func__);
+   scsi_unblock_requests(shost);
return 1;
}
 
@@ -1060,9 +,7 @@ 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 = (struct uas_dev_info *)shost-hostdata;
 
-   /* Wait for any pending requests to complete */
-   flush_work(devinfo-work);
-   if (usb_wait_anchor_empty_timeout(devinfo-sense_urbs, 5000) == 0) {
+   if (uas_wait_for_pending_cmnds(devinfo) != 0) {
shost_printk(KERN_ERR, shost, %s: timed out\n, __func__);
return -ETIME;
}
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 21/24] uas: Remove protype hardware usb interface info

[Hans de Goede]

We've removed all hack from the driver for pre-production hardware.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index cbd4312..a77c5e0 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -830,8 +830,6 @@ 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 */
-   { USB_INTERFACE_INFO(USB_CLASS_MASS_STORAGE, USB_SC_SCSI, 0xaa) },
{ }
 };
 MODULE_DEVICE_TABLE(usb, uas_usb_ids);
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 17/24] uas: Do not log urb status error on cancellation

[Hans de Goede]

Check for both type of cancellation codes for sense and data urbs.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 7 ++-
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 187e7bf..90afe4a 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -318,10 +318,7 @@ static void uas_stat_cmplt(struct urb *urb)
goto out;
 
if (urb-status) {
-   if (urb-status == -ENOENT) {
-   dev_err(urb-dev-dev, stat urb: killed, stream %d\n,
-   urb-stream_id);
-   } else {
+   if (urb-status != -ENOENT  urb-status != -ECONNRESET) {
dev_err(urb-dev-dev, stat urb: status %d\n,
urb-status);
}
@@ -428,7 +425,7 @@ static void uas_data_cmplt(struct urb *urb)
}
 
if (urb-status) {
-   if (urb-status != -ECONNRESET) {
+   if (urb-status != -ENOENT  urb-status != -ECONNRESET) {
uas_log_cmd_state(cmnd, __func__);
scmd_printk(KERN_ERR, cmnd,
data cmplt err %d stream %d\n,
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 10/24] uas: zap_pending: data urbs should have completed at this time

[Hans de Goede]

The data urbs are all killed before calling zap_pending, and their completion
handler should have cleared their inflight flag.

Do not 0 the data inflight flags, and add a check for try_complete succeeding,
as it should always succeed when called from zap_pending.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 10 +-
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 08edb6b..85bbc1d 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -145,6 +145,7 @@ static void uas_zap_pending(struct uas_dev_info *devinfo, 
int result)
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-dead_list, list) {
@@ -152,12 +153,11 @@ static void uas_zap_pending(struct uas_dev_info *devinfo, 
int result)
struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd,
  SCp);
uas_log_cmd_state(cmnd, __func__);
-   /* all urbs are killed, clear inflight bits */
-   cmdinfo-state = ~(COMMAND_INFLIGHT |
-   DATA_IN_URB_INFLIGHT |
-   DATA_OUT_URB_INFLIGHT);
+   /* Sense urbs were killed, clear COMMAND_INFLIGHT manually */
+   cmdinfo-state = ~COMMAND_INFLIGHT;
cmnd-result = result  16;
-   uas_try_complete(cmnd, __func__);
+   err = uas_try_complete(cmnd, __func__);
+   WARN_ON(err != 0);
}
spin_unlock_irqrestore(devinfo-lock, flags);
 }
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 22/24] uas: Cleanup uas_log_cmd_state usage

[Hans de Goede]

Instead of doing:

uas_log_cmd_state(cmnd, __func__)
scmd_printk(KERN_ERR, cmnd, error doing foo %d\n, err)

On error, resulting in 2 log calls for a single error, make uas_log_cmd_state
take a status code, and change calls like the above to:

uas_log_cmd_state(cmnd, error doing foo, err)

Also change various sanity checks (which should never trigger) from:
scmd_printk(KERN_ERR, cmnd, sanity foo failed\n) to calling the new
uas_log_cmd_state(), so that when they do trigger we get more info.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 52 +--
 1 file changed, 19 insertions(+), 33 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index a77c5e0..0b3d3a5 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -78,7 +78,8 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,
 static void uas_do_work(struct work_struct *work);
 static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller);
 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_log_cmd_state(struct scsi_cmnd *cmnd, const char *prefix,
+   int status);
 
 static void uas_do_work(struct work_struct *work)
 {
@@ -139,7 +140,7 @@ static void uas_zap_pending(struct uas_dev_info *devinfo, 
int result)
 
cmnd = devinfo-cmnd[i];
cmdinfo = (void *)cmnd-SCp;
-   uas_log_cmd_state(cmnd, __func__);
+   uas_log_cmd_state(cmnd, __func__, 0);
/* Sense urbs were killed, clear COMMAND_INFLIGHT manually */
cmdinfo-state = ~COMMAND_INFLIGHT;
cmnd-result = result  16;
@@ -188,13 +189,14 @@ static int uas_get_tag(struct scsi_cmnd *cmnd)
return tag;
 }
 
-static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller)
+static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *prefix,
+ int status)
 {
struct uas_cmd_info *ci = (void *)cmnd-SCp;
 
scmd_printk(KERN_INFO, cmnd,
-   %s tag %d inflight:%s%s%s%s%s%s%s%s%s%s%s%s ,
-   caller, uas_get_tag(cmnd),
+   %s %d tag %d inflight:%s%s%s%s%s%s%s%s%s%s%s%s ,
+   prefix, status, uas_get_tag(cmnd),
(ci-state  SUBMIT_STATUS_URB) ?  s-st  : ,
(ci-state  ALLOC_DATA_IN_URB) ?  a-in  : ,
(ci-state  SUBMIT_DATA_IN_URB)?  s-in  : ,
@@ -295,7 +297,7 @@ static void uas_stat_cmplt(struct urb *urb)
cmdinfo = (void *)cmnd-SCp;
 
if (!(cmdinfo-state  COMMAND_INFLIGHT)) {
-   scmd_printk(KERN_ERR, cmnd, unexpected status cmplt\n);
+   uas_log_cmd_state(cmnd, unexpected status cmplt, 0);
goto out;
}
 
@@ -313,7 +315,7 @@ static void uas_stat_cmplt(struct urb *urb)
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);
+   uas_log_cmd_state(cmnd, unexpected read rdy, 0);
break;
}
uas_xfer_data(urb, cmnd, SUBMIT_DATA_IN_URB);
@@ -321,14 +323,13 @@ static void uas_stat_cmplt(struct urb *urb)
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);
+   uas_log_cmd_state(cmnd, unexpected write rdy, 0);
break;
}
uas_xfer_data(urb, cmnd, SUBMIT_DATA_OUT_URB);
break;
default:
-   scmd_printk(KERN_ERR, cmnd,
-   Bogus IU (%d) received on status pipe\n, iu-iu_id);
+   uas_log_cmd_state(cmnd, bogus IU, iu-iu_id);
}
 out:
usb_free_urb(urb);
@@ -374,17 +375,13 @@ static void uas_data_cmplt(struct urb *urb)
 
/* Data urbs should not complete before the cmd urb is submitted */
if (cmdinfo-state  SUBMIT_CMD_URB) {
-   scmd_printk(KERN_ERR, cmnd, unexpected data cmplt\n);
+   uas_log_cmd_state(cmnd, unexpected data cmplt, 0);
goto out;
}
 
if (urb-status) {
-   if (urb-status != -ENOENT  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);
-   }
+   if (urb-status != -ENOENT  urb-status != -ECONNRESET)
+   

[PATCH v2 07/24] uas: Simplify unlink of data urbs on error

[Hans de Goede]

There is no need for all the trickery with dropping the lock, we can
simply reference the urbs while we hold the lock to ensure the urbs don't
disappear beneath us, and do the actual unlink (+ unreference) after we've
dropped the lock.

This also fixes a race where we may loose of cmnd ownership to the scsi
midlayer without holding the lock due to the midlayer re-claiming ownership
through an abort (which will be handled by a future patch in this series).

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 48 ++-
 1 file changed, 18 insertions(+), 30 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 8614ddf..68853b5 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -78,8 +78,7 @@ enum {
DATA_OUT_URB_INFLIGHT   = (1  10),
COMMAND_COMPLETED   = (1  11),
COMMAND_ABORTED = (1  12),
-   UNLINK_DATA_URBS= (1  13),
-   IS_IN_WORK_LIST = (1  14),
+   IS_IN_WORK_LIST = (1  13),
 };
 
 /* Overrides scsi_pointer */
@@ -100,28 +99,6 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const 
char *caller);
 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,
-unsigned long *lock_flags)
-{
-   /*
-* The UNLINK_DATA_URBS flag makes sure uas_try_complete
-* (called by urb completion) doesn't release cmdinfo
-* underneath us.
-*/
-   cmdinfo-state |= UNLINK_DATA_URBS;
-   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, *lock_flags);
-   cmdinfo-state = ~UNLINK_DATA_URBS;
-}
-
 static void uas_do_work(struct work_struct *work)
 {
struct uas_dev_info *devinfo =
@@ -281,8 +258,8 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const 
char *caller)
 {
struct uas_cmd_info *ci = (void *)cmnd-SCp;
 
-   scmd_printk(KERN_INFO, cmnd, %s %p tag %d, inflight:
-   %s%s%s%s%s%s%s%s%s%s%s%s%s%s\n,
+   scmd_printk(KERN_INFO, cmnd,
+   %s %p tag %d, inflight:%s%s%s%s%s%s%s%s%s%s%s%s%s\n,
caller, cmnd, uas_get_tag(cmnd),
(ci-state  SUBMIT_STATUS_URB) ?  s-st  : ,
(ci-state  ALLOC_DATA_IN_URB) ?  a-in  : ,
@@ -296,7 +273,6 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const 
char *caller)
(ci-state  DATA_OUT_URB_INFLIGHT) ?  OUT   : ,
(ci-state  COMMAND_COMPLETED) ?  done  : ,
(ci-state  COMMAND_ABORTED)   ?  abort : ,
-   (ci-state  UNLINK_DATA_URBS)  ?  unlink: ,
(ci-state  IS_IN_WORK_LIST)   ?  work  : );
 }
 
@@ -308,8 +284,7 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const 
char *caller)
lockdep_assert_held(devinfo-lock);
if (cmdinfo-state  (COMMAND_INFLIGHT |
  DATA_IN_URB_INFLIGHT |
- DATA_OUT_URB_INFLIGHT |
- UNLINK_DATA_URBS))
+ DATA_OUT_URB_INFLIGHT))
return -EBUSY;
WARN_ON_ONCE(cmdinfo-state  COMMAND_COMPLETED);
cmdinfo-state |= COMMAND_COMPLETED;
@@ -341,6 +316,8 @@ 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 = (struct uas_dev_info *)shost-hostdata;
+   struct urb *data_in_urb = NULL;
+   struct urb *data_out_urb = NULL;
struct scsi_cmnd *cmnd;
struct uas_cmd_info *cmdinfo;
unsigned long flags;
@@ -387,7 +364,8 @@ static void uas_stat_cmplt(struct urb *urb)
uas_sense(urb, cmnd);
if (cmnd-result != 0) {
/* cancel data transfers on error */
-   uas_unlink_data_urbs(devinfo, cmdinfo, flags);
+   data_in_urb = usb_get_urb(cmdinfo-data_in_urb);
+   data_out_urb = usb_get_urb(cmdinfo-data_out_urb);
}
cmdinfo-state = ~COMMAND_INFLIGHT;
uas_try_complete(cmnd, __func__);
@@ -415,6 +393,16 @@ static void uas_stat_cmplt(struct urb *urb)
 out:
usb_free_urb(urb);
spin_unlock_irqrestore(devinfo-lock, flags);
+
+   /* Unlinking of data urbs must be done without holding the lock */
+   if 

[PATCH v2 16/24] uas: Use streams on upcoming 10Gbps / 3.1 USB

[Hans de Goede]

Limit the no-streams case to speeds less then USB_SPEED_SUPER.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index f4fe816..187e7bf 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -921,7 +921,7 @@ static int uas_configure_endpoints(struct uas_dev_info 
*devinfo)
devinfo-data_out_pipe = usb_sndbulkpipe(udev,
usb_endpoint_num(eps[3]-desc));
 
-   if (udev-speed != USB_SPEED_SUPER) {
+   if (udev-speed  USB_SPEED_SUPER) {
devinfo-qdepth = 32;
devinfo-use_streams = 0;
} else {
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 19/24] uas: Drop COMMAND_COMPLETED flag

[Hans de Goede]

It was only used to sanity check against completing the same cmnd twice,
but that is the case we're likely operating on free-ed memory, and doing
sanity checks on free-ed memory is not really helpful.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 10 +++---
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 652b97b..c6f7870 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -74,9 +74,8 @@ enum {
COMMAND_INFLIGHT= (1  8),
DATA_IN_URB_INFLIGHT= (1  9),
DATA_OUT_URB_INFLIGHT   = (1  10),
-   COMMAND_COMPLETED   = (1  11),
-   COMMAND_ABORTED = (1  12),
-   IS_IN_WORK_LIST = (1  13),
+   COMMAND_ABORTED = (1  11),
+   IS_IN_WORK_LIST = (1  12),
 };
 
 /* Overrides scsi_pointer */
@@ -232,7 +231,7 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const 
char *caller)
struct uas_cmd_info *ci = (void *)cmnd-SCp;
 
scmd_printk(KERN_INFO, cmnd,
-   %s tag %d inflight:%s%s%s%s%s%s%s%s%s%s%s%s%s ,
+   %s tag %d inflight:%s%s%s%s%s%s%s%s%s%s%s%s ,
caller, uas_get_tag(cmnd),
(ci-state  SUBMIT_STATUS_URB) ?  s-st  : ,
(ci-state  ALLOC_DATA_IN_URB) ?  a-in  : ,
@@ -244,7 +243,6 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const 
char *caller)
(ci-state  COMMAND_INFLIGHT)  ?  CMD   : ,
(ci-state  DATA_IN_URB_INFLIGHT)  ?  IN: ,
(ci-state  DATA_OUT_URB_INFLIGHT) ?  OUT   : ,
-   (ci-state  COMMAND_COMPLETED) ?  done  : ,
(ci-state  COMMAND_ABORTED)   ?  abort : ,
(ci-state  IS_IN_WORK_LIST)   ?  work  : );
scsi_print_command(cmnd);
@@ -280,8 +278,6 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const 
char *caller)
  DATA_OUT_URB_INFLIGHT |
  COMMAND_ABORTED))
return -EBUSY;
-   WARN_ON_ONCE(cmdinfo-state  COMMAND_COMPLETED);
-   cmdinfo-state |= COMMAND_COMPLETED;
devinfo-cmnd[uas_get_tag(cmnd) - 1] = NULL;
uas_free_unsubmitted_urbs(cmnd);
cmnd-scsi_done(cmnd);
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 12/24] uas: Remove cmnd reference from the cmd urb

[Hans de Goede]

It is not strictly necessary for the cmd urb to have a reference to the
cmnd, and without this reference it becomes easier to drop all references to
a cmnd on an abort.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 9 +++--
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 36c42b8..1781b36 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -428,12 +428,9 @@ out:
 
 static void uas_cmd_cmplt(struct urb *urb)
 {
-   struct scsi_cmnd *cmnd = urb-context;
+   if (urb-status)
+   dev_err(urb-dev-dev, cmd cmplt err %d\n, urb-status);
 
-   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);
 }
 
@@ -511,7 +508,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,
-   uas_cmd_cmplt, cmnd);
+   uas_cmd_cmplt, NULL);
urb-transfer_flags |= URB_FREE_BUFFER;
  out:
return urb;
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 23/24] uas: Log error codes when logging errors

[Hans de Goede]

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 17 -
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 0b3d3a5..1dcaeee 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -750,7 +750,8 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd)
usb_unlock_device(udev);
 
if (err) {
-   shost_printk(KERN_INFO, sdev-host, %s FAILED\n, __func__);
+   shost_printk(KERN_INFO, sdev-host, %s FAILED err %d\n,
+__func__, err);
return FAILED;
}
 
@@ -1020,13 +1021,16 @@ static int uas_post_reset(struct usb_interface *intf)
struct Scsi_Host *shost = usb_get_intfdata(intf);
struct uas_dev_info *devinfo = (struct uas_dev_info *)shost-hostdata;
unsigned long flags;
+   int err;
 
if (devinfo-shutdown)
return 0;
 
-   if (uas_configure_endpoints(devinfo) != 0) {
+   err = uas_configure_endpoints(devinfo);
+   if (err) {
shost_printk(KERN_ERR, shost,
-%s: alloc streams error after reset, __func__);
+%s: alloc streams error %d after reset,
+__func__, err);
return 1;
}
 
@@ -1062,10 +1066,13 @@ static int uas_reset_resume(struct usb_interface *intf)
struct Scsi_Host *shost = usb_get_intfdata(intf);
struct uas_dev_info *devinfo = (struct uas_dev_info *)shost-hostdata;
unsigned long flags;
+   int err;
 
-   if (uas_configure_endpoints(devinfo) != 0) {
+   err = uas_configure_endpoints(devinfo);
+   if (err) {
shost_printk(KERN_ERR, shost,
-%s: alloc streams error after reset, __func__);
+%s: alloc streams error %d after reset,
+__func__, err);
return -EIO;
}
 
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 05/24] uas: Do not use scsi_host_find_tag

[Hans de Goede]

Using scsi_host_find_tag with tags returned by the device is unsafe for
multiple reasons:

1) It returns tags-rqs[tag], which may be non NULL even when the cmnd is
   not owned by us
2) It returns tags-rqs[tag], without holding any locks protecting it
3) It returns tags-rqs[tag], without doing any boundary checking

Instead keep our own list which maps tags - inflight cmnds.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 39 ++-
 1 file changed, 18 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index ac1424c..b58d133 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -30,6 +30,8 @@
 #include uas-detect.h
 #include scsiglue.h
 
+#define MAX_CMNDS 256
+
 /*
  * The r00-r01c specs define this version of the SENSE IU data structure.
  * It's still in use by several different firmware releases.
@@ -56,7 +58,7 @@ struct uas_dev_info {
unsigned use_streams:1;
unsigned uas_sense_old:1;
unsigned shutdown:1;
-   struct scsi_cmnd *cmnd;
+   struct scsi_cmnd *cmnd[MAX_CMNDS];
spinlock_t lock;
struct work_struct work;
struct list_head inflight_list;
@@ -316,6 +318,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);
list_del(cmdinfo-list);
+   devinfo-cmnd[uas_get_tag(cmnd) - 1] = NULL;
cmnd-scsi_done(cmnd);
return 0;
 }
@@ -341,7 +344,7 @@ static void uas_stat_cmplt(struct urb *urb)
struct scsi_cmnd *cmnd;
struct uas_cmd_info *cmdinfo;
unsigned long flags;
-   u16 tag;
+   unsigned int idx;
 
spin_lock_irqsave(devinfo-lock, flags);
 
@@ -359,21 +362,17 @@ static void uas_stat_cmplt(struct urb *urb)
goto out;
}
 
-   tag = be16_to_cpup(iu-tag) - 1;
-   if (tag == 0)
-   cmnd = devinfo-cmnd;
-   else
-   cmnd = scsi_host_find_tag(shost, tag - 1);
-
-   if (!cmnd)
+   idx = be16_to_cpup(iu-tag) - 1;
+   if (idx = MAX_CMNDS || !devinfo-cmnd[idx]) {
+   dev_err(urb-dev-dev,
+   stat urb: no pending cmd for tag %d\n, idx + 1);
goto out;
+   }
 
+   cmnd = devinfo-cmnd[idx];
cmdinfo = (void *)cmnd-SCp;
switch (iu-iu_id) {
case IU_ID_STATUS:
-   if (devinfo-cmnd == cmnd)
-   devinfo-cmnd = NULL;
-
if (urb-actual_length  16)
devinfo-uas_sense_old = 1;
if (devinfo-uas_sense_old)
@@ -674,6 +673,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
struct uas_dev_info *devinfo = sdev-hostdata;
struct uas_cmd_info *cmdinfo = (void *)cmnd-SCp;
unsigned long flags;
+   unsigned int stream;
int err;
 
BUILD_BUG_ON(sizeof(struct uas_cmd_info)  sizeof(struct scsi_pointer));
@@ -696,19 +696,16 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
return 0;
}
 
-   if (devinfo-cmnd) {
+   stream = uas_get_tag(cmnd);
+   if (devinfo-cmnd[stream - 1]) {
spin_unlock_irqrestore(devinfo-lock, flags);
return SCSI_MLQUEUE_DEVICE_BUSY;
}
 
-   memset(cmdinfo, 0, sizeof(*cmdinfo));
-
-   if (!blk_rq_tagged(cmnd-request))
-   devinfo-cmnd = cmnd;
-
cmnd-scsi_done = done;
 
-   cmdinfo-stream = uas_get_tag(cmnd);
+   memset(cmdinfo, 0, sizeof(*cmdinfo));
+   cmdinfo-stream = stream;
cmdinfo-state = SUBMIT_STATUS_URB | ALLOC_CMD_URB | SUBMIT_CMD_URB;
 
switch (cmnd-sc_data_direction) {
@@ -738,6 +735,7 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
uas_add_work(cmdinfo);
}
 
+   devinfo-cmnd[stream - 1] = cmnd;
list_add_tail(cmdinfo-list, devinfo-inflight_list);
spin_unlock_irqrestore(devinfo-lock, flags);
return 0;
@@ -873,7 +871,6 @@ static int uas_configure_endpoints(struct uas_dev_info 
*devinfo)
int r;
 
devinfo-uas_sense_old = 0;
-   devinfo-cmnd = NULL;
 
r = uas_find_endpoints(devinfo-intf-cur_altsetting, eps);
if (r)
@@ -893,7 +890,7 @@ static int uas_configure_endpoints(struct uas_dev_info 
*devinfo)
devinfo-use_streams = 0;
} else {
devinfo-qdepth = usb_alloc_streams(devinfo-intf, eps + 1,
-   3, 256, GFP_NOIO);
+   3, MAX_CMNDS, GFP_NOIO);
if (devinfo-qdepth  0)
return devinfo-qdepth;
devinfo-use_streams = 1;
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to 

[PATCH v2 04/24] uas: Add uas_get_tag() helper function

[Hans de Goede]

Factor out the mapping of scsi-tags - uas-tags/stream-ids to a helper function
so that there is a single place where this magic happens.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 33 +
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 0d97602..ac1424c 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -259,13 +259,29 @@ static void uas_sense_old(struct urb *urb, struct 
scsi_cmnd *cmnd)
cmnd-result = sense_iu-status;
 }
 
+/*
+ * scsi-tags go from 0 - (nr_tags - 1), uas tags need to match stream-ids,
+ * which go from 1 - nr_streams. And we use 1 for untagged commands.
+ */
+static int uas_get_tag(struct scsi_cmnd *cmnd)
+{
+   int tag;
+
+   if (blk_rq_tagged(cmnd-request))
+   tag = cmnd-request-tag + 2;
+   else
+   tag = 1;
+
+   return tag;
+}
+
 static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const char *caller)
 {
struct uas_cmd_info *ci = (void *)cmnd-SCp;
 
scmd_printk(KERN_INFO, cmnd, %s %p tag %d, inflight:
%s%s%s%s%s%s%s%s%s%s%s%s%s%s\n,
-   caller, cmnd, cmnd-request-tag,
+   caller, cmnd, uas_get_tag(cmnd),
(ci-state  SUBMIT_STATUS_URB) ?  s-st  : ,
(ci-state  ALLOC_DATA_IN_URB) ?  a-in  : ,
(ci-state  SUBMIT_DATA_IN_URB)?  s-in  : ,
@@ -516,10 +532,7 @@ static struct urb *uas_alloc_cmd_urb(struct uas_dev_info 
*devinfo, gfp_t gfp,
goto free;
 
iu-iu_id = IU_ID_COMMAND;
-   if (blk_rq_tagged(cmnd-request))
-   iu-tag = cpu_to_be16(cmnd-request-tag + 2);
-   else
-   iu-tag = cpu_to_be16(1);
+   iu-tag = cpu_to_be16(uas_get_tag(cmnd));
iu-prio_attr = UAS_SIMPLE_TAG;
iu-len = len;
int_to_scsilun(sdev-lun, iu-lun);
@@ -690,17 +703,13 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
 
memset(cmdinfo, 0, sizeof(*cmdinfo));
 
-   if (blk_rq_tagged(cmnd-request)) {
-   cmdinfo-stream = cmnd-request-tag + 2;
-   } else {
+   if (!blk_rq_tagged(cmnd-request))
devinfo-cmnd = cmnd;
-   cmdinfo-stream = 1;
-   }
 
cmnd-scsi_done = done;
 
-   cmdinfo-state = SUBMIT_STATUS_URB |
-   ALLOC_CMD_URB | SUBMIT_CMD_URB;
+   cmdinfo-stream = uas_get_tag(cmnd);
+   cmdinfo-state = SUBMIT_STATUS_URB | ALLOC_CMD_URB | SUBMIT_CMD_URB;
 
switch (cmnd-sc_data_direction) {
case DMA_FROM_DEVICE:
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 11/24] uas: Drop inflight list

[Hans de Goede]

We've the same info doubled in both the inflight list and the cmnd array,
drop the list.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 32 
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 85bbc1d..36c42b8 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -61,7 +61,6 @@ struct uas_dev_info {
struct scsi_cmnd *cmnd[MAX_CMNDS];
spinlock_t lock;
struct work_struct work;
-   struct list_head inflight_list;
 };
 
 enum {
@@ -87,7 +86,6 @@ struct uas_cmd_info {
struct urb *cmd_urb;
struct urb *data_in_urb;
struct urb *data_out_urb;
-   struct list_head list;
 };
 
 /* I hate forward declarations, but I actually have a loop */
@@ -103,18 +101,21 @@ 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 scsi_cmnd *cmnd;
unsigned long flags;
-   int err;
+   int i, err;
 
spin_lock_irqsave(devinfo-lock, flags);
 
if (devinfo-resetting)
goto out;
 
-   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);
+   for (i = 0; i  devinfo-qdepth; i++) {
+   if (!devinfo-cmnd[i])
+   continue;
+
+   cmnd = devinfo-cmnd[i];
+   cmdinfo = (void *)cmnd-SCp;
 
if (!(cmdinfo-state  IS_IN_WORK_LIST))
continue;
@@ -143,15 +144,17 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo)
 static void uas_zap_pending(struct uas_dev_info *devinfo, int result)
 {
struct uas_cmd_info *cmdinfo;
-   struct uas_cmd_info *temp;
+   struct scsi_cmnd *cmnd;
unsigned long flags;
-   int err;
+   int i, err;
 
spin_lock_irqsave(devinfo-lock, flags);
-   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);
+   for (i = 0; i  devinfo-qdepth; i++) {
+   if (!devinfo-cmnd[i])
+   continue;
+
+   cmnd = devinfo-cmnd[i];
+   cmdinfo = (void *)cmnd-SCp;
uas_log_cmd_state(cmnd, __func__);
/* Sense urbs were killed, clear COMMAND_INFLIGHT manually */
cmdinfo-state = ~COMMAND_INFLIGHT;
@@ -260,7 +263,6 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const 
char *caller)
cmdinfo-state |= COMMAND_COMPLETED;
if (cmdinfo-state  COMMAND_ABORTED)
scmd_printk(KERN_INFO, cmnd, abort completed\n);
-   list_del(cmdinfo-list);
devinfo-cmnd[uas_get_tag(cmnd) - 1] = NULL;
cmnd-scsi_done(cmnd);
return 0;
@@ -707,7 +709,6 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
}
 
devinfo-cmnd[stream - 1] = cmnd;
-   list_add_tail(cmdinfo-list, devinfo-inflight_list);
spin_unlock_irqrestore(devinfo-lock, flags);
return 0;
 }
@@ -917,7 +918,6 @@ 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-inflight_list);
 
result = uas_configure_endpoints(devinfo);
if (result)
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 02/24] uas: Remove task-management / abort error handling code

[Hans de Goede]

There are various bug reports about oopses / hangs with the uas driver,
which all point to the abort-command and logical-unit-reset (task-management)
error handling paths.

Getting these right is very hard, there are quite a few corner cases, and
testing is almost impossible since under normal operation these code paths
are not used at all.

Another problem is that there are also some cases where it simply is not clear
what to do at all. E.g. over usb-2 multiple outstanding commands share the same
endpoint. What if a command gets aborted while its sense urb is half way
through completing (so some data has been transfered but not all). Since the
urb is not yet complete we don't know if the sense urb is actually for this
command, or for one of the other oustanding commands. If it is for one of the
other commands and we cancel it, then we end up in an undefined state. But if
it is actually for the command we're aborting, and the abort succeeds, then it
may never complete...

This exact same problem applies to logical unit resets too, if there are
multiple luns, then commands outstanding on both luns share the sense
endpoint. If there is only a single lun, then doing a logical unit reset is
little better then doing a full usb device reset.

So summarizing because:
1) abort / lun-reset is very tricky to get right
2) Not being able to test the tricky code, which means it will have bugs
3) This being a code path which under normal operation will never happen,
   so being slow / sub-optimal here is not really an issue
4) Under error conditions we will still be able to recover through usb
   device resets.
5) This may be a bit slower in some cases, but this is actually faster in
   cases where the bridge ship has locked up, which seems to be the most
   common error case sofar.

This commit removes the abort / lun-reset error handling paths, and also the
taks-mgmt code since those are the only 2 task-mgmt users. Leaving only the
(tested and testable) usb-device-reset error handling path in place.

Note I realize that this is somewhat of a big hammer, but currently people
are seeing very hard to debug oopses with uas. First let focus on making uas
work reliable, then we can later look into adding more fine grained error
handling.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 177 +-
 1 file changed, 1 insertion(+), 176 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 8c7d4a2..4cf4d58 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -2,7 +2,7 @@
  * USB Attached SCSI
  * Note that this is not the same as the USB Mass Storage driver
  *
- * Copyright Hans de Goede hdego...@redhat.com for Red Hat, Inc. 2013
+ * Copyright Hans de Goede hdego...@redhat.com for Red Hat, Inc. 2013 - 2014
  * Copyright Matthew Wilcox for Intel Corp, 2010
  * Copyright Sarah Sharp for Intel Corp, 2010
  *
@@ -52,11 +52,9 @@ struct uas_dev_info {
struct usb_anchor data_urbs;
unsigned long flags;
int qdepth, resetting;
-   struct response_iu response;
unsigned cmd_pipe, status_pipe, data_in_pipe, data_out_pipe;
unsigned use_streams:1;
unsigned uas_sense_old:1;
-   unsigned running_task:1;
unsigned shutdown:1;
struct scsi_cmnd *cmnd;
spinlock_t lock;
@@ -207,7 +205,6 @@ 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);
 }
 
@@ -350,13 +347,6 @@ static void uas_stat_cmplt(struct urb *urb)
cmnd = scsi_host_find_tag(shost, tag - 1);
 
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));
-   }
usb_free_urb(urb);
spin_unlock_irqrestore(devinfo-lock, flags);
return;
@@ -536,56 +526,6 @@ static struct urb *uas_alloc_cmd_urb(struct uas_dev_info 
*devinfo, gfp_t gfp,
return NULL;
 }
 
-static int uas_submit_task_urb(struct scsi_cmnd *cmnd, gfp_t gfp,
-  u8 function, u16 stream_id)
-{
-   struct uas_dev_info *devinfo = (void *)cmnd-device-hostdata;
-   struct usb_device *udev = devinfo-udev;
-   struct urb *urb = usb_alloc_urb(0, gfp);
-   struct task_mgmt_iu *iu;
-   int err = -ENOMEM;
-
-   if (!urb)
-   goto err;
-
-   iu = kzalloc(sizeof(*iu), gfp);
-   if (!iu)
-   goto err;
-
-   iu-iu_id = IU_ID_TASK_MGMT;
-  

[PATCH v2 20/24] uas: Remove support for old sense ui as used in pre-production hardware

[Hans de Goede]

I've access to a number of different uas devices now, and none of them use
old style sense urbs. The only case where these code-paths trigger is with
the asm1051 and there they do the wrong thing, as the asm1051 sends 8 bytes
status iu-s when it does not have any sense data, but uses new style
sense iu-s regardless, as can be seen for scsi cmnds where there is sense
data.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 47 +--
 1 file changed, 1 insertion(+), 46 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index c6f7870..cbd4312 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -32,20 +32,6 @@
 
 #define MAX_CMNDS 256
 
-/*
- * The r00-r01c specs define this version of the SENSE IU data structure.
- * It's still in use by several different firmware releases.
- */
-struct sense_iu_old {
-   __u8 iu_id;
-   __u8 rsvd1;
-   __be16 tag;
-   __be16 len;
-   __u8 status;
-   __u8 service_response;
-   __u8 sense[SCSI_SENSE_BUFFERSIZE];
-};
-
 struct uas_dev_info {
struct usb_interface *intf;
struct usb_device *udev;
@@ -56,7 +42,6 @@ struct uas_dev_info {
int qdepth, resetting;
unsigned cmd_pipe, status_pipe, data_in_pipe, data_out_pipe;
unsigned use_streams:1;
-   unsigned uas_sense_old:1;
unsigned shutdown:1;
struct scsi_cmnd *cmnd[MAX_CMNDS];
spinlock_t lock;
@@ -187,29 +172,6 @@ static void uas_sense(struct urb *urb, struct scsi_cmnd 
*cmnd)
cmnd-result = sense_iu-status;
 }
 
-static void uas_sense_old(struct urb *urb, struct scsi_cmnd *cmnd)
-{
-   struct sense_iu_old *sense_iu = urb-transfer_buffer;
-   struct scsi_device *sdev = cmnd-device;
-
-   if (urb-actual_length  8) {
-   unsigned len = be16_to_cpup(sense_iu-len) - 2;
-   if (len + 8 != urb-actual_length) {
-   int newlen = min(len + 8, urb-actual_length) - 8;
-   if (newlen  0)
-   newlen = 0;
-   sdev_printk(KERN_INFO, sdev, %s: urb length %d 
-   disagrees with IU sense data length %d, 
-   using %d bytes of sense data\n, __func__,
-   urb-actual_length, len, newlen);
-   len = newlen;
-   }
-   memcpy(cmnd-sense_buffer, sense_iu-sense, len);
-   }
-
-   cmnd-result = sense_iu-status;
-}
-
 /*
  * scsi-tags go from 0 - (nr_tags - 1), uas tags need to match stream-ids,
  * which go from 1 - nr_streams. And we use 1 for untagged commands.
@@ -339,12 +301,7 @@ static void uas_stat_cmplt(struct urb *urb)
 
switch (iu-iu_id) {
case IU_ID_STATUS:
-   if (urb-actual_length  16)
-   devinfo-uas_sense_old = 1;
-   if (devinfo-uas_sense_old)
-   uas_sense_old(urb, cmnd);
-   else
-   uas_sense(urb, cmnd);
+   uas_sense(urb, cmnd);
if (cmnd-result != 0) {
/* cancel data transfers on error */
data_in_urb = usb_get_urb(cmdinfo-data_in_urb);
@@ -900,8 +857,6 @@ static int uas_configure_endpoints(struct uas_dev_info 
*devinfo)
struct usb_device *udev = devinfo-udev;
int r;
 
-   devinfo-uas_sense_old = 0;
-
r = uas_find_endpoints(devinfo-intf-cur_altsetting, eps);
if (r)
return r;
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 01/24] uas: replace WARN_ON_ONCE() with lockdep_assert_held()

[Hans de Goede]

From: Sanjeev Sharma sanjeev_sha...@mentor.com

On some architecture spin_is_locked() always return false in
uniprocessor configuration and therefore it would be advise to replace
with lockdep_assert_held().

Signed-off-by: Sanjeev Sharma sanjeev_sha...@mentor.com
Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 75d2ccd..8c7d4a2 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -156,7 +156,7 @@ static void uas_mark_cmd_dead(struct uas_dev_info *devinfo,
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));
+   lockdep_assert_held(devinfo-lock);
WARN_ON_ONCE(cmdinfo-state  COMMAND_ABORTED);
cmdinfo-state |= COMMAND_ABORTED;
cmdinfo-state = ~IS_IN_WORK_LIST;
@@ -183,7 +183,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_ONCE(!spin_is_locked(devinfo-lock));
+   lockdep_assert_held(devinfo-lock);
cmdinfo-state |= IS_IN_WORK_LIST;
schedule_work(devinfo-work);
 }
@@ -285,7 +285,7 @@ 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_ONCE(!spin_is_locked(devinfo-lock));
+   lockdep_assert_held(devinfo-lock);
if (cmdinfo-state  (COMMAND_INFLIGHT |
  DATA_IN_URB_INFLIGHT |
  DATA_OUT_URB_INFLIGHT |
@@ -624,7 +624,7 @@ static int uas_submit_urbs(struct scsi_cmnd *cmnd,
struct urb *urb;
int err;
 
-   WARN_ON_ONCE(!spin_is_locked(devinfo-lock));
+   lockdep_assert_held(devinfo-lock);
if (cmdinfo-state  SUBMIT_STATUS_URB) {
urb = uas_submit_sense_urb(cmnd, gfp, cmdinfo-stream);
if (!urb)
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 13/24] uas: Drop all references to a scsi_cmnd once it has been aborted

[Hans de Goede]

Do not keep references around to a cmnd which is under error handling.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 47 ---
 1 file changed, 44 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 1781b36..ce2d8fc 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -257,12 +257,11 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const 
char *caller)
lockdep_assert_held(devinfo-lock);
if (cmdinfo-state  (COMMAND_INFLIGHT |
  DATA_IN_URB_INFLIGHT |
- DATA_OUT_URB_INFLIGHT))
+ DATA_OUT_URB_INFLIGHT |
+ COMMAND_ABORTED))
return -EBUSY;
WARN_ON_ONCE(cmdinfo-state  COMMAND_COMPLETED);
cmdinfo-state |= COMMAND_COMPLETED;
-   if (cmdinfo-state  COMMAND_ABORTED)
-   scmd_printk(KERN_INFO, cmnd, abort completed\n);
devinfo-cmnd[uas_get_tag(cmnd) - 1] = NULL;
cmnd-scsi_done(cmnd);
return 0;
@@ -712,6 +711,47 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
 
 static DEF_SCSI_QCMD(uas_queuecommand)
 
+/*
+ * For now we do not support actually sending an abort to the device, so
+ * this eh always fails. Still we must define it to make sure that we've
+ * dropped all references to the cmnd in question once this function exits.
+ */
+static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
+{
+   struct uas_cmd_info *cmdinfo = (void *)cmnd-SCp;
+   struct uas_dev_info *devinfo = (void *)cmnd-device-hostdata;
+   struct urb *data_in_urb = NULL;
+   struct urb *data_out_urb = NULL;
+   unsigned long flags;
+
+   spin_lock_irqsave(devinfo-lock, flags);
+
+   uas_log_cmd_state(cmnd, __func__);
+
+   /* Ensure that try_complete does not call scsi_done */
+   cmdinfo-state |= COMMAND_ABORTED;
+
+   /* Drop all refs to this cmnd, kill data urbs to break their ref */
+   devinfo-cmnd[uas_get_tag(cmnd) - 1] = NULL;
+   if (cmdinfo-state  DATA_IN_URB_INFLIGHT)
+   data_in_urb = usb_get_urb(cmdinfo-data_in_urb);
+   if (cmdinfo-state  DATA_OUT_URB_INFLIGHT)
+   data_out_urb = usb_get_urb(cmdinfo-data_out_urb);
+
+   spin_unlock_irqrestore(devinfo-lock, flags);
+
+   if (data_in_urb) {
+   usb_kill_urb(data_in_urb);
+   usb_put_urb(data_in_urb);
+   }
+   if (data_out_urb) {
+   usb_kill_urb(data_out_urb);
+   usb_put_urb(data_out_urb);
+   }
+
+   return FAILED;
+}
+
 static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd)
 {
struct scsi_device *sdev = cmnd-device;
@@ -793,6 +833,7 @@ static struct scsi_host_template uas_host_template = {
.queuecommand = uas_queuecommand,
.slave_alloc = uas_slave_alloc,
.slave_configure = uas_slave_configure,
+   .eh_abort_handler = uas_eh_abort_handler,
.eh_bus_reset_handler = uas_eh_bus_reset_handler,
.can_queue = 65536, /* Is there a limit on the _host_ ? */
.this_id = -1,
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 06/24] uas: Check against unexpected completions

[Hans de Goede]

The status urb should not complete before the command has been submitted, nor
should we get a second status urb for the same tag after a IU_ID_STATUS.

Data urbs should not complete before the command has been submitted, but may
complete after the IU_ID_STATUS.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index b58d133..8614ddf 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -371,6 +371,12 @@ static void uas_stat_cmplt(struct urb *urb)
 
cmnd = devinfo-cmnd[idx];
cmdinfo = (void *)cmnd-SCp;
+
+   if (!(cmdinfo-state  COMMAND_INFLIGHT)) {
+   scmd_printk(KERN_ERR, cmnd, unexpected status cmplt\n);
+   goto out;
+   }
+
switch (iu-iu_id) {
case IU_ID_STATUS:
if (urb-actual_length  16)
@@ -436,6 +442,12 @@ static void uas_data_cmplt(struct urb *urb)
if (devinfo-resetting)
goto out;
 
+   /* Data urbs should not complete before the cmd urb is submitted */
+   if (cmdinfo-state  SUBMIT_CMD_URB) {
+   scmd_printk(KERN_ERR, cmnd, unexpected data cmplt\n);
+   goto out;
+   }
+
if (urb-status) {
if (urb-status != -ECONNRESET) {
uas_log_cmd_state(cmnd, __func__);
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 09/24] uas: Simplify reset / disconnect handling

[Hans de Goede]

Drop the whole dance with first moving cmnds to a dead-list. The resetting
flag ensures that no new cmds / urbs will be submitted, and that any urb
completions are short-circuited without trying to complete the scsi cmnd.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 43 ++-
 1 file changed, 6 insertions(+), 37 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 70b44f0..08edb6b 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -62,7 +62,6 @@ struct uas_dev_info {
spinlock_t lock;
struct work_struct work;
struct list_head inflight_list;
-   struct list_head dead_list;
 };
 
 enum {
@@ -130,35 +129,6 @@ out:
spin_unlock_irqrestore(devinfo-lock, flags);
 }
 
-static void uas_mark_cmd_dead(struct uas_dev_info *devinfo,
- 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);
-
-   uas_log_cmd_state(cmnd, caller);
-   lockdep_assert_held(devinfo-lock);
-   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, int result,
-  const char *caller)
-{
-   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-inflight_list, list)
-   uas_mark_cmd_dead(devinfo, cmdinfo, result, caller);
-   spin_unlock_irqrestore(devinfo-lock, flags);
-}
-
 static void uas_add_work(struct uas_cmd_info *cmdinfo)
 {
struct scsi_pointer *scp = (void *)cmdinfo;
@@ -170,7 +140,7 @@ static void uas_add_work(struct uas_cmd_info *cmdinfo)
schedule_work(devinfo-work);
 }
 
-static void uas_zap_dead(struct uas_dev_info *devinfo)
+static void uas_zap_pending(struct uas_dev_info *devinfo, int result)
 {
struct uas_cmd_info *cmdinfo;
struct uas_cmd_info *temp;
@@ -182,11 +152,11 @@ 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_ONCE(!(cmdinfo-state  COMMAND_ABORTED));
/* all urbs are killed, clear inflight bits */
cmdinfo-state = ~(COMMAND_INFLIGHT |
DATA_IN_URB_INFLIGHT |
DATA_OUT_URB_INFLIGHT);
+   cmnd-result = result  16;
uas_try_complete(cmnd, __func__);
}
spin_unlock_irqrestore(devinfo-lock, flags);
@@ -765,11 +735,11 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd 
*cmnd)
devinfo-resetting = 1;
spin_unlock_irqrestore(devinfo-lock, flags);
 
-   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);
-   uas_zap_dead(devinfo);
+   uas_zap_pending(devinfo, DID_RESET);
+
err = usb_reset_device(udev);
 
spin_lock_irqsave(devinfo-lock, flags);
@@ -948,7 +918,6 @@ 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-inflight_list);
-   INIT_LIST_HEAD(devinfo-dead_list);
 
result = uas_configure_endpoints(devinfo);
if (result)
@@ -1076,11 +1045,11 @@ static void uas_disconnect(struct usb_interface *intf)
spin_unlock_irqrestore(devinfo-lock, flags);
 
cancel_work_sync(devinfo-work);
-   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);
-   uas_zap_dead(devinfo);
+   uas_zap_pending(devinfo, DID_NO_CONNECT);
+
scsi_remove_host(shost);
uas_free_streams(devinfo);
scsi_host_put(shost);
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: fix regression that accidentally disabled block-based tcq

[Hans de Goede]

Hi,

On 09/13/2014 01:00 AM, Christoph Hellwig wrote:
 Please try the fix below, looks like the commit broke TCQ for all drivers 
 using block-level tagging.

Yes this one does the trick and fixes things. Note the git tree I used for
testing also had your previous fix to split up the blk_tcq union in 2
separate struct members. Let me know if you want me to re-test without that
fix.

Thanks  Regards,

Hans


 
 ---
 From 865a19b760d2786fe37d3b5c151a4ecea4c0e95e Mon Sep 17 00:00:00 2001
 From: Christoph Hellwig h...@lst.de
 Date: Fri, 12 Sep 2014 16:00:19 -0700
 Subject: scsi: fix regression that accidentally disabled block-based tcq
 
 The scsi blk-mq support accidentally flipped a conditional, which lead to
 never enabling block based tcq when using the legacy request path.
 
 Fixes: d285203cf647d7c9 scsi: add support for a blk-mq based I/O path.
 Reported-by: Hans de Goede hdego...@redhat.com
 Signed-off-by: Christoph Hellwig h...@lst.de
 ---
  include/scsi/scsi_tcq.h | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
 
 diff --git a/include/scsi/scsi_tcq.h b/include/scsi/scsi_tcq.h
 index cdcc90b..e645835 100644
 --- a/include/scsi/scsi_tcq.h
 +++ b/include/scsi/scsi_tcq.h
 @@ -68,7 +68,7 @@ static inline void scsi_activate_tcq(struct scsi_device 
 *sdev, int depth)
   return;
  
   if (!shost_use_blk_mq(sdev-host) 
 - blk_queue_tagged(sdev-request_queue))
 + !blk_queue_tagged(sdev-request_queue))
   blk_queue_init_tags(sdev-request_queue, depth,
   sdev-host-bqt);
  
 
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 08/24] uas: Free data urbs on completion

[Hans de Goede]

Now that we no longer drop our lock to unlink the data urbs, we can simply
free them on completion, making their handling consistent with the other urbs.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 68853b5..70b44f0 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -288,8 +288,6 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const 
char *caller)
return -EBUSY;
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);
if (cmdinfo-state  COMMAND_ABORTED)
scmd_printk(KERN_INFO, cmnd, abort completed\n);
list_del(cmdinfo-list);
@@ -418,9 +416,11 @@ static void uas_data_cmplt(struct urb *urb)
if (cmdinfo-data_in_urb == urb) {
sdb = scsi_in(cmnd);
cmdinfo-state = ~DATA_IN_URB_INFLIGHT;
+   cmdinfo-data_in_urb = NULL;
} else if (cmdinfo-data_out_urb == urb) {
sdb = scsi_out(cmnd);
cmdinfo-state = ~DATA_OUT_URB_INFLIGHT;
+   cmdinfo-data_out_urb = NULL;
}
if (sdb == NULL) {
WARN_ON_ONCE(1);
@@ -450,6 +450,7 @@ static void uas_data_cmplt(struct urb *urb)
}
uas_try_complete(cmnd, __func__);
 out:
+   usb_free_urb(urb);
spin_unlock_irqrestore(devinfo-lock, flags);
 }
 
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 18/24] uas: Use scsi_print_command

[Hans de Goede]

Use scsi_print_command to print commands during errors, rather then printing
the rather meaningless pointer to the command.

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 90afe4a..652b97b 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -232,8 +232,8 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const 
char *caller)
struct uas_cmd_info *ci = (void *)cmnd-SCp;
 
scmd_printk(KERN_INFO, cmnd,
-   %s %p tag %d, inflight:%s%s%s%s%s%s%s%s%s%s%s%s%s\n,
-   caller, cmnd, uas_get_tag(cmnd),
+   %s tag %d inflight:%s%s%s%s%s%s%s%s%s%s%s%s%s ,
+   caller, uas_get_tag(cmnd),
(ci-state  SUBMIT_STATUS_URB) ?  s-st  : ,
(ci-state  ALLOC_DATA_IN_URB) ?  a-in  : ,
(ci-state  SUBMIT_DATA_IN_URB)?  s-in  : ,
@@ -247,6 +247,7 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, const 
char *caller)
(ci-state  COMMAND_COMPLETED) ?  done  : ,
(ci-state  COMMAND_ABORTED)   ?  abort : ,
(ci-state  IS_IN_WORK_LIST)   ?  work  : );
+   scsi_print_command(cmnd);
 }
 
 static void uas_free_unsubmitted_urbs(struct scsi_cmnd *cmnd)
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 03/24] uas: Fix resetting flag handling

[Hans de Goede]

- Make sure we always hold the lock when setting / checking resetting
- Check resetting before checking urb-status
- Add missing check for resetting to uas_data_cmplt
- Add missing check for resetting to uas_do_work

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 49 +--
 1 file changed, 35 insertions(+), 14 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 4cf4d58..0d97602 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -129,6 +129,10 @@ static void uas_do_work(struct work_struct *work)
int err;
 
spin_lock_irqsave(devinfo-lock, flags);
+
+   if (devinfo-resetting)
+   goto out;
+
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,
@@ -143,6 +147,7 @@ static void uas_do_work(struct work_struct *work)
else
schedule_work(devinfo-work);
}
+out:
spin_unlock_irqrestore(devinfo-lock, flags);
 }
 
@@ -322,6 +327,11 @@ static void uas_stat_cmplt(struct urb *urb)
unsigned long flags;
u16 tag;
 
+   spin_lock_irqsave(devinfo-lock, flags);
+
+   if (devinfo-resetting)
+   goto out;
+
if (urb-status) {
if (urb-status == -ENOENT) {
dev_err(urb-dev-dev, stat urb: killed, stream %d\n,
@@ -330,27 +340,17 @@ static void uas_stat_cmplt(struct urb *urb)
dev_err(urb-dev-dev, stat urb: status %d\n,
urb-status);
}
-   usb_free_urb(urb);
-   return;
-   }
-
-   if (devinfo-resetting) {
-   usb_free_urb(urb);
-   return;
+   goto out;
}
 
-   spin_lock_irqsave(devinfo-lock, flags);
tag = be16_to_cpup(iu-tag) - 1;
if (tag == 0)
cmnd = devinfo-cmnd;
else
cmnd = scsi_host_find_tag(shost, tag - 1);
 
-   if (!cmnd) {
-   usb_free_urb(urb);
-   spin_unlock_irqrestore(devinfo-lock, flags);
-   return;
-   }
+   if (!cmnd)
+   goto out;
 
cmdinfo = (void *)cmnd-SCp;
switch (iu-iu_id) {
@@ -391,6 +391,7 @@ static void uas_stat_cmplt(struct urb *urb)
scmd_printk(KERN_ERR, cmnd,
Bogus IU (%d) received on status pipe\n, iu-iu_id);
}
+out:
usb_free_urb(urb);
spin_unlock_irqrestore(devinfo-lock, flags);
 }
@@ -404,6 +405,7 @@ static void uas_data_cmplt(struct urb *urb)
unsigned long flags;
 
spin_lock_irqsave(devinfo-lock, flags);
+
if (cmdinfo-data_in_urb == urb) {
sdb = scsi_in(cmnd);
cmdinfo-state = ~DATA_IN_URB_INFLIGHT;
@@ -413,7 +415,13 @@ static void uas_data_cmplt(struct urb *urb)
}
if (sdb == NULL) {
WARN_ON_ONCE(1);
-   } else if (urb-status) {
+   goto out;
+   }
+
+   if (devinfo-resetting)
+   goto out;
+
+   if (urb-status) {
if (urb-status != -ECONNRESET) {
uas_log_cmd_state(cmnd, __func__);
scmd_printk(KERN_ERR, cmnd,
@@ -426,6 +434,7 @@ static void uas_data_cmplt(struct urb *urb)
sdb-resid = sdb-length - urb-actual_length;
}
uas_try_complete(cmnd, __func__);
+out:
spin_unlock_irqrestore(devinfo-lock, flags);
 }
 
@@ -732,6 +741,7 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd *cmnd)
struct scsi_device *sdev = cmnd-device;
struct uas_dev_info *devinfo = sdev-hostdata;
struct usb_device *udev = devinfo-udev;
+   unsigned long flags;
int err;
 
err = usb_lock_device_for_reset(udev, devinfo-intf);
@@ -742,14 +752,21 @@ static int uas_eh_bus_reset_handler(struct scsi_cmnd 
*cmnd)
}
 
shost_printk(KERN_INFO, sdev-host, %s start\n, __func__);
+
+   spin_lock_irqsave(devinfo-lock, flags);
devinfo-resetting = 1;
+   spin_unlock_irqrestore(devinfo-lock, flags);
+
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);
uas_zap_dead(devinfo);
err = usb_reset_device(udev);
+
+   spin_lock_irqsave(devinfo-lock, flags);
devinfo-resetting = 0;
+   spin_unlock_irqrestore(devinfo-lock, flags);
 
usb_unlock_device(udev);
 
@@ -1045,8 +1062,12 @@ static void uas_disconnect(struct usb_interface *intf)
 {
struct Scsi_Host *shost = usb_get_intfdata(intf);
struct uas_dev_info *devinfo = (struct uas_dev_info *)shost-hostdata;
+  

[PATCH v2 14/24] uas: Fix memleak of non-submitted urbs

[Hans de Goede]

Not all urbs we've allocated are necessarily also submitted, non-submitted
urbs will not be free-ed by their completion handler. So we need to free
them manually.

There are 2 scenarios where this can happen:

1) We have failed to submit some urbs at abort / disconnect
2) When running over usb-2 we may have never tried to submit the data urbs
   when completing the scsi cmnd, because we never got a READ/WRITE_READY iu

Signed-off-by: Hans de Goede hdego...@redhat.com
---
 drivers/usb/storage/uas.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index ce2d8fc..fca8775 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -249,6 +249,25 @@ static void uas_log_cmd_state(struct scsi_cmnd *cmnd, 
const char *caller)
(ci-state  IS_IN_WORK_LIST)   ?  work  : );
 }
 
+static void uas_free_unsubmitted_urbs(struct scsi_cmnd *cmnd)
+{
+   struct uas_cmd_info *cmdinfo;
+
+   if (!cmnd)
+   return;
+
+   cmdinfo = (void *)cmnd-SCp;
+
+   if (cmdinfo-state  SUBMIT_CMD_URB)
+   usb_free_urb(cmdinfo-cmd_urb);
+
+   /* data urbs may have never gotten their submit flag set */
+   if (!(cmdinfo-state  DATA_IN_URB_INFLIGHT))
+   usb_free_urb(cmdinfo-data_in_urb);
+   if (!(cmdinfo-state  DATA_OUT_URB_INFLIGHT))
+   usb_free_urb(cmdinfo-data_out_urb);
+}
+
 static int uas_try_complete(struct scsi_cmnd *cmnd, const char *caller)
 {
struct uas_cmd_info *cmdinfo = (void *)cmnd-SCp;
@@ -263,6 +282,7 @@ static int uas_try_complete(struct scsi_cmnd *cmnd, const 
char *caller)
WARN_ON_ONCE(cmdinfo-state  COMMAND_COMPLETED);
cmdinfo-state |= COMMAND_COMPLETED;
devinfo-cmnd[uas_get_tag(cmnd) - 1] = NULL;
+   uas_free_unsubmitted_urbs(cmnd);
cmnd-scsi_done(cmnd);
return 0;
 }
@@ -738,6 +758,8 @@ static int uas_eh_abort_handler(struct scsi_cmnd *cmnd)
if (cmdinfo-state  DATA_OUT_URB_INFLIGHT)
data_out_urb = usb_get_urb(cmdinfo-data_out_urb);
 
+   uas_free_unsubmitted_urbs(cmnd);
+
spin_unlock_irqrestore(devinfo-lock, flags);
 
if (data_in_urb) {
-- 
2.1.0

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH fix for 3.17] uas: Add a quirk for rejecting ATA_12 and ATA_16 commands

[Alan Stern]

On Sat, 13 Sep 2014, Hans de Goede wrote:

 And set this quirk for the Seagate Expansion Desk (0bc2:2312), as that one
 seems to hang upon receiving an ATA_12 or ATA_16 command.
 
 https://bugzilla.kernel.org/show_bug.cgi?id=79511
 
 Cc: sta...@vger.kernel.org # 3.16
 Signed-off-by: Hans de Goede hdego...@redhat.com

 --- a/drivers/usb/storage/usb.c
 +++ b/drivers/usb/storage/usb.c
 @@ -478,7 +478,8 @@ void usb_stor_adjust_quirks(struct usb_device *udev, 
 unsigned long *fflags)
   US_FL_CAPACITY_OK | US_FL_IGNORE_RESIDUE |
   US_FL_SINGLE_LUN | US_FL_NO_WP_DETECT |
   US_FL_NO_READ_DISC_INFO | US_FL_NO_READ_CAPACITY_16 |
 - US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE);
 + US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE |
 + US_FL_NO_ATA_1X);
  
   p = quirks;
   while (*p) {
 @@ -543,6 +544,9 @@ void usb_stor_adjust_quirks(struct usb_device *udev, 
 unsigned long *fflags)
   case 's':
   f |= US_FL_SINGLE_LUN;
   break;
 + case 't':
 + f |= US_FL_NO_ATA_1X;
 + break;
   case 'u':
   f |= US_FL_IGNORE_UAS;
   break;

You must not add an aditional value for a module parameter without 
documenting it in Documentation/kernel-parameters.txt.

Alan Stern

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] scsi: fix regression that accidentally disabled block-based tcq

[Christoph Hellwig]

On Sat, Sep 13, 2014 at 12:28:41PM +0200, Hans de Goede wrote:
 Yes this one does the trick and fixes things. Note the git tree I used for
 testing also had your previous fix to split up the blk_tcq union in 2
 separate struct members. Let me know if you want me to re-test without that
 fix.

I'm pretty sure that isn't needed, so double testing it indeed doesn't
would be helpful.  I'd like to add your Tested-by: tag after that, too.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH fix for 3.17 v2] uas: Add a quirk for rejecting ATA_12 and ATA_16 commands

[Hans de Goede]

And set this quirk for the Seagate Expansion Desk (0bc2:2312), as that one
seems to hang upon receiving an ATA_12 or ATA_16 command.

https://bugzilla.kernel.org/show_bug.cgi?id=79511

While at it also add missing documentation for the u value for usb-storage
quirks.

Cc: sta...@vger.kernel.org # 3.16
Signed-off-by: Hans de Goede hdego...@redhat.com

--
Changes in v2: Add documentation for new t and u usb-storage.quirks flags
---
 Documentation/kernel-parameters.txt |  3 +++
 drivers/usb/storage/uas.c   | 13 +
 drivers/usb/storage/unusual_uas.h   | 16 ++--
 drivers/usb/storage/usb.c   |  6 +-
 include/linux/usb_usual.h   |  2 ++
 5 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 5ae8608..7c32053 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3541,6 +3541,9 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
bogus residue values);
s = SINGLE_LUN (the device has only one
Logical Unit);
+   t = NO_ATA_1X (don't allow ATA12 and ATA12
+   commands, uas only);
+   u = IGNORE_UAS (do not try to use uas);
w = NO_WP_DETECT (don't test whether the
medium is write-protected).
Example: quirks=0419:aaf5:rl,0421:0433:rc
diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 3f42785..75d2ccd 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -28,6 +28,7 @@
 #include scsi/scsi_tcq.h
 
 #include uas-detect.h
+#include scsiglue.h
 
 /*
  * The r00-r01c specs define this version of the SENSE IU data structure.
@@ -49,6 +50,7 @@ struct uas_dev_info {
struct usb_anchor cmd_urbs;
struct usb_anchor sense_urbs;
struct usb_anchor data_urbs;
+   unsigned long flags;
int qdepth, resetting;
struct response_iu response;
unsigned cmd_pipe, status_pipe, data_in_pipe, data_out_pipe;
@@ -714,6 +716,15 @@ static int uas_queuecommand_lck(struct scsi_cmnd *cmnd,
 
BUILD_BUG_ON(sizeof(struct uas_cmd_info)  sizeof(struct scsi_pointer));
 
+   if ((devinfo-flags  US_FL_NO_ATA_1X) 
+   (cmnd-cmnd[0] == ATA_12 || cmnd-cmnd[0] == ATA_16)) {
+   memcpy(cmnd-sense_buffer, usb_stor_sense_invalidCDB,
+  sizeof(usb_stor_sense_invalidCDB));
+   cmnd-result = SAM_STAT_CHECK_CONDITION;
+   cmnd-scsi_done(cmnd);
+   return 0;
+   }
+
spin_lock_irqsave(devinfo-lock, flags);
 
if (devinfo-resetting) {
@@ -1080,6 +1091,8 @@ static int uas_probe(struct usb_interface *intf, const 
struct usb_device_id *id)
devinfo-resetting = 0;
devinfo-running_task = 0;
devinfo-shutdown = 0;
+   devinfo-flags = id-driver_info;
+   usb_stor_adjust_quirks(udev, devinfo-flags);
init_usb_anchor(devinfo-cmd_urbs);
init_usb_anchor(devinfo-sense_urbs);
init_usb_anchor(devinfo-data_urbs);
diff --git a/drivers/usb/storage/unusual_uas.h 
b/drivers/usb/storage/unusual_uas.h
index 724..52f7012 100644
--- a/drivers/usb/storage/unusual_uas.h
+++ b/drivers/usb/storage/unusual_uas.h
@@ -40,13 +40,9 @@
  * and don't forget to CC: the USB development list linux-usb@vger.kernel.org
  */
 
-/*
- * This is an example entry for the US_FL_IGNORE_UAS flag. Once we have an
- * actual entry using US_FL_IGNORE_UAS this entry should be removed.
- *
- * UNUSUAL_DEV(  0xabcd, 0x1234, 0x0100, 0x0100,
- * Example,
- * Storage with broken UAS,
- * USB_SC_DEVICE, USB_PR_DEVICE, NULL,
- * US_FL_IGNORE_UAS),
- */
+/* https://bugzilla.kernel.org/show_bug.cgi?id=79511 */
+UNUSUAL_DEV(0x0bc2, 0x2312, 0x, 0x,
+   Seagate,
+   Expansion Desk,
+   USB_SC_DEVICE, USB_PR_DEVICE, NULL,
+   US_FL_NO_ATA_1X),
diff --git a/drivers/usb/storage/usb.c b/drivers/usb/storage/usb.c
index cedb292..b9d1b93 100644
--- a/drivers/usb/storage/usb.c
+++ b/drivers/usb/storage/usb.c
@@ -478,7 +478,8 @@ void usb_stor_adjust_quirks(struct usb_device *udev, 
unsigned long *fflags)
US_FL_CAPACITY_OK | US_FL_IGNORE_RESIDUE |
US_FL_SINGLE_LUN | US_FL_NO_WP_DETECT |
US_FL_NO_READ_DISC_INFO | US_FL_NO_READ_CAPACITY_16 |
-   US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE);
+   US_FL_INITIAL_READ10 | US_FL_WRITE_CACHE |
+   US_FL_NO_ATA_1X);
 
p = quirks;
while (*p) {
@@ -543,6 +544,9 @@ void usb_stor_adjust_quirks(struct 

Re: [PATCH v2 10/24] uas: zap_pending: data urbs should have completed at this time

[Sergei Shtylyov]

Hello.

On 9/13/2014 1:26 PM, Hans de Goede wrote:


The data urbs are all killed before calling zap_pending, and their completion
handler should have cleared their inflight flag.



Do not 0 the data inflight flags, and add a check for try_complete succeeding,
as it should always succeed when called from zap_pending.



Signed-off-by: Hans de Goede hdego...@redhat.com
---
  drivers/usb/storage/uas.c | 10 +-
  1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/storage/uas.c b/drivers/usb/storage/uas.c
index 08edb6b..85bbc1d 100644
--- a/drivers/usb/storage/uas.c
+++ b/drivers/usb/storage/uas.c
@@ -145,6 +145,7 @@ static void uas_zap_pending(struct uas_dev_info *devinfo, 
int result)
struct uas_cmd_info *cmdinfo;
struct uas_cmd_info *temp;
unsigned long flags;
+   int err;


   Er, I don't see why this variable is necessary.

[...]

@@ -152,12 +153,11 @@ static void uas_zap_pending(struct uas_dev_info *devinfo, 
int result)
struct scsi_cmnd *cmnd = container_of(scp, struct scsi_cmnd,
  SCp);
uas_log_cmd_state(cmnd, __func__);
-   /* all urbs are killed, clear inflight bits */
-   cmdinfo-state = ~(COMMAND_INFLIGHT |
-   DATA_IN_URB_INFLIGHT |
-   DATA_OUT_URB_INFLIGHT);
+   /* Sense urbs were killed, clear COMMAND_INFLIGHT manually */
+   cmdinfo-state = ~COMMAND_INFLIGHT;
cmnd-result = result  16;
-   uas_try_complete(cmnd, __func__);
+   err = uas_try_complete(cmnd, __func__);
+   WARN_ON(err != 0);


   Why not:

WARN_ON(uas_try_complete(cmnd, __func__));


}
spin_unlock_irqrestore(devinfo-lock, flags);
  }


WBR, Sergei

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH fix for 3.17 v2] uas: Add a quirk for rejecting ATA_12 and ATA_16 commands

[Thomas Backlund]

Den 13.09.2014 21:02, Hans de Goede skrev:

And set this quirk for the Seagate Expansion Desk (0bc2:2312), as that one
seems to hang upon receiving an ATA_12 or ATA_16 command.

https://bugzilla.kernel.org/show_bug.cgi?id=79511

While at it also add missing documentation for the u value for usb-storage
quirks.

Cc: sta...@vger.kernel.org # 3.16
Signed-off-by: Hans de Goede hdego...@redhat.com

--
Changes in v2: Add documentation for new t and u usb-storage.quirks flags
---
  Documentation/kernel-parameters.txt |  3 +++
  drivers/usb/storage/uas.c   | 13 +
  drivers/usb/storage/unusual_uas.h   | 16 ++--
  drivers/usb/storage/usb.c   |  6 +-
  include/linux/usb_usual.h   |  2 ++
  5 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/Documentation/kernel-parameters.txt 
b/Documentation/kernel-parameters.txt
index 5ae8608..7c32053 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3541,6 +3541,9 @@ bytes respectively. Such letter suffixes can also be 
entirely omitted.
bogus residue values);
s = SINGLE_LUN (the device has only one
Logical Unit);
+   t = NO_ATA_1X (don't allow ATA12 and ATA12
+   commands, uas only);


I guess you meant ATA12 and *ATA16*


+   u = IGNORE_UAS (do not try to use uas);
w = NO_WP_DETECT (don't test whether the
medium is write-protected).
Example: quirks=0419:aaf5:rl,0421:0433:rc


--

Thomas

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH][RFC] storage: Add quirk for Adaptec USBConnect 2000 USB-to-SCSI Adapter

[Mark]

Hi,

The Adaptec USBConnect 2000 is another SCSI-USB converter which uses
Shuttle Technology/SCM Microsystems chips. The US_FL_SCM_MULT_TARG quirk
is required to use SCSI devices with ID other than 0.

I don't have a USBConnect 2000, but based on the other entries for
Shuttle/SCM-based converters this patch is very likely correct.
Confirmation would be good, especially for bcdDeviceMin and bcdDeviceMax
fields. If anyone reading this has a USBConnect 2000, connect it to a SCSI
device, power on the device then connect the USB plug to your computer.
Then do (as root)
  lsusb -v -d 03f3:0001
and post the result.


Signed-off-by: Mark Knibbs ma...@clara.co.uk
---

diff -up linux-3.17-rc4/drivers/usb/storage/unusual_devs.h.orig 
linux-3.17-rc4/drivers/usb/storage/unusual_devs.h
--- linux-3.17-rc4/drivers/usb/storage/unusual_devs.h.orig  2014-09-08 
00:09:43.0 +0100
+++ linux-3.17-rc4/drivers/usb/storage/unusual_devs.h   2014-09-13 
20:47:01.0 +0100
@@ -101,6 +101,12 @@ UNUSUAL_DEV(  0x03f0, 0x4002, 0x0001, 0x
PhotoSmart R707,
USB_SC_DEVICE, USB_PR_DEVICE, NULL, US_FL_FIX_CAPACITY),
 
+UNUSUAL_DEV(  0x03f3, 0x0001, 0x0100, 0x0100,
+   Adaptec,
+   USBConnect 2000,
+   USB_SC_DEVICE, USB_PR_DEVICE, usb_stor_euscsi_init,
+   US_FL_SCM_MULT_TARG ),
+
 /* Reported by Sebastian Kapfer sebastian_kap...@gmx.net
  * and Olaf Hering o...@suse.de (different bcd's, same vendor/product)
  * for USB floppies that need the SINGLE_LUN enforcement.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH fix for 3.17 v2] uas: Add a quirk for rejecting ATA_12 and ATA_16 commands

[Sergei Shtylyov]

Hello.

On 9/13/2014 10:21 PM, Thomas Backlund wrote:


And set this quirk for the Seagate Expansion Desk (0bc2:2312), as that one
seems to hang upon receiving an ATA_12 or ATA_16 command.



https://bugzilla.kernel.org/show_bug.cgi?id=79511



While at it also add missing documentation for the u value for usb-storage
quirks.



Cc: sta...@vger.kernel.org # 3.16
Signed-off-by: Hans de Goede hdego...@redhat.com



--
Changes in v2: Add documentation for new t and u usb-storage.quirks flags
---
  Documentation/kernel-parameters.txt |  3 +++
  drivers/usb/storage/uas.c   | 13 +
  drivers/usb/storage/unusual_uas.h   | 16 ++--
  drivers/usb/storage/usb.c   |  6 +-
  include/linux/usb_usual.h   |  2 ++
  5 files changed, 29 insertions(+), 11 deletions(-)



diff --git a/Documentation/kernel-parameters.txt
b/Documentation/kernel-parameters.txt
index 5ae8608..7c32053 100644
--- a/Documentation/kernel-parameters.txt
+++ b/Documentation/kernel-parameters.txt
@@ -3541,6 +3541,9 @@ bytes respectively. Such letter suffixes can also be
entirely omitted.
  bogus residue values);
  s = SINGLE_LUN (the device has only one
  Logical Unit);
+t = NO_ATA_1X (don't allow ATA12 and ATA12
+commands, uas only);



I guess you meant ATA12 and *ATA16*


   ... or even ATA(12) and ATA(16). As far as I remember SCSI, it has CDB 
length in parens for the command names.


WBR, Sergei

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH net-next v2] r8152: support VLAN

[David Miller]

From: Hayes Wang hayesw...@realtek.com
Date: Fri, 12 Sep 2014 10:43:11 +0800

 Support hw VLAN for tx and rx. And enable them by default.
 
 Signed-off-by: Hayes Wang hayesw...@realtek.com

Applied.
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: XHCI, brain-dead scanner, and microframe rounding

[Mike Mammarella]

On Sat, 26 Jul 2014, Mike Mammarella wrote:


On Fri, 4 Jul 2014, Mathias Nyman wrote:


On 07/01/2014 09:07 AM, Mike Mammarella wrote:

Hi

Can you add xhci debugging by enabling CONFIG_DYNAMIC_DEBUG, and run
`echo -n 'module xhci_hcd =p'  /sys/kernel/debug/dynamic_debug/control`
as root,
and send me the output of dmesg.

Without debugging info it's hard to guess what's going on.

The microframe rounding look a bit suspicious:
[12864.453456] usb 3-4: ep 0x81 - rounding interval to 128 microframes, 
ep desc says 255 microframes


xhci specs says it needs the interval rounded to nearest 2^(X) value, 
which would be 256, not 128. I'll take a look at that.


An other possibility is that it's related to how xhci handles halted 
endpoints. I got some untested code to fix this, It needs a lot of 
cleanup but can be tested.


If you are able to test my ep_reset_halt_test branch (with xhci 
debugging) I'd be interested to know if it helps.


Code is at:
git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git 
ep_reset_halt_test


-Mathias


Thanks! I've built a kernel from fb58633e with CONFIG_DYNAMIC_DEBUG 
enabled.

(I also had to mount debugfs, it turns out.) The scanner does not work in
this configuration. I've posted the logs here:

http://spark.crystalorb.net/mikem/dmesg.log
http://spark.crystalorb.net/mikem/scanadf.log

dmesg seems to have much more information than what showed up on the
console (which showed only MATTU messages); it may be relevant when
sifting through that output that the root file system is also on USB.



Thanks,

Took a quick look, but can't find any obvious reason why it fails.
I'll be out of office next week, but I'll try to take a better look again 
when I return


usbmon traces of this could give some hint on what is happening

-Mathias



Sorry for the delay getting back to you - I've captured traces with EHCI 
(works) and XHCI (fails). In both cases the scanner is device 10 on bus 1. 
The kernel is the Debian 3.14.12 kernel (so, in particular, not your 
ep_reset_halt_test branch), in case that's important for these traces.


http://spark.crystalorb.net/mikem/usbmon-success.log
http://spark.crystalorb.net/mikem/usbmon-failure.log

I appreciate your help looking into this!

Mike


Hi, just curious if these traces were useful? Is there any other 
information I could collect that would help?


Thanks!

Mike
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 0/5] usb: phy: samsung: remove old USB PHY code

[Kukjin Kim]

On 08/23/14 02:14, Bartlomiej Zolnierkiewicz wrote:


Hi,

On Wednesday, August 20, 2014 01:12:44 PM Felipe Balbi wrote:

Hi,

On Thu, Aug 14, 2014 at 04:25:22PM +0200, Bartlomiej Zolnierkiewicz wrote:

Hi,

This patch series removes the old Samsung USB PHY drivers that
got replaced by the new ones using the generic PHY layer.

Depends on:
- next-20140813 branch of linux-next kernel


this thread seems to have died, what do I need to do with these patches?


Please apply them (patches #3, #4 and #5).  Patches #1 and #2
should be applied (or Acked-by) by Kukjin Kim.


I've applied #1 and #2, sorry for late taking.

Thanks,
Kukjin


Are we deleting the phy drivers or not ?


Yes, we are deleting them.  It has been agreed on by Kishon and
Vivek.


Please rebase on v3.17-rc1 and resend with all Acks in place.


Done:

https://lkml.org/lkml/2014/8/22/446

Please note that if you want to apply it to current -next kernel
(next-20140822) then you need to resolve conflict between patch
#5/5 (usb: phy: samsung: remove old common USB PHY code) and
commit bbc66e140bab (usb: phy: samsung: Fix wrong bit mask for
PHYPARAM1_PCS_TXDEEMPH) by simply removing the new version of
drivers/usb/phy/phy-samsung-usb.h file.

--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: Hitting unused qh not empty BUG in qh_destroy

[Joe Lawrence]

On Fri, 12 Sep 2014, Alan Stern wrote:

 On Fri, 12 Sep 2014, Joe Lawrence wrote:
 
  On Fri, 12 Sep 2014 11:31:46 -0400
  Alan Stern st...@rowland.harvard.edu wrote:
  
   On Thu, 11 Sep 2014, Joe Lawrence wrote:
   
Hi Alan,

I've got another USB bug to report that manifests during automated
device removal testing on RHEL7.  This one hits the BUG() inside
qh_destroy:
   
   How reliably can you trigger this bug?
  
  I have collected a few crashes within a few days, so somewhat
  frequently.
  
 67 static void qh_destroy(struct ehci_hcd *ehci, struct ehci_qh *qh)
 68 {
 69 /* clean qtds first, and know this is not linked */
 70 if (!list_empty (qh-qtd_list) || qh-qh_next.ptr) {
 71 ehci_dbg (ehci, unused qh not empty!\n);
 72 BUG ();
 73 }
   
and finally a dump of the ehci_qh in question:

crash struct ehci_qh 88084b84dc80
struct ehci_qh {
  hw = 0x880078d1a000, 
   
   It would be good to see the contents of the ehci_qh_hw structure.  That 
   would tell us what device and endpoint this QH was for.
  
  crash struct ehci_qh_hw 0x880078d1a000
  struct ehci_qh_hw {
hw_next = 0x78d1a062, 
hw_info1 = 0x8000, 
 
 No maxpacket value, device address, or endpoint number, but the QH_HEAD
 bit is set.  That happens only with the head of the async ring.  And
 indeed, the QH address agrees with ehci-async = 0x88084b84dc80 in
 your earlier email.
 
hw_info2 = 0x0, 
hw_current = 0x0, 
hw_qtd_next = 0x1, 
hw_alt_next = 0x78d22000, 
hw_token = 0x40, 
hw_buf = {0x0, 0x0, 0x0, 0x0, 0x0}, 
hw_buf_hi = {0x0, 0x0, 0x0, 0x0, 0x0}
  }
  
  qh_dma = 0x78d1a000, 
  qh_next = {
qh = 0x88084efe6730, 
itd = 0x88084efe6730, 
sitd = 0x88084efe6730, 
fstn = 0x88084efe6730, 
hw_next = 0x88084efe6730, 
ptr = 0x88084efe6730  !NULL
  }, 
 
 So there's a leftover qh_next pointer, presumably to a QH that used to
 be on the async list but no longer exists.
 
 This means the list pointers got corrupted somehow.  No way at this
 point to know just how.  You can add some debugging code to check the
 links at the end of qh_link_async (which adds a new QH to the async
 list) and single_unlink_async (which removes a QH from the list).

Hi Alan,

I've collected 16 crashes since kicking off automated tests a little
over 24 hrs ago.

Each crash hit the BUG in qh_destroy() and the only unique debugging
printk from ehci_stop() was: ehci_stop: ehci-num_async = 0.

I can include (or upload) a full (or filtered) vmcore-dmesg.txt if that
would be more helpful.

The debug code I added as you suggested is provided below...

Thanks,

-- Joe


diff -Nupr before/drivers/usb/host/ehci.h after/drivers/usb/host/ehci.h
--- before/drivers/usb/host/ehci.h  2014-07-16 14:25:31.0 -0400
+++ after/drivers/usb/host/ehci.h   2014-09-12 15:47:18.943478397 -0400
@@ -231,6 +231,8 @@ struct ehci_hcd {   /* one per controlle
 
/* platform-specific data -- must come last */
unsigned long   priv[0] __aligned(sizeof(s64));
+
+   int num_async;
 };
 
 /* convert between an HCD pointer and the corresponding EHCI_HCD */
diff -Nupr before/drivers/usb/host/ehci-hcd.c after/drivers/usb/host/ehci-hcd.c
--- before/drivers/usb/host/ehci-hcd.c  2014-07-16 14:25:31.0 -0400
+++ after/drivers/usb/host/ehci-hcd.c   2014-09-12 15:53:16.863163627 -0400
@@ -440,6 +440,8 @@ static void ehci_stop (struct usb_hcd *h
if (ehci-amd_pll_fix == 1)
usb_amd_dev_put();
 
+   pr_err(%s: ehci-num_async = %d\n, __func__, ehci-num_async);
+
dbg_status (ehci, ehci_stop completed,
ehci_readl(ehci, ehci-regs-status));
 }
diff -Nupr before/drivers/usb/host/ehci-q.c after/drivers/usb/host/ehci-q.c
--- before/drivers/usb/host/ehci-q.c2014-07-16 14:25:31.0 -0400
+++ after/drivers/usb/host/ehci-q.c 2014-09-12 15:52:08.023292291 -0400
@@ -959,6 +959,24 @@ static void disable_async(struct ehci_hc
ehci_poll_ASS(ehci);
 }
 
+static void check_async_ring(struct ehci_hcd *ehci, int add)
+{
+   struct ehci_qh *qh;
+   int n;
+
+   qh = ehci-async-qh_next.qh;
+   n = ehci-num_async += add;
+   while (qh  n  0) {
+   qh = qh-qh_next.qh;
+   --n;
+   }
+   if (qh || n != 0) {
+   ehci_err(ehci, EHCI async list corrupted: num %d n %d qh %p\n,
+   ehci-num_async, n, qh);
+   BUG();
+   }
+}
+
 /* move qh (and its qtds) onto async queue; maybe enable queue.  */
 
 static void qh_link_async (struct ehci_hcd *ehci, struct ehci_qh *qh)
@@ -990,6 +1008,8 @@ static void qh_link_async (struct ehci_h
/* qtd completions reported later by interrupt */
 
enable_async(ehci);
+
+   

Re: [Patch v9 3/3] phy: Add Qualcomm DWC3 HS/SS PHY driver

[Felipe Balbi]

Hi,

On Sat, Sep 13, 2014 at 12:16:01PM +0530, Kishon Vijay Abraham I wrote:
 On Saturday 13 September 2014 12:58 AM, Andy Gross wrote:
  This patch adds a new driver for the Qualcomm USB 3.0 PHY that exists on 
  some
  Qualcomm platforms.  This driver uses the generic PHY framework and will
  interact with the DWC3 controller.
 
 Do you have dt documentation for this driver?

see patch 1

  +static inline void qcom_dwc3_phy_write_readback(
  +   struct qcom_dwc3_usb_phy *phy_dwc3, u32 offset,
  +   const u32 mask, u32 val)
  +{
  +   u32 write_val, tmp = readl(phy_dwc3-base + offset);
  +
  +   tmp = ~mask;   /* retain other bits */
  +   write_val = tmp | val;
  +
  +   writel(write_val, phy_dwc3-base + offset);
  +
  +   /* Read back to see if val was written */
 
 Does it fail sometime? I'm not sure if this should be present in the
 driver since this looks more of a debug code.

this was mentioned before. Silicon bug.

  +   writel_relaxed(data | SSUSB_CTRL_SS_PHY_RESET,
  +   phy_dwc3-base + SSUSB_PHY_CTRL_REG);
  +   usleep_range(2000, 2200);
 
 use msleep here..

why ? usleep_range() gives the scheduler oportunity to group timers.

-- 
balbi


signature.asc
Description: Digital signature