Re: [PATCH v2 4/6] usb: usbip tool: Fix refresh_imported_device_list()

[Yuyang Du]

Hi Khan,

On Tue, Apr 04, 2017 at 11:14:33AM -0600, Shuah Khan wrote:
> On 03/30/2017 06:28 PM, Yuyang Du wrote:
> > The commit 0775a9cbc694e8c7 ("usbip: vhci extension: modifications
> > to vhci driver") introduced multiple controllers, but the status
> > of the ports are only extracted from the first status file, fix it.
> 
> Are sure this change is needed? What bug are you fixing? This change
> will impact every single code path that calls usbip_vhci_driver_open()
> 
> It might be sufficient to look at just the first file status in many of
> these cases?

Yes, I'm sure. The 1st status file only has the 1st controller's ports,
but we must have the status of all the ports of all the controllers.

> > 
> > Signed-off-by: Yuyang Du 
> > ---
> >  tools/usb/usbip/libsrc/vhci_driver.c | 27 +--
> >  1 file changed, 21 insertions(+), 6 deletions(-)
> > 
> > diff --git a/tools/usb/usbip/libsrc/vhci_driver.c 
> > b/tools/usb/usbip/libsrc/vhci_driver.c
> > index ccecd47..630b139 100644
> > --- a/tools/usb/usbip/libsrc/vhci_driver.c
> > +++ b/tools/usb/usbip/libsrc/vhci_driver.c
> > @@ -108,18 +108,33 @@ static int parse_status(const char *value)
> > return 0;
> >  }
> >  
> > +#define MAX_STATUS_NAME 16
> > +
> >  static int refresh_imported_device_list(void)
> >  {
> > const char *attr_status;
> > +   char status[MAX_STATUS_NAME+1] = "status";
> > +   int i, ret;
> >  
> > -   attr_status = udev_device_get_sysattr_value(vhci_driver->hc_device,
> > -  "status");
> > -   if (!attr_status) {
> > -   err("udev_device_get_sysattr_value failed");
> > -   return -1;
> > +   for (i = 0; i < vhci_driver->ncontrollers; i++) {
> > +   if (i > 0)
> > +   snprintf(status, sizeof(status), "status.%d", i);
> > +
> > +   attr_status = 
> > udev_device_get_sysattr_value(vhci_driver->hc_device,
> > +   status);
> > +   if (!attr_status) {
> > +   err("udev_device_get_sysattr_value failed");
> > +   return -1;
> > +   }
> > +
> > +   dbg("controller %d", i);
> > +
> > +   ret = parse_status(attr_status);
> > +   if (ret != 0)
> > +   return ret;
> 
> usbip_vhci_driver_open() will fail even if one of these fails? Is that
> what you would want?

Ditto.

Thanks,
Yuyang
--
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 v2 3/6] usb: usbip tool: Add ncontrollers in vhci_driver structure

[Yuyang Du]

Hi Krzysztof,

On Tue, Apr 04, 2017 at 04:52:32PM +0200, Krzysztof Opasiak wrote:
> 
> 
> On 03/31/2017 02:28 AM, Yuyang Du wrote:
> >A new field ncontrollers is added to the vhci_driver structure.
> >And this field is stored by scanning the vhci_hcd* dirs in the
> >platform udev.
> >
> >Suggested-by: Krzysztof Opasiak 
> >Signed-off-by: Yuyang Du 
> >---
> > tools/usb/usbip/libsrc/vhci_driver.c | 32 +++-
> > tools/usb/usbip/libsrc/vhci_driver.h |  1 +
> > 2 files changed, 32 insertions(+), 1 deletion(-)
> >
> >diff --git a/tools/usb/usbip/libsrc/vhci_driver.c 
> >b/tools/usb/usbip/libsrc/vhci_driver.c
> >index f659c14..ccecd47 100644
> >--- a/tools/usb/usbip/libsrc/vhci_driver.c
> >+++ b/tools/usb/usbip/libsrc/vhci_driver.c
> >@@ -7,6 +7,7 @@
> > #include 
> > #include 
> > #include 
> >+#include 
> > #include "sysfs_utils.h"
> >
> > #undef  PROGNAME
> >@@ -134,6 +135,33 @@ static int get_nports(void)
> > return (int)strtoul(attr_nports, NULL, 10);
> > }
> >
> >+static int vhci_hcd_filter(const struct dirent *dirent)
> >+{
> >+return strcmp(dirent->d_name, "vhci_hcd") >= 0 ? 1: 0;
> 
> The ? operator may be omitted here as according to doc we need to
> return nonzero not 1 exactly.

No, it can't. strcmp() would return negative if not containing "vhci_hcd". E.g.,

strcmp("!@#", "vhci_hcd") ==> -1
strcmp("v", "vhci_hcd") ==> -1

> >+
> >+static int get_ncontrollers(void)
> >+{
> >+struct dirent **namelist;
> >+struct udev_device *platform;
> >+int n;
> >+
> >+platform = udev_device_get_parent(vhci_driver->hc_device);
> >+if (platform == NULL)
> >+return -1;
> >+
> >+n = scandir(udev_device_get_syspath(platform), , 
> >vhci_hcd_filter, NULL);
> >+if (n < 0)
> >+err("scandir failed");
> >+else {
> >+for (int i = 0; i < n; i++)
> >+free(namelist[i]);
> >+free(namelist);
> >+}
> >+
> >+return n;
> >+}
> >+
> > /*
> >  * Read the given port's record.
> >  *
> >@@ -220,9 +248,11 @@ int usbip_vhci_driver_open(void)
> > }
> >
> > vhci_driver->nports = get_nports();
> >-
> 
> Seems to be unrelated.
> 
> > dbg("available ports: %d", vhci_driver->nports);
> >
> >+vhci_driver->ncontrollers = get_ncontrollers();
> 
> shouldn't we check here if we didn't got error from scandir()?

If scandir() failed, we should have (only) printed errors. But yes, both
get_nports() and get_ncontrollers() should check and fail gracefully.

Thanks,
Yuyang
--
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 net-next] net: usbnet: Remove unused driver_name variable

[Florian Fainelli]

With GCC 6.3, we can get the following warning:

drivers/net/usb/usbnet.c:85:19: warning: 'driver_name' defined but not
used [-Wunused-const-variable=]
 static const char driver_name [] = "usbnet";
   ^~~

Signed-off-by: Florian Fainelli 
---
 drivers/net/usb/usbnet.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
index 9890656af735..1cc945cbeaa3 100644
--- a/drivers/net/usb/usbnet.c
+++ b/drivers/net/usb/usbnet.c
@@ -82,8 +82,6 @@
 // randomly generated ethernet address
 static u8  node_id [ETH_ALEN];
 
-static const char driver_name [] = "usbnet";
-
 /* use ethtool to change the level for any given device */
 static int msg_level = -1;
 module_param (msg_level, int, 0);
-- 
2.9.3

--
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: usb: musb: regression since 4.9 on omap4-panda-es (caused by d8e5f0eca1e8)

[Tony Lindgren]

* Tony Lindgren  [170404 07:06]:
> * Bin Liu  [170404 05:30]:
> > On Tue, Apr 04, 2017 at 10:09:50AM +0300, Peter Ujfalusi wrote:
> > > Tony,
> > > 
> > > since 4.9 (4.8 was fine) I can not boot omap4-panda-es if the musb
> > > is compiled in. The kernel will stuck printing:
> > > 
> > > ** 206 printk messages dropped ** [8.926727] musb_bus_suspend
> > > 2584: trying to suspend as a_idle while active
> 
> OK so compiled in. Do you have something connected also when
> booting?
> 
> > Does it sound a similar issue to
> > http://marc.info/?l=linux-usb=149036531809506=2
> 
> Yup.
> 
> > > The bisect (log is [1]) points to:
> > > d8e5f0eca1e8 usb: musb: Fix hardirq-safe hardirq-unsafe lock order error
> > > 
> > > and reverting the d8e5f0eca1e8 makes the board to boot up fine
> > > (Linux 4.11-rc5 and next-20170331).
> 
> OK thanks for bisecting it.
> 
> > > any idea on how to fix this w/o reverting the commit?
> 
> I'll take a look.

OK I was able to reproduce this with loadable modules by reloading
the modules while having OTG-A cable inserted with a hub and USB
drive connected.

Peter, care to check if the following fixes the problem for you?
There should no longer be much any musb core tinkering happening
in the glue layers..

Regards,

Tony

8< ---
diff --git a/drivers/usb/musb/omap2430.c b/drivers/usb/musb/omap2430.c
--- a/drivers/usb/musb/omap2430.c
+++ b/drivers/usb/musb/omap2430.c
@@ -91,12 +91,6 @@ static void omap2430_musb_set_vbus(struct musb *musb, int 
is_on)
}
 
otg_set_vbus(otg, 1);
-   } else {
-   musb->is_active = 1;
-   otg->default_a = 1;
-   musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
-   devctl |= MUSB_DEVCTL_SESSION;
-   MUSB_HST_MODE(musb);
}
} else {
musb->is_active = 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: dwc2_hc_chhltd_intr_dma - ChHltd set errors?

[John Stultz]

On Tue, Apr 4, 2017 at 12:38 AM, Felipe Balbi
 wrote:
>
> Hi,
>
> Minas Harutyunyan  writes:
>>>   We've noticed that when using usb ethernet adapters on HiKey, we
>>> occasionally see errors like:
>>>
>>> dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 0 - ChHltd set,
>>> but reason is unknown
>>> dwc2 f72c.usb: hcint 0x0002, intsts 0x06200029
>>>
>>> dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 11 - ChHltd set,
>>> but reason is unknown
>>> dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
>>>
>>> Sometimes followed up by a usb error in the driver, something like:
>>> asix 1-1.2:1.0 eth0: asix_rx_fixup() Bad Header Length 0x36000807, 
>>> offset 68
>>>
>>> Curious if you've seen any reports like this?

 The core version is less than 2.71a, am I right?
>>>
>>> So it looks like its reporting 0x4f54300a for hsotg->regs + GSNPSID
>>> which looks like DWC2_CORE_REV_3_00a
>>>
 Please send full debug log to do more investigation.
>>>
>>> Full dmesg, or is there special debugging you want me to enable?
>>
>> Full dmesg around issue.
>>>
 Also send us regdump after connecting ethernet adapter.
>>>
>>> Sorry, can you clarify how to generate this?
>>
>> cat regdump. To locate dwc2 regdump file: cd /; find -name regdump
>
> this won't work if his distro doesn't mount debugfs. Please give
> complete instructions ;-)
>
> # mkdir -p /d
> # mount -t debugfs none /d
> # cd /d
> # find . -name regdump
>
> The directory name is the same name as the dwc2 device name, AFAICT. So,
> check your DTS for the name of the device.

Thanks for the extra details! I didn't have DEBUG_FS built in, so this
helped clue me in.

Attached are dmesg including the issue and the regdump.

I did notice when cating the regdump file, I saw:
dwc2 f72c.usb: Mode Mismatch Interrupt: currently in Host mode
twice. (You'll see it 4 times in the dmesg around 1077 as I cat'ed
regdump again to verify it wasn't just chance).

Let me know if there is anything else you need!

thanks
-john


hikey-usb.dmesg
Description: Binary data


hikey-usb.regdump
Description: Binary data

Re: [PATCH v6 1/4] dt-bindings: phy: Add support for QUSB2 phy

[Stephen Boyd]

On 03/20, Vivek Gautam wrote:
> diff --git a/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt 
> b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
> new file mode 100644
> index ..a6d19acde9e0
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/phy/qcom-qusb2-phy.txt
> @@ -0,0 +1,45 @@
> +Qualcomm QUSB2 phy controller
> +=
> +
> +QUSB2 controller supports LS/FS/HS usb connectivity on Qualcomm chipsets.
> +
> +Required properties:
> + - compatible: compatible list, contains "qcom,msm8996-qusb2-phy".
> + - reg: offset and length of the PHY register set.
> + - #phy-cells: must be 0.
> +
> + - clocks: a list of phandles and clock-specifier pairs,
> +one for each entry in clock-names.
> + - clock-names: must be "cfg_ahb" for phy config clock,
> + "ref" for 19.2 MHz ref clk,
> + "iface" for phy interface clock (Optional).
> +
> + - vdd-phy-supply: Phandle to a regulator supply to PHY core block.
> + - vdda-pll-supply: Phandle to 1.8V regulator supply to PHY refclk pll block.
> + - vdda-phy-dpdm-supply: Phandle to 3.1V regulator supply to Dp/Dm port 
> signals.
> +
> + - resets: Phandle to reset to phy block.
> +
> +Optional properties:
> + - nvmem-cells: Phandle to nvmem cell that contains 'HS Tx trim'
> + tuning parameter value for qusb2 phy.
> +
> + - qcom,tcsr-syscon: Phandle to TCSR syscon register region.
> +
> +Example:
> + hsusb_phy: phy@7411000 {
> + compatible = "qcom,msm8996-qusb2-phy";
> + reg = <0x7411000 0x180>;
> + #phy-cells = <0>;
> +
> + clocks = < GCC_USB_PHY_CFG_AHB2PHY_CLK>,
> + < GCC_RX1_USB2_CLKREF_CLK>,
> + clock-names = "cfg_ahb", "ref";
> +
> + vdd-phy-supply = <_s2>;

pm8994_s2 is a "corner" regulator. I'm not sure how we're going
to have it listed here as something like a regulator supply in
the binding. We probably should leave it out for now and let the
power domain + pm_qos stuff for corners work out. From what I see
in the downstream driver the code is setting the corner to '4'
when active, and '1' when inactive.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
--
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 v2 6/6] usb: usbip tool: Remove empty lines

[Shuah Khan]

On 03/30/2017 06:28 PM, Yuyang Du wrote:
> Signed-off-by: Yuyang Du 
> ---
>  tools/usb/usbip/libsrc/vhci_driver.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/tools/usb/usbip/libsrc/vhci_driver.c 
> b/tools/usb/usbip/libsrc/vhci_driver.c
> index f596ef4..d34a482 100644
> --- a/tools/usb/usbip/libsrc/vhci_driver.c
> +++ b/tools/usb/usbip/libsrc/vhci_driver.c
> @@ -36,8 +36,6 @@ imported_device_init(struct usbip_imported_device *idev, 
> char *busid)
>   return NULL;
>  }
>  
> -
> -
>  static int parse_status(const char *value)
>  {
>   int ret = 0;
> 


No change log - also it could be combined with a prior patch.

-- Shuah
--
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 v2 5/6] usb: usbip tool: Fix parse_status()

[Shuah Khan]

On 03/30/2017 06:28 PM, Yuyang Du wrote:
> parse_status() reads the status file one by one, so it can only
> update the available and according vhci_driver->idev's.

What are you fixing? This change log doesn't me what is the bug and
what is being fixed.

thanks,
-- Shuah

> 
> Signed-off-by: Yuyang Du 
> ---
>  tools/usb/usbip/libsrc/vhci_driver.c | 36 
> +++-
>  tools/usb/usbip/src/usbip_attach.c   |  2 ++
>  2 files changed, 17 insertions(+), 21 deletions(-)
> 
> diff --git a/tools/usb/usbip/libsrc/vhci_driver.c 
> b/tools/usb/usbip/libsrc/vhci_driver.c
> index 630b139..f596ef4 100644
> --- a/tools/usb/usbip/libsrc/vhci_driver.c
> +++ b/tools/usb/usbip/libsrc/vhci_driver.c
> @@ -43,11 +43,6 @@ static int parse_status(const char *value)
>   int ret = 0;
>   char *c;
>  
> -
> - for (int i = 0; i < vhci_driver->nports; i++)
> - memset(_driver->idev[i], 0, sizeof(vhci_driver->idev[i]));
> -
> -
>   /* skip a header line */
>   c = strchr(value, '\n');
>   if (!c)
> @@ -58,6 +53,7 @@ static int parse_status(const char *value)
>   int port, status, speed, devid;
>   unsigned long socket;
>   char lbusid[SYSFS_BUS_ID_SIZE];
> + struct usbip_imported_device *idev;
>  
>   ret = sscanf(c, "%d %d %d %x %lx %31s\n",
>   , , ,
> @@ -72,30 +68,28 @@ static int parse_status(const char *value)
>   port, status, speed, devid);
>   dbg("socket %lx lbusid %s", socket, lbusid);
>  
> -
>   /* if a device is connected, look at it */
> - {
> - struct usbip_imported_device *idev = 
> _driver->idev[port];
> + idev = _driver->idev[port];
> +
> + memset(idev, 0, sizeof(*idev));
>  
> - idev->port  = port;
> - idev->status= status;
> + idev->port  = port;
> + idev->status= status;
>  
> - idev->devid = devid;
> + idev->devid = devid;
>  
> - idev->busnum= (devid >> 16);
> - idev->devnum= (devid & 0x);
> + idev->busnum= (devid >> 16);
> + idev->devnum= (devid & 0x);
>  
> - if (idev->status != VDEV_ST_NULL
> - && idev->status != VDEV_ST_NOTASSIGNED) {
> - idev = imported_device_init(idev, lbusid);
> - if (!idev) {
> - dbg("imported_device_init failed");
> - return -1;
> - }
> + if (idev->status != VDEV_ST_NULL
> + && idev->status != VDEV_ST_NOTASSIGNED) {
> + idev = imported_device_init(idev, lbusid);
> + if (!idev) {
> + dbg("imported_device_init failed");
> + return -1;
>   }
>   }
>  
> -
>   /* go to the next line */
>   c = strchr(c, '\n');
>   if (!c)
> diff --git a/tools/usb/usbip/src/usbip_attach.c 
> b/tools/usb/usbip/src/usbip_attach.c
> index 70a6b50..62a297f 100644
> --- a/tools/usb/usbip/src/usbip_attach.c
> +++ b/tools/usb/usbip/src/usbip_attach.c
> @@ -108,6 +108,8 @@ static int import_device(int sockfd, struct 
> usbip_usb_device *udev)
>   return -1;
>   }
>  
> + dbg("got free port %d", port);
> +
>   rc = usbip_vhci_attach_device(port, sockfd, udev->busnum,
> udev->devnum, udev->speed);
>   if (rc < 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 v2 4/6] usb: usbip tool: Fix refresh_imported_device_list()

[Shuah Khan]

On 03/30/2017 06:28 PM, Yuyang Du wrote:
> The commit 0775a9cbc694e8c7 ("usbip: vhci extension: modifications
> to vhci driver") introduced multiple controllers, but the status
> of the ports are only extracted from the first status file, fix it.

Are sure this change is needed? What bug are you fixing? This change
will impact every single code path that calls usbip_vhci_driver_open()

It might be sufficient to look at just the first file status in many of
these cases?

> 
> Signed-off-by: Yuyang Du 
> ---
>  tools/usb/usbip/libsrc/vhci_driver.c | 27 +--
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/usb/usbip/libsrc/vhci_driver.c 
> b/tools/usb/usbip/libsrc/vhci_driver.c
> index ccecd47..630b139 100644
> --- a/tools/usb/usbip/libsrc/vhci_driver.c
> +++ b/tools/usb/usbip/libsrc/vhci_driver.c
> @@ -108,18 +108,33 @@ static int parse_status(const char *value)
>   return 0;
>  }
>  
> +#define MAX_STATUS_NAME 16
> +
>  static int refresh_imported_device_list(void)
>  {
>   const char *attr_status;
> + char status[MAX_STATUS_NAME+1] = "status";
> + int i, ret;
>  
> - attr_status = udev_device_get_sysattr_value(vhci_driver->hc_device,
> -"status");
> - if (!attr_status) {
> - err("udev_device_get_sysattr_value failed");
> - return -1;
> + for (i = 0; i < vhci_driver->ncontrollers; i++) {
> + if (i > 0)
> + snprintf(status, sizeof(status), "status.%d", i);
> +
> + attr_status = 
> udev_device_get_sysattr_value(vhci_driver->hc_device,
> + status);
> + if (!attr_status) {
> + err("udev_device_get_sysattr_value failed");
> + return -1;
> + }
> +
> + dbg("controller %d", i);
> +
> + ret = parse_status(attr_status);
> + if (ret != 0)
> + return ret;

usbip_vhci_driver_open() will fail even if one of these fails? Is that
what you would want?

thanks,
-- Shuah

>   }
>  
> - return parse_status(attr_status);
> + return 0;
>  }
>  
>  static int get_nports(void)
> 

--
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] usb: dwc3: gadget: skip Set/Clear Halt when invalid

[John Youn]

On 04/04/2017 01:09 AM, Felipe Balbi wrote:
>
> Hi John,
>
> Felipe Balbi  writes:
>> At least macOS seems to be sending
>> ClearFeature(ENDPOINT_HALT) to endpoints which
>> aren't Halted. This makes DWC3's CLEARSTALL command
>> time out which causes several issues for the driver.
>>
>> Instead, let's just return 0 and bail out early.
>>
>> Cc: 
>> Signed-off-by: Felipe Balbi 
>> ---
>>
>> this falls into "has never worked before" category, so I'll be sending
>> it together with other patches for v4.11 merge window. Still, it's a
>> valid bug that's likely needed for stable trees.
>>
>>  drivers/usb/dwc3/gadget.c | 5 +
>>  1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 6faf484e5dfc..0a664d8eba3f 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -1379,6 +1379,9 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int 
>> value, int protocol)
>>  unsigned transfer_in_flight;
>>  unsigned started;
>>
>> +if (dep->flags & DWC3_EP_STALL)
>> +return 0;
>> +
>>  if (dep->number > 1)
>>  trb = dwc3_ep_prev_trb(dep, dep->trb_enqueue);
>>  else
>> @@ -1400,6 +1403,8 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int 
>> value, int protocol)
>>  else
>>  dep->flags |= DWC3_EP_STALL;
>>  } else {
>> +if (!(dep->flags & DWC3_EP_STALL))
>> +return 0;
>>
>>  ret = dwc3_send_clear_stall_ep_cmd(dep);
>>  if (ret)
>
> Reviving this old thread here. While $subject allowed dwc3 to work when
> attached to macOS Host, I fear that we might have more issues than not
> in the future. The reason is that USB20 spec allows hosts to use
> ClearFeature(ENDPOINT_HALT) as a "Reset Data Toggle/SeqN" hint.
>
> With this, we're basically blocking that possibility. Still, without
> $subject, ClearStall commands were timing out. I'll try to do a local
> revert here and check what happens in this case, but would you have any
> idea why ClearStall would time out like that?
>

Hi Felipe,

I think Thinh Nguyen e-mailed you about this before saying it caused a
regression for us. But he has not had time to look into it further and
follow-up yet.

No idea about the timing out on Mac though. We can try to reproduce
this when we have some time and take a look. Do you have a USB trace?

Regards,
John
--
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 v2 2/6] usb: usbip tool: Fix get_nports()

[Shuah Khan]

On 03/30/2017 06:28 PM, Yuyang Du wrote:
> The commit 0775a9cbc694e8c72 ("usbip: vhci extension: modifications
> to vhci driver") introduced multiple controllers, and nports as a sys
> file, and claimed to read the nports from it, but it didn't.
> 
> In addition, the get_nports() has been so wrong that even with 8 port
> lines for instance, it gets 7 (I am guessing it is due to a '\n' mess).
> Nevertheless, we fix it by reading the nports attribute.
> 
> Reviewed-by: Krzysztof Opasiak 

Thanks for the review.

> Signed-off-by: Yuyang Du 

Greg,

Please pick this up:

Acked-by: Shuah Khan 

thanks,
-- Shuah

> ---
>  tools/usb/usbip/libsrc/vhci_driver.c | 28 +---
>  1 file changed, 5 insertions(+), 23 deletions(-)
> 
> diff --git a/tools/usb/usbip/libsrc/vhci_driver.c 
> b/tools/usb/usbip/libsrc/vhci_driver.c
> index ad92047..f659c14 100644
> --- a/tools/usb/usbip/libsrc/vhci_driver.c
> +++ b/tools/usb/usbip/libsrc/vhci_driver.c
> @@ -123,33 +123,15 @@ static int refresh_imported_device_list(void)
>  
>  static int get_nports(void)
>  {
> - char *c;
> - int nports = 0;
> - const char *attr_status;
> + const char *attr_nports;
>  
> - attr_status = udev_device_get_sysattr_value(vhci_driver->hc_device,
> -"status");
> - if (!attr_status) {
> - err("udev_device_get_sysattr_value failed");
> + attr_nports = udev_device_get_sysattr_value(vhci_driver->hc_device, 
> "nports");
> + if (!attr_nports) {
> + err("udev_device_get_sysattr_value nports failed");
>   return -1;
>   }
>  
> - /* skip a header line */
> - c = strchr(attr_status, '\n');
> - if (!c)
> - return 0;
> - c++;
> -
> - while (*c != '\0') {
> - /* go to the next line */
> - c = strchr(c, '\n');
> - if (!c)
> - return nports;
> - c++;
> - nports += 1;
> - }
> -
> - return nports;
> + return (int)strtoul(attr_nports, NULL, 10);
>  }
>  
>  /*
> 

--
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 v2 6/6] usb: usbip tool: Remove empty lines

[Krzysztof Opasiak]

On 03/31/2017 02:28 AM, Yuyang Du wrote:

Signed-off-by: Yuyang Du 
---


Greg is probably not going to accept a commit without commit message so 
please fix this.


At least for me it looks like this commit could be squashed with the 
previous one as they are both fixing parse_status()


Cheers,
--
Krzysztof Opasiak
Samsung R Institute Poland
Samsung Electronics
--
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 v2 5/6] usb: usbip tool: Fix parse_status()

[Krzysztof Opasiak]

On 03/31/2017 02:28 AM, Yuyang Du wrote:

parse_status() reads the status file one by one, so it can only
update the available and according vhci_driver->idev's.

Signed-off-by: Yuyang Du 
---
 tools/usb/usbip/libsrc/vhci_driver.c | 36 +++-
 tools/usb/usbip/src/usbip_attach.c   |  2 ++
 2 files changed, 17 insertions(+), 21 deletions(-)

diff --git a/tools/usb/usbip/libsrc/vhci_driver.c 
b/tools/usb/usbip/libsrc/vhci_driver.c
index 630b139..f596ef4 100644
--- a/tools/usb/usbip/libsrc/vhci_driver.c
+++ b/tools/usb/usbip/libsrc/vhci_driver.c
@@ -43,11 +43,6 @@ static int parse_status(const char *value)
int ret = 0;
char *c;

-
-   for (int i = 0; i < vhci_driver->nports; i++)
-   memset(_driver->idev[i], 0, sizeof(vhci_driver->idev[i]));
-
-
/* skip a header line */
c = strchr(value, '\n');
if (!c)
@@ -58,6 +53,7 @@ static int parse_status(const char *value)
int port, status, speed, devid;
unsigned long socket;
char lbusid[SYSFS_BUS_ID_SIZE];
+   struct usbip_imported_device *idev;

ret = sscanf(c, "%d %d %d %x %lx %31s\n",
, , ,
@@ -72,30 +68,28 @@ static int parse_status(const char *value)
port, status, speed, devid);
dbg("socket %lx lbusid %s", socket, lbusid);

-
/* if a device is connected, look at it */
-   {
-   struct usbip_imported_device *idev = 
_driver->idev[port];
+   idev = _driver->idev[port];
+
+   memset(idev, 0, sizeof(*idev));

-   idev->port   = port;
-   idev->status = status;
+   idev->port   = port;
+   idev->status = status;

-   idev->devid  = devid;
+   idev->devid  = devid;

-   idev->busnum = (devid >> 16);
-   idev->devnum = (devid & 0x);
+   idev->busnum = (devid >> 16);
+   idev->devnum = (devid & 0x);

-   if (idev->status != VDEV_ST_NULL
-   && idev->status != VDEV_ST_NOTASSIGNED) {
-   idev = imported_device_init(idev, lbusid);
-   if (!idev) {
-   dbg("imported_device_init failed");
-   return -1;
-   }
+   if (idev->status != VDEV_ST_NULL
+   && idev->status != VDEV_ST_NOTASSIGNED) {
+   idev = imported_device_init(idev, lbusid);
+   if (!idev) {
+   dbg("imported_device_init failed");
+   return -1;
}
}

-
/* go to the next line */
c = strchr(c, '\n');
if (!c)
diff --git a/tools/usb/usbip/src/usbip_attach.c 
b/tools/usb/usbip/src/usbip_attach.c
index 70a6b50..62a297f 100644
--- a/tools/usb/usbip/src/usbip_attach.c
+++ b/tools/usb/usbip/src/usbip_attach.c
@@ -108,6 +108,8 @@ static int import_device(int sockfd, struct 
usbip_usb_device *udev)
return -1;
}

+   dbg("got free port %d", port);
+
rc = usbip_vhci_attach_device(port, sockfd, udev->busnum,


Apart from this unrelated change above:

Reviewed-by: Krzysztof Opasiak 

Cheers,
--
Krzysztof Opasiak
Samsung R Institute Poland
Samsung Electronics
--
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 v2 1/6] usb: usbip: Remove unnecessary get_vdev()

[Shuah Khan]

On 04/04/2017 08:43 AM, Krzysztof Opasiak wrote:
> 
> 
> On 03/31/2017 02:28 AM, Yuyang Du wrote:
>> vhci_tx_urb() should be able to get the vhci_device from
>> its caller vhci_urb_enqueue(), instead of brutal-force
>> searching it.
>>
>> Signed-off-by: Yuyang Du 
>> ---
>>  drivers/usb/usbip/vhci_hcd.c | 32 ++--
>>  1 file changed, 2 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
>> index e4cb9f0..5d8b2c2 100644
>> --- a/drivers/usb/usbip/vhci_hcd.c
>> +++ b/drivers/usb/usbip/vhci_hcd.c
>> @@ -430,36 +430,8 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 
>> typeReq, u16 wValue,
>>  return retval;
>>  }
>>
>> -static struct vhci_device *get_vdev(struct usb_device *udev)
>> +static void vhci_tx_urb(struct urb *urb, struct vhci_device *vdev)
>>  {
>> -struct platform_device *pdev;
>> -struct usb_hcd *hcd;
>> -struct vhci_hcd *vhci;
>> -int pdev_nr, rhport;
>> -
>> -if (!udev)
>> -return NULL;
>> -
>> -for (pdev_nr = 0; pdev_nr < vhci_num_controllers; pdev_nr++) {
>> -pdev = *(vhci_pdevs + pdev_nr);
>> -if (pdev == NULL)
>> -continue;
>> -hcd = platform_get_drvdata(pdev);
>> -if (hcd == NULL)
>> -continue;
>> -vhci = hcd_to_vhci(hcd);
>> -for (rhport = 0; rhport < VHCI_HC_PORTS; rhport++) {
>> -if (vhci->vdev[rhport].udev == udev)
>> -return >vdev[rhport];
>> -}
>> -}
>> -
>> -return NULL;
>> -}
>> -
>> -static void vhci_tx_urb(struct urb *urb)
>> -{
>> -struct vhci_device *vdev = get_vdev(urb->dev);
>>  struct vhci_priv *priv;
>>  struct vhci_hcd *vhci;
>>  unsigned long flags;
>> @@ -601,7 +573,7 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct 
>> urb *urb,
>>  }
>>
>>  out:
>> -vhci_tx_urb(urb);
>> +vhci_tx_urb(urb, vdev);
>>  spin_unlock_irqrestore(>lock, flags);
>>
>>  return 0;
>>
> 
> I think that I've already gave you my reviewed by for this patch.
> Nevertheless:
> 
> Reviewed-by: Krzysztof Opasiak 
> 

Thanks.

Here is my Ack for Greg to pick this up:

Acked-by: Shuah Khan 

thanks,
-- Shuah
--
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 v2 4/6] usb: usbip tool: Fix refresh_imported_device_list()

[Krzysztof Opasiak]

On 03/31/2017 02:28 AM, Yuyang Du wrote:

The commit 0775a9cbc694e8c7 ("usbip: vhci extension: modifications
to vhci driver") introduced multiple controllers, but the status
of the ports are only extracted from the first status file, fix it.

Signed-off-by: Yuyang Du 
---
 tools/usb/usbip/libsrc/vhci_driver.c | 27 +--
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/tools/usb/usbip/libsrc/vhci_driver.c 
b/tools/usb/usbip/libsrc/vhci_driver.c
index ccecd47..630b139 100644
--- a/tools/usb/usbip/libsrc/vhci_driver.c
+++ b/tools/usb/usbip/libsrc/vhci_driver.c
@@ -108,18 +108,33 @@ static int parse_status(const char *value)
return 0;
 }

+#define MAX_STATUS_NAME 16
+
 static int refresh_imported_device_list(void)
 {
const char *attr_status;
+   char status[MAX_STATUS_NAME+1] = "status";
+   int i, ret;

-   attr_status = udev_device_get_sysattr_value(vhci_driver->hc_device,
-  "status");
-   if (!attr_status) {
-   err("udev_device_get_sysattr_value failed");
-   return -1;
+   for (i = 0; i < vhci_driver->ncontrollers; i++) {
+   if (i > 0)
+   snprintf(status, sizeof(status), "status.%d", i);
+
+   attr_status = 
udev_device_get_sysattr_value(vhci_driver->hc_device,
+   status);
+   if (!attr_status) {
+   err("udev_device_get_sysattr_value failed");
+   return -1;
+   }
+
+   dbg("controller %d", i);
+
+   ret = parse_status(attr_status);
+   if (ret != 0)
+   return ret;
}

-   return parse_status(attr_status);
+   return 0;
 }


I decided to not urge here with OO programming introduction so just 
ignore my previous comment and add:


Reviewed-by: Krzysztof Opasiak 

Cheers,
--
Krzysztof Opasiak
Samsung R Institute Poland
Samsung Electronics
--
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 v2 3/6] usb: usbip tool: Add ncontrollers in vhci_driver structure

[Krzysztof Opasiak]

On 03/31/2017 02:28 AM, Yuyang Du wrote:

A new field ncontrollers is added to the vhci_driver structure.
And this field is stored by scanning the vhci_hcd* dirs in the
platform udev.

Suggested-by: Krzysztof Opasiak 
Signed-off-by: Yuyang Du 
---
 tools/usb/usbip/libsrc/vhci_driver.c | 32 +++-
 tools/usb/usbip/libsrc/vhci_driver.h |  1 +
 2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/tools/usb/usbip/libsrc/vhci_driver.c 
b/tools/usb/usbip/libsrc/vhci_driver.c
index f659c14..ccecd47 100644
--- a/tools/usb/usbip/libsrc/vhci_driver.c
+++ b/tools/usb/usbip/libsrc/vhci_driver.c
@@ -7,6 +7,7 @@
 #include 
 #include 
 #include 
+#include 
 #include "sysfs_utils.h"

 #undef  PROGNAME
@@ -134,6 +135,33 @@ static int get_nports(void)
return (int)strtoul(attr_nports, NULL, 10);
 }

+static int vhci_hcd_filter(const struct dirent *dirent)
+{
+   return strcmp(dirent->d_name, "vhci_hcd") >= 0 ? 1: 0;


The ? operator may be omitted here as according to doc we need to return 
nonzero not 1 exactly.



+}
+
+static int get_ncontrollers(void)
+{
+   struct dirent **namelist;
+   struct udev_device *platform;
+   int n;
+
+   platform = udev_device_get_parent(vhci_driver->hc_device);
+   if (platform == NULL)
+   return -1;
+
+   n = scandir(udev_device_get_syspath(platform), , 
vhci_hcd_filter, NULL);
+   if (n < 0)
+   err("scandir failed");
+   else {
+   for (int i = 0; i < n; i++)
+   free(namelist[i]);
+   free(namelist);
+   }
+
+   return n;
+}
+
 /*
  * Read the given port's record.
  *
@@ -220,9 +248,11 @@ int usbip_vhci_driver_open(void)
}

vhci_driver->nports = get_nports();
-


Seems to be unrelated.


dbg("available ports: %d", vhci_driver->nports);

+   vhci_driver->ncontrollers = get_ncontrollers();


shouldn't we check here if we didn't got error from scandir()?

Cheers,
--
Krzysztof Opasiak
Samsung R Institute Poland
Samsung Electronics
--
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 v2 1/6] usb: usbip: Remove unnecessary get_vdev()

[Krzysztof Opasiak]

On 03/31/2017 02:28 AM, Yuyang Du wrote:

vhci_tx_urb() should be able to get the vhci_device from
its caller vhci_urb_enqueue(), instead of brutal-force
searching it.

Signed-off-by: Yuyang Du 
---
 drivers/usb/usbip/vhci_hcd.c | 32 ++--
 1 file changed, 2 insertions(+), 30 deletions(-)

diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
index e4cb9f0..5d8b2c2 100644
--- a/drivers/usb/usbip/vhci_hcd.c
+++ b/drivers/usb/usbip/vhci_hcd.c
@@ -430,36 +430,8 @@ static int vhci_hub_control(struct usb_hcd *hcd, u16 
typeReq, u16 wValue,
return retval;
 }

-static struct vhci_device *get_vdev(struct usb_device *udev)
+static void vhci_tx_urb(struct urb *urb, struct vhci_device *vdev)
 {
-   struct platform_device *pdev;
-   struct usb_hcd *hcd;
-   struct vhci_hcd *vhci;
-   int pdev_nr, rhport;
-
-   if (!udev)
-   return NULL;
-
-   for (pdev_nr = 0; pdev_nr < vhci_num_controllers; pdev_nr++) {
-   pdev = *(vhci_pdevs + pdev_nr);
-   if (pdev == NULL)
-   continue;
-   hcd = platform_get_drvdata(pdev);
-   if (hcd == NULL)
-   continue;
-   vhci = hcd_to_vhci(hcd);
-   for (rhport = 0; rhport < VHCI_HC_PORTS; rhport++) {
-   if (vhci->vdev[rhport].udev == udev)
-   return >vdev[rhport];
-   }
-   }
-
-   return NULL;
-}
-
-static void vhci_tx_urb(struct urb *urb)
-{
-   struct vhci_device *vdev = get_vdev(urb->dev);
struct vhci_priv *priv;
struct vhci_hcd *vhci;
unsigned long flags;
@@ -601,7 +573,7 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb 
*urb,
}

 out:
-   vhci_tx_urb(urb);
+   vhci_tx_urb(urb, vdev);
spin_unlock_irqrestore(>lock, flags);

return 0;



I think that I've already gave you my reviewed by for this patch.
Nevertheless:

Reviewed-by: Krzysztof Opasiak 

--
Krzysztof Opasiak
Samsung R Institute Poland
Samsung Electronics
--
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] fsl_udc_core: add support for devices provided by fsl-mph-dr-of

[Michael Grzeschik]

Currently the of glue code in fsl-mph-dr-of will create the platform
device fsl-usb2-udc. As this driver should also be probed by this name,
this patch adds it to the devtypes list.

Signed-off-by: Michael Grzeschik 
---
 drivers/usb/gadget/udc/fsl_udc_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/gadget/udc/fsl_udc_core.c 
b/drivers/usb/gadget/udc/fsl_udc_core.c
index b76fcdb763a0d..6f2f71c054be2 100644
--- a/drivers/usb/gadget/udc/fsl_udc_core.c
+++ b/drivers/usb/gadget/udc/fsl_udc_core.c
@@ -2676,6 +2676,8 @@ static const struct platform_device_id fsl_udc_devtype[] 
= {
}, {
.name = "imx-udc-mx51",
}, {
+   .name = "fsl-usb2-udc",
+   }, {
/* sentinel */
}
 };
-- 
2.11.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: Ragentek usb-serial driver

[Alexander Kozyrev]

On Tue, 2017-04-04 at 15:47 +0200, Johan Hovold wrote:
> > cat /sys/bus/usb-serial/drivers/generic/new_id
> > 0e8d 2000
> 
> You only enabled "0e8d 2000", you need to add also "0bb4 0c01" if you
> want the driver to bind to that id.
> 
Got it
cat /sys/bus/usb-serial/drivers/generic/new_id
0e8d 2000
0bb4 0c01
#  ll /dev/ttyUSB0
crw-rw. 1 root dialout 188, 0 Apr  4 16:36 /dev/ttyUSB0


[110255.199557] usb 1-1: new high-speed USB device number 79 using
xhci_hcd
[110255.364361] usb 1-1: New USB device found, idVendor=0bb4,
idProduct=0c01
[110255.364370] usb 1-1: New USB device strings: Mfr=1, Product=2,
SerialNumber=3
[110255.364375] usb 1-1: Product: Android
[110255.364379] usb 1-1: Manufacturer: MediaTek
[110255.364382] usb 1-1: SerialNumber: X560I1ZR5A241388
[110255.365642] usbserial_generic 1-1:1.0: The "generic" usb-serial
driver is only for testing and one-off prototypes.
[110255.365653] usbserial_generic 1-1:1.0: Tell
linux-usb@vger.kernel.org to add your device to a proper driver.
[110255.365659] usbserial_generic 1-1:1.0: generic converter detected
[110255.366208] usb 1-1: generic converter now attached to ttyUSB0

Thanx! 


--
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 v3 1/3] usb: udc: allow adding and removing the same gadget device

[Alan Stern]

On Tue, 4 Apr 2017, Felipe Balbi wrote:

> Hi,
> 
> Alan Stern  writes:
> > On Mon, 3 Apr 2017, Roger Quadros wrote:
> >
> >> allow usb_del_gadget_udc() and usb add_gadget_udc() to be called
> >> repeatedly on the same gadget->dev structure.
> >> 
> >> We need to clear the gadget->dev structure so that kobject_init()
> >> doesn't complain about already initialized object.
> >> 
> >> Signed-off-by: Roger Quadros 
> >> ---
> >>  drivers/usb/gadget/udc/core.c | 1 +
> >>  1 file changed, 1 insertion(+)
> >> 
> >> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
> >> index d685d82..efce68e 100644
> >> --- a/drivers/usb/gadget/udc/core.c
> >> +++ b/drivers/usb/gadget/udc/core.c
> >> @@ -1273,6 +1273,7 @@ void usb_del_gadget_udc(struct usb_gadget *gadget)
> >>flush_work(>work);
> >>device_unregister(>dev);
> >>device_unregister(>dev);
> >> +  memset(>dev, 0x00, sizeof(gadget->dev));
> >>  }
> >>  EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
> >
> > Isn't this dangerous?  It's quite possible that the device_unregister() 
> 
> not on the gadget API, no.
> 
> > call on the previous line invokes the gadget->dev.release callback, 
> > which might deallocate gadget.  If that happens, your new memset will 
> > oops.
> 
> that won't happen. struct usb_gadget is a member of the UDC's private
> structure, like this:
> 
> struct dwc3 {
>   [...]
>   struct usb_gadget   gadget;
>   struct usb_gadget_driver *gadget_driver;
>   [...]
> };

Yes.  So what?  Can't the UDC driver use the refcount inside struct 
usb_gadget to control the lifetime of its private structure?

(By the way, can you tell what's going on in net2280.c?  I must be
missing something; it looks like gadget_release() would quickly run
into problems because it calls dev_get_drvdata() for >dev, but
net2280_probe() never calls dev_set_drvdata() for that device.  
Furthermore, net2280_remove() continues to reference the net2280 struct
after calling usb_del_gadget_udc(), and it never does seem to do a
final put.)

> I'm actually thinking that struct usb_gadget shouldn't have a struct
> device at all. Just a pointer to a device, that would solve all these
> issues.

A pointer to which device?  The UDC?  That would change the directory 
layout in sysfs.

Or a pointer to a separate dynamically allocated device (the way struct 
usb_hcd contains a pointer to the root hub device)?  That would work.  
If the UDC driver wanted to re-register the gadget, it would have to 
allocate a new device.

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: usb: musb: regression since 4.9 on omap4-panda-es (caused by d8e5f0eca1e8)

[Tony Lindgren]

* Bin Liu  [170404 05:30]:
> On Tue, Apr 04, 2017 at 10:09:50AM +0300, Peter Ujfalusi wrote:
> > Tony,
> > 
> > since 4.9 (4.8 was fine) I can not boot omap4-panda-es if the musb
> > is compiled in. The kernel will stuck printing:
> > 
> > ** 206 printk messages dropped ** [8.926727] musb_bus_suspend
> > 2584: trying to suspend as a_idle while active

OK so compiled in. Do you have something connected also when
booting?

> Does it sound a similar issue to
> http://marc.info/?l=linux-usb=149036531809506=2

Yup.

> > The bisect (log is [1]) points to:
> > d8e5f0eca1e8 usb: musb: Fix hardirq-safe hardirq-unsafe lock order error
> > 
> > and reverting the d8e5f0eca1e8 makes the board to boot up fine
> > (Linux 4.11-rc5 and next-20170331).

OK thanks for bisecting it.

> > any idea on how to fix this w/o reverting the commit?

I'll take a look.

Regards,

Tony

> > [1] https://pastebin.com/Z2HJY229
--
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: Ragentek usb-serial driver

[Johan Hovold]

On Tue, Apr 04, 2017 at 03:26:41PM +0200, Alexander Kozyrev wrote:
> On Tue, 2017-04-04 at 15:06 +0200, Johan Hovold wrote:

> > How did you get the generic driver to bind to 0e8d:2000? You need to
> > tell the generic driver to bind also to 0bb4:0c01, for example,
> > 
> > echo 0bb4 0c01 > /sys/bus/usb-serial/drivers/generic/new_id
> 
> Yes, I have pushed it before:
> cat /sys/bus/usb-serial/drivers/generic/new_id
> 0e8d 2000

You only enabled "0e8d 2000", you need to add also "0bb4 0c01" if you
want the driver to bind to that id.

Johan
--
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 v2 2/2] usb: misc: refactor code

[Alan Stern]

On Mon, 3 Apr 2017, Gustavo A. R. Silva wrote:

> Code refactoring to make the flow easier to follow.
> 
> Cc: Alan Stern 
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Gustavo A. R. Silva 
> ---
> Changes in v2:
>  Remove unfruitful changes in previous patch.
>  Revert change to comment.

For both patches:

Acked-by: Alan Stern 

>  drivers/usb/misc/usbtest.c | 49 
> --
>  1 file changed, 21 insertions(+), 28 deletions(-)
> 
> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
> index 7bfb6b78..88f627e 100644
> --- a/drivers/usb/misc/usbtest.c
> +++ b/drivers/usb/misc/usbtest.c
> @@ -124,6 +124,20 @@ static struct usb_device *testdev_to_usbdev(struct 
> usbtest_dev *test)
>  
>  /*-*/
>  
> +static inline void endpoint_update(int edi,
> +struct usb_host_endpoint **in,
> +struct usb_host_endpoint **out,
> +struct usb_host_endpoint *e)
> +{
> + if (edi) {
> + if (!*in)
> + *in = e;
> + } else {
> + if (!*out)
> + *out = e;
> + }
> +}
> +
>  static int
>  get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)
>  {
> @@ -151,47 +165,26 @@ get_endpoints(struct usbtest_dev *dev, struct 
> usb_interface *intf)
>*/
>   for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) {
>   struct usb_host_endpoint*e;
> + int edi;
>  
>   e = alt->endpoint + ep;
> + edi = usb_endpoint_dir_in(>desc);
> +
>   switch (usb_endpoint_type(>desc)) {
>   case USB_ENDPOINT_XFER_BULK:
> - break;
> + endpoint_update(edi, , , e);
> + continue;
>   case USB_ENDPOINT_XFER_INT:
>   if (dev->info->intr)
> - goto try_intr;
> + endpoint_update(edi, _in, _out, 
> e);
>   continue;
>   case USB_ENDPOINT_XFER_ISOC:
>   if (dev->info->iso)
> - goto try_iso;
> + endpoint_update(edi, _in, _out, 
> e);
>   /* FALLTHROUGH */
>   default:
>   continue;
>   }
> - if (usb_endpoint_dir_in(>desc)) {
> - if (!in)
> - in = e;
> - } else {
> - if (!out)
> - out = e;
> - }
> - continue;
> -try_intr:
> - if (usb_endpoint_dir_in(>desc)) {
> - if (!int_in)
> - int_in = e;
> - } else {
> - if (!int_out)
> - int_out = e;
> - }
> - continue;
> -try_iso:
> - if (usb_endpoint_dir_in(>desc)) {
> - if (!iso_in)
> - iso_in = e;
> - } else {
> - if (!iso_out)
> - iso_out = e;
> - }
>   }
>   if ((in && out)  ||  iso_in || iso_out || int_in || int_out)
>   goto found;
> 

--
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: Ragentek usb-serial driver

[Alexander Kozyrev]

On Tue, 2017-04-04 at 15:06 +0200, Johan Hovold wrote:
> > SerialNumber=3
> > [73.663718] usb 1-1: Product: Android
> > [73.663720] usb 1-1: Manufacturer: MediaTek
> > [73.663722] usb 1-1: SerialNumber: X560I1ZR5A241388
> 
> And here it comes up with a new personality (if you didn't switch the
> device).

As described to Oliver before, I try to catch a MiTM, and my Android is
near-brick statement..

In my case I observe different behavior of Android phone. When
connected in fastboot mode I can see adb devices output:
0123456789ABCDEFoffline
this fake serial number belong Ragentek j5808 chip.

And I have made symlink /dev/mediatek in udev rules, it's looks like
work, I cal see it wit ls -la (but can't send|receive AT commands via
minicom). 

If I boot normally, the output  of adb devices shown as
X560I1ZR5A241388offline
this serial belong to real Mediatek Android device. 
> 
> > [root@gigabyte flash-kernel]# ll /dev/ttyUSB0
> > ls: cannot access /dev/ttyUSB0: No such file or directory
> 
> How did you get the generic driver to bind to 0e8d:2000? You need to
> tell the generic driver to bind also to 0bb4:0c01, for example,
> 
> echo 0bb4 0c01 > /sys/bus/usb-serial/drivers/generic/new_id

Yes, I have pushed it before:
cat /sys/bus/usb-serial/drivers/generic/new_id
0e8d 2000

> Johan


--
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: Ragentek usb-serial driver

[Johan Hovold]

On Tue, Apr 04, 2017 at 12:36:00PM +0200, Alexander Kozyrev wrote:
> Hi all!
> 
> Have troubles with Ragentek chip in Mediatek Leagoo Android phone. It's
> tries to connect as ttyACM0 device and ttyUSB0 - unsuccessful.
> 
> [2.219806] usb 1-1: new high-speed USB device number 36 using xhci_hcd
> [2.385119] usb 1-1: New USB device found, idVendor=0e8d, idProduct=2008
> [2.385125] usb 1-1: New USB device strings: Mfr=2, Product=3,
> SerialNumber=4
> [2.385128] usb 1-1: Product: j5808
> [2.385130] usb 1-1: Manufacturer: Ragentek
> [2.385132] usb 1-1: SerialNumber: 0123456789ABCDEF
> [3.368257] usb 1-1: USB disconnect, device number 36
> [3.742359] usb 1-1: new high-speed USB device number 37 using xhci_hcd
> [3.908089] usb 1-1: New USB device found, idVendor=0e8d, idProduct=201c
> [3.908095] usb 1-1: New USB device strings: Mfr=2, Product=3,
> SerialNumber=4
> [3.908097] usb 1-1: Product: j5808
> 
> 
> [3.908099] usb 1-1: Manufacturer: Ragentek
> [3.908101] usb 1-1: SerialNumber: 0123456789ABCDEF
> [6.436388] usb 1-1: USB disconnect, device number 37
> [8.691575] usb 1-1: new high-speed USB device number 38 using xhci_hcd
> [8.856068] usb 1-1: New USB device found, idVendor=0e8d, idProduct=2000
> [8.856073] usb 1-1: New USB device strings: Mfr=1, Product=2,
> SerialNumber=0
> [8.856077] usb 1-1: Product: MT65xx Preloader
> [8.856080] usb 1-1: Manufacturer: MediaTek
> [8.878070] usbserial_generic 1-1:1.0: The "generic" usb-serial driver is
> only for testing and one-off prototypes.
> [8.878074] usbserial_generic 1-1:1.0: Tell linux-usb@vger.kernel.org to
> add your device to a proper driver.
> [8.878076] usbserial_generic 1-1:1.0: generic converter detected
> [8.878333] usb 1-1: generic converter now attached to ttyUSB0
> [8.918180] usbserial_generic 1-1:1.1: Generic device with no bulk out,
> not allowed.
> [8.918191] usbserial_generic: probe of 1-1:1.1 failed with error -5
> [8.918207] cdc_acm: probe of 1-1:1.1 failed with error -16
> [11.358968] usb 1-1: USB disconnect, device number 38

The driver does bind to interface 0 above, but then the device
disconnects itself a couple of seconds later.

> [11.359186] generic ttyUSB0: generic converter now disconnected from
> ttyUSB0
> [71.359205] usbserial_generic 1-1:1.0: device disconnected
> [73.499064] usb 1-1: new high-speed USB device number 39 using xhci_hcd
> [73.663695] usb 1-1: New USB device found, idVendor=0bb4, idProduct=0c01
> [73.663716] usb 1-1: New USB device strings: Mfr=1, Product=2,
> SerialNumber=3
> [73.663718] usb 1-1: Product: Android
> [73.663720] usb 1-1: Manufacturer: MediaTek
> [73.663722] usb 1-1: SerialNumber: X560I1ZR5A241388

And here it comes up with a new personality (if you didn't switch the
device).

> [root@gigabyte flash-kernel]# ll /dev/ttyUSB0
> ls: cannot access /dev/ttyUSB0: No such file or directory

How did you get the generic driver to bind to 0e8d:2000? You need to
tell the generic driver to bind also to 0bb4:0c01, for example,

echo 0bb4 0c01 > /sys/bus/usb-serial/drivers/generic/new_id

Johan
--
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 23/24] usb: xhci: refine xhci_decode_trb()

[Mathias Nyman]

From: Lu Baolu 

Replace 'TRB_FIELD_TO_TYPE(field3)' with 'type' to simplify
code.

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.h | 40 
 1 file changed, 20 insertions(+), 20 deletions(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 4d49f5e..dbe8ba9 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2137,7 +2137,7 @@ static inline const char *xhci_decode_trb(u32 field0, u32 
field1, u32 field2,
sprintf(str,
"LINK %08x%08x intr %d type '%s' flags %c:%c:%c:%c",
field1, field0, GET_INTR_TARGET(field2),
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
field3 & TRB_IOC ? 'I' : 'i',
field3 & TRB_CHAIN ? 'C' : 'c',
field3 & TRB_TC ? 'T' : 't',
@@ -2158,7 +2158,7 @@ static inline const char *xhci_decode_trb(u32 field0, u32 
field1, u32 field2,
EVENT_TRB_LEN(field2), TRB_TO_SLOT_ID(field3),
/* Macro decrements 1, maybe it shouldn't?!? */
TRB_TO_EP_INDEX(field3) + 1,
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
field3 & EVENT_DATA ? 'E' : 'e',
field3 & TRB_CYCLE ? 'C' : 'c');
 
@@ -2175,7 +2175,7 @@ static inline const char *xhci_decode_trb(u32 field0, u32 
field1, u32 field2,
(field1 & 0xff) >> 16,
TRB_LEN(field2), GET_TD_SIZE(field2),
GET_INTR_TARGET(field2),
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
field3 & TRB_IDT ? 'I' : 'i',
field3 & TRB_IOC ? 'I' : 'i',
field3 & TRB_CYCLE ? 'C' : 'c');
@@ -2184,7 +2184,7 @@ static inline const char *xhci_decode_trb(u32 field0, u32 
field1, u32 field2,
sprintf(str, "Buffer %08x%08x length %d TD size %d intr %d type 
'%s' flags %c:%c:%c:%c:%c:%c:%c",
field1, field0, TRB_LEN(field2), 
GET_TD_SIZE(field2),
GET_INTR_TARGET(field2),
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
field3 & TRB_IDT ? 'I' : 'i',
field3 & TRB_IOC ? 'I' : 'i',
field3 & TRB_CHAIN ? 'C' : 'c',
@@ -2197,7 +2197,7 @@ static inline const char *xhci_decode_trb(u32 field0, u32 
field1, u32 field2,
sprintf(str, "Buffer %08x%08x length %d TD size %d intr %d type 
'%s' flags %c:%c:%c:%c",
field1, field0, TRB_LEN(field2), 
GET_TD_SIZE(field2),
GET_INTR_TARGET(field2),
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
field3 & TRB_IOC ? 'I' : 'i',
field3 & TRB_CHAIN ? 'C' : 'c',
field3 & TRB_ENT ? 'E' : 'e',
@@ -2211,7 +2211,7 @@ static inline const char *xhci_decode_trb(u32 field0, u32 
field1, u32 field2,
"Buffer %08x%08x length %d TD size %d intr %d type '%s' 
flags %c:%c:%c:%c:%c:%c:%c:%c",
field1, field0, TRB_LEN(field2), GET_TD_SIZE(field2),
GET_INTR_TARGET(field2),
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
field3 & TRB_BEI ? 'B' : 'b',
field3 & TRB_IDT ? 'I' : 'i',
field3 & TRB_IOC ? 'I' : 'i',
@@ -2226,21 +2226,21 @@ static inline const char *xhci_decode_trb(u32 field0, 
u32 field1, u32 field2,
case TRB_ENABLE_SLOT:
sprintf(str,
"%s: flags %c",
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
field3 & TRB_CYCLE ? 'C' : 'c');
break;
case TRB_DISABLE_SLOT:
case TRB_NEG_BANDWIDTH:
sprintf(str,
"%s: slot %d flags %c",
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   xhci_trb_type_string(type),
TRB_TO_SLOT_ID(field3),
field3 & TRB_CYCLE ? 'C' : 'c');

[PATCH 24/24] usb: xhci: bInterval quirk for TI TUSB73x0

[Mathias Nyman]

From: Roger Quadros 

As per [1] issue #4,
"The periodic EP scheduler always tries to schedule the EPs
that have large intervals (interval equal to or greater than
128 microframes) into different microframes. So it maintains
an internal counter and increments for each large interval
EP added. When the counter is greater than 128, the scheduler
rejects the new EP. So when the hub re-enumerated 128 times,
it triggers this condition."

This results in Bandwidth error when devices with periodic
endpoints (ISO/INT) having bInterval > 7 are plugged and
unplugged several times on a TUSB73x0 XHCI host.

Workaround this issue by limiting the bInterval to 7
(i.e. interval to 6) for High-speed or faster periodic endpoints.

[1] - http://www.ti.com/lit/er/sllz076/sllz076.pdf

Signed-off-by: Roger Quadros 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-mem.c | 11 +++
 drivers/usb/host/xhci-pci.c |  3 +++
 drivers/usb/host/xhci.h |  1 +
 3 files changed, 15 insertions(+)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index b88ec9a..2954b90 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -1503,6 +1503,17 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
 */
max_esit_payload = xhci_get_max_esit_payload(udev, ep);
interval = xhci_get_endpoint_interval(udev, ep);
+
+   /* Periodic endpoint bInterval limit quirk */
+   if (usb_endpoint_xfer_int(>desc) ||
+   usb_endpoint_xfer_isoc(>desc)) {
+   if ((xhci->quirks & XHCI_LIMIT_ENDPOINT_INTERVAL_7) &&
+   udev->speed >= USB_SPEED_HIGH &&
+   interval >= 7) {
+   interval = 6;
+   }
+   }
+
mult = xhci_get_endpoint_mult(udev, ep);
max_packet = usb_endpoint_maxp(>desc);
max_burst = xhci_get_endpoint_max_burst(udev, ep);
diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index fc99f51..7b86508 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -199,6 +199,9 @@ static void xhci_pci_quirks(struct device *dev, struct 
xhci_hcd *xhci)
pdev->device == 0x1042)
xhci->quirks |= XHCI_BROKEN_STREAMS;
 
+   if (pdev->vendor == PCI_VENDOR_ID_TI && pdev->device == 0x8241)
+   xhci->quirks |= XHCI_LIMIT_ENDPOINT_INTERVAL_7;
+
if (xhci->quirks & XHCI_RESET_ON_RESUME)
xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
"QUIRK: Resetting on resume");
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index dbe8ba9..914968c 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1820,6 +1820,7 @@ struct xhci_hcd {
 #define XHCI_MISSING_CAS   (1 << 24)
 /* For controller with a broken Port Disable implementation */
 #define XHCI_BROKEN_PORT_PED   (1 << 25)
+#define XHCI_LIMIT_ENDPOINT_INTERVAL_7 (1 << 26)
 
unsigned intnum_active_eps;
unsigned intlimit_active_eps;
-- 
1.9.1

--
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 19/24] usb: xhci: remove ring debugging code

[Mathias Nyman]

From: Lu Baolu 

XHCI ring changes have already been traced by the ring trace
events. It's unnecessary to put the same messages in kernel
log. This patch removes the debugging code for a ring.

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-dbg.c  | 62 
 drivers/usb/host/xhci-ring.c |  4 ---
 drivers/usb/host/xhci.c  |  6 -
 drivers/usb/host/xhci.h  |  3 ---
 4 files changed, 75 deletions(-)

diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index 21c563f..77f80ce 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -311,68 +311,6 @@ void xhci_debug_trb(struct xhci_hcd *xhci, union xhci_trb 
*trb)
}
 }
 
-/**
- * Debug a segment with an xHCI ring.
- *
- * @return The Link TRB of the segment, or NULL if there is no Link TRB
- * (which is a bug, since all segments must have a Link TRB).
- *
- * Prints out all TRBs in the segment, even those after the Link TRB.
- *
- * XXX: should we print out TRBs that the HC owns?  As long as we don't
- * write, that should be fine...  We shouldn't expect that the memory pointed 
to
- * by the TRB is valid at all.  Do we care about ones the HC owns?  Probably,
- * for HC debugging.
- */
-void xhci_debug_segment(struct xhci_hcd *xhci, struct xhci_segment *seg)
-{
-   int i;
-   u64 addr = seg->dma;
-   union xhci_trb *trb = seg->trbs;
-
-   for (i = 0; i < TRBS_PER_SEGMENT; i++) {
-   trb = >trbs[i];
-   xhci_dbg(xhci, "@%016llx %08x %08x %08x %08x\n", addr,
-lower_32_bits(le64_to_cpu(trb->link.segment_ptr)),
-upper_32_bits(le64_to_cpu(trb->link.segment_ptr)),
-le32_to_cpu(trb->link.intr_target),
-le32_to_cpu(trb->link.control));
-   addr += sizeof(*trb);
-   }
-}
-
-void xhci_dbg_ring_ptrs(struct xhci_hcd *xhci, struct xhci_ring *ring)
-{
-   xhci_dbg(xhci, "Ring deq = %p (virt), 0x%llx (dma)\n",
-   ring->dequeue,
-   (unsigned long long)xhci_trb_virt_to_dma(ring->deq_seg,
-   ring->dequeue));
-   xhci_dbg(xhci, "Ring enq = %p (virt), 0x%llx (dma)\n",
-   ring->enqueue,
-   (unsigned long long)xhci_trb_virt_to_dma(ring->enq_seg,
-   ring->enqueue));
-}
-
-/**
- * Debugging for an xHCI ring, which is a queue broken into multiple segments.
- *
- * Print out each segment in the ring.  Check that the DMA address in
- * each link segment actually matches the segment's stored DMA address.
- * Check that the link end bit is only set at the end of the ring.
- * Check that the dequeue and enqueue pointers point to real data in this ring
- * (not some other ring).
- */
-void xhci_debug_ring(struct xhci_hcd *xhci, struct xhci_ring *ring)
-{
-   /* FIXME: Throw an error if any segment doesn't have a Link TRB */
-   struct xhci_segment *seg;
-   struct xhci_segment *first_seg = ring->first_seg;
-   xhci_debug_segment(xhci, first_seg);
-
-   for (seg = first_seg->next; seg != first_seg; seg = seg->next)
-   xhci_debug_segment(xhci, seg);
-}
-
 void xhci_dbg_erst(struct xhci_hcd *xhci, struct xhci_erst *erst)
 {
u64 addr = erst->erst_dma_addr;
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index b382cf0..a2bfd75 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2292,8 +2292,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 upper_32_bits(le64_to_cpu(event->buffer)),
 le32_to_cpu(event->transfer_len),
 le32_to_cpu(event->flags));
-   xhci_dbg(xhci, "Event ring:\n");
-   xhci_debug_segment(xhci, xhci->event_ring->deq_seg);
return -ENODEV;
}
 
@@ -2314,8 +2312,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
 upper_32_bits(le64_to_cpu(event->buffer)),
 le32_to_cpu(event->transfer_len),
 le32_to_cpu(event->flags));
-   xhci_dbg(xhci, "Event ring:\n");
-   xhci_debug_segment(xhci, xhci->event_ring->deq_seg);
return -ENODEV;
}
 
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 9d4801a..245ed40 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -622,16 +622,10 @@ int xhci_run(struct usb_hcd *hcd)
if (ret)
return ret;
 
-   xhci_dbg(xhci, "Command ring memory map follows:\n");
-   xhci_debug_ring(xhci, xhci->cmd_ring);
-   xhci_dbg_ring_ptrs(xhci, xhci->cmd_ring);
xhci_dbg_cmd_ptrs(xhci);
 

[PATCH 22/24] usb: xhci: fix link trb decoding

[Mathias Nyman]

From: Lu Baolu 

xhci_decode_trb() treats a link trb in the same way as that for
an event trb. This patch fixes this by decoding the link trb
according to the spec.

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.h | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 04e041f..4d49f5e 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2135,14 +2135,12 @@ static inline const char *xhci_decode_trb(u32 field0, 
u32 field1, u32 field2,
switch (type) {
case TRB_LINK:
sprintf(str,
-   "TRB %08x%08x status '%s' len %d slot %d ep %d type 
'%s' flags %c:%c",
-   field1, field0,
-   xhci_trb_comp_code_string(GET_COMP_CODE(field2)),
-   EVENT_TRB_LEN(field2), TRB_TO_SLOT_ID(field3),
-   /* Macro decrements 1, maybe it shouldn't?!? */
-   TRB_TO_EP_INDEX(field3) + 1,
+   "LINK %08x%08x intr %d type '%s' flags %c:%c:%c:%c",
+   field1, field0, GET_INTR_TARGET(field2),
xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
-   field3 & EVENT_DATA ? 'E' : 'e',
+   field3 & TRB_IOC ? 'I' : 'i',
+   field3 & TRB_CHAIN ? 'C' : 'c',
+   field3 & TRB_TC ? 'T' : 't',
field3 & TRB_CYCLE ? 'C' : 'c');
break;
case TRB_TRANSFER:
-- 
1.9.1

--
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 18/24] usb: xhci: remove enq_updates and deq_updates from ring

[Mathias Nyman]

From: Lu Baolu 

enq_updates and deq_updates were introduced in the first place
to check whether an xhci hardware is able to respond to trbs
enqueued in the ring. We now have trb tracers to trace every
single enqueue/dequeue trb. It's time to remove them and the
associated debugging code.

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-dbg.c  | 8 
 drivers/usb/host/xhci-mem.c  | 3 ---
 drivers/usb/host/xhci-ring.c | 3 ---
 drivers/usb/host/xhci.h  | 2 --
 4 files changed, 16 deletions(-)

diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index f6d3031..21c563f 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -347,14 +347,10 @@ void xhci_dbg_ring_ptrs(struct xhci_hcd *xhci, struct 
xhci_ring *ring)
ring->dequeue,
(unsigned long long)xhci_trb_virt_to_dma(ring->deq_seg,
ring->dequeue));
-   xhci_dbg(xhci, "Ring deq updated %u times\n",
-   ring->deq_updates);
xhci_dbg(xhci, "Ring enq = %p (virt), 0x%llx (dma)\n",
ring->enqueue,
(unsigned long long)xhci_trb_virt_to_dma(ring->enq_seg,
ring->enqueue));
-   xhci_dbg(xhci, "Ring enq updated %u times\n",
-   ring->enq_updates);
 }
 
 /**
@@ -373,10 +369,6 @@ void xhci_debug_ring(struct xhci_hcd *xhci, struct 
xhci_ring *ring)
struct xhci_segment *first_seg = ring->first_seg;
xhci_debug_segment(xhci, first_seg);
 
-   if (!ring->enq_updates && !ring->deq_updates) {
-   xhci_dbg(xhci, "  Ring has not been updated\n");
-   return;
-   }
for (seg = first_seg->next; seg != first_seg; seg = seg->next)
xhci_debug_segment(xhci, seg);
 }
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 7050131..b88ec9a 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -315,9 +315,6 @@ static void xhci_initialize_ring_info(struct xhci_ring 
*ring,
 * handling ring expansion, set the cycle state equal to the old ring.
 */
ring->cycle_state = cycle_state;
-   /* Not necessary for new rings, but needed for re-initialized rings */
-   ring->enq_updates = 0;
-   ring->deq_updates = 0;
 
/*
 * Each segment has a link TRB, and leave an extra TRB for SW
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index deb318e..b382cf0 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -167,8 +167,6 @@ static void next_trb(struct xhci_hcd *xhci,
  */
 static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring *ring)
 {
-   ring->deq_updates++;
-
/* event ring doesn't have link trbs, check for last trb */
if (ring->type == TYPE_EVENT) {
if (!last_trb_on_seg(ring->deq_seg, ring->dequeue)) {
@@ -226,7 +224,6 @@ static void inc_enq(struct xhci_hcd *xhci, struct xhci_ring 
*ring,
ring->num_trbs_free--;
next = ++(ring->enqueue);
 
-   ring->enq_updates++;
/* Update the dequeue pointer further if that was a link TRB */
while (trb_is_link(next)) {
 
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index dc69d3c..1a64466 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1566,10 +1566,8 @@ struct xhci_ring {
struct xhci_segment *last_seg;
union  xhci_trb *enqueue;
struct xhci_segment *enq_seg;
-   unsigned intenq_updates;
union  xhci_trb *dequeue;
struct xhci_segment *deq_seg;
-   unsigned intdeq_updates;
struct list_headtd_list;
/*
 * Write the cycle state into the TRB cycle field to give ownership of
-- 
1.9.1

--
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 14/24] usb: xhci: add xhci_log_ring trace events

[Mathias Nyman]

From: Lu Baolu 

This patch creates a new event class called xhci_log_ring, and
defines the events used for tracing the change of all kinds of
rings used by an xhci host. An xHCI ring is basically a memory
block shared between software and hardware. By tracing changes
of rings, it makes the life easier for debugging hardware or
software problems.

This info can be used, later, to print, in a human readable way,
the life cycle of an xHCI ring using the trace-cmd tool and the
appropriate plugin.

Signed-off-by: Lu Baolu 
Reviewed-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-mem.c   |  4 +++
 drivers/usb/host/xhci-ring.c  |  5 
 drivers/usb/host/xhci-trace.h | 65 +++
 3 files changed, 74 insertions(+)

diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index 032a702..c970107 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -288,6 +288,8 @@ void xhci_ring_free(struct xhci_hcd *xhci, struct xhci_ring 
*ring)
if (!ring)
return;
 
+   trace_xhci_ring_free(ring);
+
if (ring->first_seg) {
if (ring->type == TYPE_STREAM)
xhci_remove_stream_mapping(ring);
@@ -400,6 +402,7 @@ static struct xhci_ring *xhci_ring_alloc(struct xhci_hcd 
*xhci,
cpu_to_le32(LINK_TOGGLE);
}
xhci_initialize_ring_info(ring, cycle_state);
+   trace_xhci_ring_alloc(ring);
return ring;
 
 fail:
@@ -504,6 +507,7 @@ int xhci_ring_expansion(struct xhci_hcd *xhci, struct 
xhci_ring *ring,
}
 
xhci_link_rings(xhci, ring, first, last, num_segs);
+   trace_xhci_ring_expansion(ring);
xhci_dbg_trace(xhci, trace_xhci_dbg_ring_expansion,
"ring expansion succeed, now has %d segments",
ring->num_segs);
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index c8910fd..28ea693 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -191,6 +191,9 @@ static void inc_deq(struct xhci_hcd *xhci, struct xhci_ring 
*ring)
ring->deq_seg = ring->deq_seg->next;
ring->dequeue = ring->deq_seg->trbs;
}
+
+   trace_xhci_inc_deq(ring);
+
return;
 }
 
@@ -259,6 +262,8 @@ static void inc_enq(struct xhci_hcd *xhci, struct xhci_ring 
*ring,
ring->enqueue = ring->enq_seg->trbs;
next = ring->enqueue;
}
+
+   trace_xhci_inc_enq(ring);
 }
 
 /*
diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 7baeab94..8ce96de 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -388,6 +388,71 @@
TP_ARGS(ctx)
 );
 
+DECLARE_EVENT_CLASS(xhci_log_ring,
+   TP_PROTO(struct xhci_ring *ring),
+   TP_ARGS(ring),
+   TP_STRUCT__entry(
+   __field(u32, type)
+   __field(void *, ring)
+   __field(dma_addr_t, enq)
+   __field(dma_addr_t, deq)
+   __field(dma_addr_t, enq_seg)
+   __field(dma_addr_t, deq_seg)
+   __field(unsigned int, num_segs)
+   __field(unsigned int, stream_id)
+   __field(unsigned int, cycle_state)
+   __field(unsigned int, num_trbs_free)
+   __field(unsigned int, bounce_buf_len)
+   ),
+   TP_fast_assign(
+   __entry->ring = ring;
+   __entry->type = ring->type;
+   __entry->num_segs = ring->num_segs;
+   __entry->stream_id = ring->stream_id;
+   __entry->enq_seg = ring->enq_seg->dma;
+   __entry->deq_seg = ring->deq_seg->dma;
+   __entry->cycle_state = ring->cycle_state;
+   __entry->num_trbs_free = ring->num_trbs_free;
+   __entry->bounce_buf_len = ring->bounce_buf_len;
+   __entry->enq = xhci_trb_virt_to_dma(ring->enq_seg, 
ring->enqueue);
+   __entry->deq = xhci_trb_virt_to_dma(ring->deq_seg, 
ring->dequeue);
+   ),
+   TP_printk("%s %p: enq %pad(%pad) deq %pad(%pad) segs %d stream %d 
free_trbs %d bounce %d cycle %d",
+   xhci_ring_type_string(__entry->type), __entry->ring,
+   &__entry->enq, &__entry->enq_seg,
+   &__entry->deq, &__entry->deq_seg,
+   __entry->num_segs,
+   __entry->stream_id,
+   __entry->num_trbs_free,
+   __entry->bounce_buf_len,
+   __entry->cycle_state
+   )
+);
+
+DEFINE_EVENT(xhci_log_ring, xhci_ring_alloc,
+   TP_PROTO(struct xhci_ring *ring),
+   TP_ARGS(ring)
+);
+
+DEFINE_EVENT(xhci_log_ring, xhci_ring_free,
+   TP_PROTO(struct xhci_ring *ring),
+  

[PATCH 17/24] usb: xhci: remove error messages for failed memory allocation

[Mathias Nyman]

From: Lu Baolu 

Omit extra messages for memory allocation failure.

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c  |  4 +---
 drivers/usb/host/xhci-mem.c  |  1 -
 drivers/usb/host/xhci-ring.c | 10 --
 drivers/usb/host/xhci.c  | 13 +++--
 4 files changed, 12 insertions(+), 16 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 0e23775..1c95a52 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -392,10 +392,8 @@ static int xhci_stop_device(struct xhci_hcd *xhci, int 
slot_id, int suspend)
trace_xhci_stop_device(virt_dev);
 
cmd = xhci_alloc_command(xhci, false, true, GFP_NOIO);
-   if (!cmd) {
-   xhci_dbg(xhci, "Couldn't allocate command structure.\n");
+   if (!cmd)
return -ENOMEM;
-   }
 
spin_lock_irqsave(>lock, flags);
for (i = LAST_EP_INDEX; i > 0; i--) {
diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
index c970107..7050131 100644
--- a/drivers/usb/host/xhci-mem.c
+++ b/drivers/usb/host/xhci-mem.c
@@ -2605,7 +2605,6 @@ int xhci_mem_init(struct xhci_hcd *xhci, gfp_t flags)
return 0;
 
 fail:
-   xhci_warn(xhci, "Couldn't initialize memory\n");
xhci_halt(xhci);
xhci_reset(xhci);
xhci_mem_cleanup(xhci);
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 28ea693..deb318e 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -1135,11 +1135,11 @@ static void xhci_handle_cmd_reset_ep(struct xhci_hcd 
*xhci, int slot_id,
 */
if (xhci->quirks & XHCI_RESET_EP_QUIRK) {
struct xhci_command *command;
+
command = xhci_alloc_command(xhci, false, false, GFP_ATOMIC);
-   if (!command) {
-   xhci_warn(xhci, "WARN Cannot submit cfg ep: ENOMEM\n");
+   if (!command)
return;
-   }
+
xhci_dbg_trace(xhci, trace_xhci_dbg_quirks,
"Queueing configure endpoint command");
xhci_queue_configure_endpoint(xhci, command,
@@ -4015,10 +4015,8 @@ void xhci_queue_new_dequeue_state(struct xhci_hcd *xhci,
 
/* This function gets called from contexts where it cannot sleep */
cmd = xhci_alloc_command(xhci, false, false, GFP_ATOMIC);
-   if (!cmd) {
-   xhci_warn(xhci, "WARN Cannot submit Set TR Deq Ptr: ENOMEM\n");
+   if (!cmd)
return;
-   }
 
ep->queued_deq_seg = deq_state->new_deq_seg;
ep->queued_deq_ptr = deq_state->new_deq_ptr;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 853b618..9d4801a 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -664,9 +664,11 @@ int xhci_run(struct usb_hcd *hcd)
 
if (xhci->quirks & XHCI_NEC_HOST) {
struct xhci_command *command;
+
command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
if (!command)
return -ENOMEM;
+
xhci_queue_vendor_command(xhci, command, 0, 0, 0,
TRB_TYPE(TRB_NEC_GET_FW));
}
@@ -3144,10 +3146,9 @@ static int xhci_alloc_streams(struct usb_hcd *hcd, 
struct usb_device *udev,
}
 
config_cmd = xhci_alloc_command(xhci, true, true, mem_flags);
-   if (!config_cmd) {
-   xhci_dbg(xhci, "Could not allocate xHCI command structure.\n");
+   if (!config_cmd)
return -ENOMEM;
-   }
+
ctrl_ctx = xhci_get_input_control_ctx(config_cmd->in_ctx);
if (!ctrl_ctx) {
xhci_warn(xhci, "%s: Could not get input context, bad type.\n",
@@ -4753,11 +4754,11 @@ static int xhci_update_hub_device(struct usb_hcd *hcd, 
struct usb_device *hdev,
xhci_warn(xhci, "Cannot update hub desc for unknown device.\n");
return -EINVAL;
}
+
config_cmd = xhci_alloc_command(xhci, true, true, mem_flags);
-   if (!config_cmd) {
-   xhci_dbg(xhci, "Could not allocate xHCI command structure.\n");
+   if (!config_cmd)
return -ENOMEM;
-   }
+
ctrl_ctx = xhci_get_input_control_ctx(config_cmd->in_ctx);
if (!ctrl_ctx) {
xhci_warn(xhci, "%s: Could not get input context, bad type.\n",
-- 
1.9.1

--
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 21/24] usb: xhci: remove xhci_dbg_ctx()

[Mathias Nyman]

From: Lu Baolu 

XHCI context changes have already been traced by the trace
events. It's unnecessary to put the same message in kernel
log. This patch removes the use of xhci_dbg_ctx().

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-dbg.c | 143 
 drivers/usb/host/xhci.c |  37 
 drivers/usb/host/xhci.h |   1 -
 3 files changed, 181 deletions(-)

diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index dc0194b..2c83b37 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -283,19 +283,6 @@ void xhci_dbg_cmd_ptrs(struct xhci_hcd *xhci)
upper_32_bits(val));
 }
 
-/* Print the last 32 bytes for 64-byte contexts */
-static void dbg_rsvd64(struct xhci_hcd *xhci, u64 *ctx, dma_addr_t dma)
-{
-   int i;
-   for (i = 0; i < 4; i++) {
-   xhci_dbg(xhci, "@%p (virt) @%08llx "
-"(dma) %#08llx - rsvd64[%d]\n",
-[4 + i], (unsigned long long)dma,
-ctx[4 + i], i);
-   dma += 8;
-   }
-}
-
 char *xhci_get_slot_state(struct xhci_hcd *xhci,
struct xhci_container_ctx *ctx)
 {
@@ -305,136 +292,6 @@ char *xhci_get_slot_state(struct xhci_hcd *xhci,
return xhci_slot_state_string(state);
 }
 
-static void xhci_dbg_slot_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx 
*ctx)
-{
-   /* Fields are 32 bits wide, DMA addresses are in bytes */
-   int field_size = 32 / 8;
-   int i;
-
-   struct xhci_slot_ctx *slot_ctx = xhci_get_slot_ctx(xhci, ctx);
-   dma_addr_t dma = ctx->dma +
-   ((unsigned long)slot_ctx - (unsigned long)ctx->bytes);
-   int csz = HCC_64BYTE_CONTEXT(xhci->hcc_params);
-
-   xhci_dbg(xhci, "Slot Context:\n");
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - dev_info\n",
-   _ctx->dev_info,
-   (unsigned long long)dma, slot_ctx->dev_info);
-   dma += field_size;
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - dev_info2\n",
-   _ctx->dev_info2,
-   (unsigned long long)dma, slot_ctx->dev_info2);
-   dma += field_size;
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - tt_info\n",
-   _ctx->tt_info,
-   (unsigned long long)dma, slot_ctx->tt_info);
-   dma += field_size;
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - dev_state\n",
-   _ctx->dev_state,
-   (unsigned long long)dma, slot_ctx->dev_state);
-   dma += field_size;
-   for (i = 0; i < 4; i++) {
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - rsvd[%d]\n",
-   _ctx->reserved[i], (unsigned long long)dma,
-   slot_ctx->reserved[i], i);
-   dma += field_size;
-   }
-
-   if (csz)
-   dbg_rsvd64(xhci, (u64 *)slot_ctx, dma);
-}
-
-static void xhci_dbg_ep_ctx(struct xhci_hcd *xhci,
-struct xhci_container_ctx *ctx,
-unsigned int last_ep)
-{
-   int i, j;
-   int last_ep_ctx = 31;
-   /* Fields are 32 bits wide, DMA addresses are in bytes */
-   int field_size = 32 / 8;
-   int csz = HCC_64BYTE_CONTEXT(xhci->hcc_params);
-
-   if (last_ep < 31)
-   last_ep_ctx = last_ep + 1;
-   for (i = 0; i < last_ep_ctx; i++) {
-   unsigned int epaddr = xhci_get_endpoint_address(i);
-   struct xhci_ep_ctx *ep_ctx = xhci_get_ep_ctx(xhci, ctx, i);
-   dma_addr_t dma = ctx->dma +
-   ((unsigned long)ep_ctx - (unsigned long)ctx->bytes);
-
-   xhci_dbg(xhci, "%s Endpoint %02d Context (ep_index %02d):\n",
-   usb_endpoint_out(epaddr) ? "OUT" : "IN",
-   epaddr & USB_ENDPOINT_NUMBER_MASK, i);
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - ep_info\n",
-   _ctx->ep_info,
-   (unsigned long long)dma, ep_ctx->ep_info);
-   dma += field_size;
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - ep_info2\n",
-   _ctx->ep_info2,
-   (unsigned long long)dma, ep_ctx->ep_info2);
-   dma += field_size;
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08llx - deq\n",
-   _ctx->deq,
-   (unsigned long long)dma, ep_ctx->deq);
-   dma += 2*field_size;
-   xhci_dbg(xhci, "@%p (virt) @%08llx (dma) %#08x - tx_info\n",
-   _ctx->tx_info,
-   (unsigned long long)dma, 

[PATCH 20/24] usb: xhci: remove xhci_debug_trb()

[Mathias Nyman]

From: Lu Baolu 

Every XHCI TRB has already been traced by the trb trace events.
It is unnecessary to put the same message in kernel log. This
patch removes xhci_debug_trb().

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-dbg.c  | 57 
 drivers/usb/host/xhci-ring.c |  4 
 drivers/usb/host/xhci.h  |  2 --
 3 files changed, 63 deletions(-)

diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index 77f80ce..dc0194b 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -254,63 +254,6 @@ void xhci_print_registers(struct xhci_hcd *xhci)
xhci_print_ports(xhci);
 }
 
-void xhci_print_trb_offsets(struct xhci_hcd *xhci, union xhci_trb *trb)
-{
-   int i;
-   for (i = 0; i < 4; i++)
-   xhci_dbg(xhci, "Offset 0x%x = 0x%x\n",
-   i*4, trb->generic.field[i]);
-}
-
-/**
- * Debug a transfer request block (TRB).
- */
-void xhci_debug_trb(struct xhci_hcd *xhci, union xhci_trb *trb)
-{
-   u64 address;
-   u32 type = le32_to_cpu(trb->link.control) & TRB_TYPE_BITMASK;
-
-   switch (type) {
-   case TRB_TYPE(TRB_LINK):
-   xhci_dbg(xhci, "Link TRB:\n");
-   xhci_print_trb_offsets(xhci, trb);
-
-   address = le64_to_cpu(trb->link.segment_ptr);
-   xhci_dbg(xhci, "Next ring segment DMA address = 0x%llx\n", 
address);
-
-   xhci_dbg(xhci, "Interrupter target = 0x%x\n",
-GET_INTR_TARGET(le32_to_cpu(trb->link.intr_target)));
-   xhci_dbg(xhci, "Cycle bit = %u\n",
-le32_to_cpu(trb->link.control) & TRB_CYCLE);
-   xhci_dbg(xhci, "Toggle cycle bit = %u\n",
-le32_to_cpu(trb->link.control) & LINK_TOGGLE);
-   xhci_dbg(xhci, "No Snoop bit = %u\n",
-le32_to_cpu(trb->link.control) & TRB_NO_SNOOP);
-   break;
-   case TRB_TYPE(TRB_TRANSFER):
-   address = le64_to_cpu(trb->trans_event.buffer);
-   /*
-* FIXME: look at flags to figure out if it's an address or if
-* the data is directly in the buffer field.
-*/
-   xhci_dbg(xhci, "DMA address or buffer contents= %llu\n", 
address);
-   break;
-   case TRB_TYPE(TRB_COMPLETION):
-   address = le64_to_cpu(trb->event_cmd.cmd_trb);
-   xhci_dbg(xhci, "Command TRB pointer = %llu\n", address);
-   xhci_dbg(xhci, "Completion status = %u\n",
-GET_COMP_CODE(le32_to_cpu(trb->event_cmd.status)));
-   xhci_dbg(xhci, "Flags = 0x%x\n",
-le32_to_cpu(trb->event_cmd.flags));
-   break;
-   default:
-   xhci_dbg(xhci, "Unknown TRB with TRB type ID %u\n",
-   (unsigned int) type>>10);
-   xhci_print_trb_offsets(xhci, trb);
-   break;
-   }
-}
-
 void xhci_dbg_erst(struct xhci_hcd *xhci, struct xhci_erst *erst)
 {
u64 addr = erst->erst_dma_addr;
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index a2bfd75..74bf5c6 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2463,10 +2463,6 @@ static int handle_tx_event(struct xhci_hcd *xhci,
xhci_warn(xhci, "WARN Event TRB for slot %d ep 
%d with no TDs queued?\n",

TRB_TO_SLOT_ID(le32_to_cpu(event->flags)),
ep_index);
-   xhci_dbg(xhci, "Event TRB with TRB type ID 
%u\n",
-   (le32_to_cpu(event->flags) &
-TRB_TYPE_BITMASK)>>10);
-   xhci_print_trb_offsets(xhci, (union xhci_trb *) 
event);
}
if (ep->skip) {
ep->skip = false;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index dca5909..aef4fd5 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1921,8 +1921,6 @@ static inline int xhci_link_trb_quirk(struct xhci_hcd 
*xhci)
 void xhci_print_registers(struct xhci_hcd *xhci);
 void xhci_dbg_regs(struct xhci_hcd *xhci);
 void xhci_print_run_regs(struct xhci_hcd *xhci);
-void xhci_print_trb_offsets(struct xhci_hcd *xhci, union xhci_trb *trb);
-void xhci_debug_trb(struct xhci_hcd *xhci, union xhci_trb *trb);
 void xhci_dbg_erst(struct xhci_hcd *xhci, struct xhci_erst *erst);
 void xhci_dbg_cmd_ptrs(struct xhci_hcd *xhci);
 void xhci_dbg_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx, 
unsigned int last_ep);
-- 
1.9.1

--
To unsubscribe from this list: 

[PATCH 16/24] usb: xhci: make several functions static

[Mathias Nyman]

From: Lu Baolu 

Several functions have a single user in the same file where it
is defined. There's no need to expose it anywhere else.

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.c | 59 +
 drivers/usb/host/xhci.h | 41 --
 2 files changed, 30 insertions(+), 70 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 3e5d89b..853b618 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -542,7 +542,7 @@ static int xhci_all_ports_seen_u0(struct xhci_hcd *xhci)
  * device contexts (?), set up a command ring segment (or two?), create event
  * ring (one for now).
  */
-int xhci_init(struct usb_hcd *hcd)
+static int xhci_init(struct usb_hcd *hcd)
 {
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
int retval = 0;
@@ -685,7 +685,7 @@ int xhci_run(struct usb_hcd *hcd)
  * Disable device contexts, disable IRQs, and quiesce the HC.
  * Reset the HC, finish any completed transactions, and cleanup memory.
  */
-void xhci_stop(struct usb_hcd *hcd)
+static void xhci_stop(struct usb_hcd *hcd)
 {
u32 temp;
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
@@ -746,7 +746,7 @@ void xhci_stop(struct usb_hcd *hcd)
  *
  * This will only ever be called with the main usb_hcd (the USB3 roothub).
  */
-void xhci_shutdown(struct usb_hcd *hcd)
+static void xhci_shutdown(struct usb_hcd *hcd)
 {
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
 
@@ -1179,7 +1179,7 @@ unsigned int xhci_get_endpoint_address(unsigned int 
ep_index)
  * endpoint index to create a bitmask.  The slot context is bit 0, endpoint 0 
is
  * bit 1, etc.
  */
-unsigned int xhci_get_endpoint_flag(struct usb_endpoint_descriptor *desc)
+static unsigned int xhci_get_endpoint_flag(struct usb_endpoint_descriptor 
*desc)
 {
return 1 << (xhci_get_endpoint_index(desc) + 1);
 }
@@ -1188,7 +1188,7 @@ unsigned int xhci_get_endpoint_flag(struct 
usb_endpoint_descriptor *desc)
  * endpoint index to create a bitmask.  The slot context is bit 0, endpoint 0 
is
  * bit 1, etc.
  */
-unsigned int xhci_get_endpoint_flag_from_index(unsigned int ep_index)
+static unsigned int xhci_get_endpoint_flag_from_index(unsigned int ep_index)
 {
return 1 << (ep_index + 1);
 }
@@ -1332,7 +1332,7 @@ static int xhci_check_maxpacket(struct xhci_hcd *xhci, 
unsigned int slot_id,
  * non-error returns are a promise to giveback() the urb later
  * we drop ownership so next owner (or urb unlink) can get it
  */
-int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flags)
+static int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t 
mem_flags)
 {
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
unsigned long flags;
@@ -1468,7 +1468,7 @@ int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb 
*urb, gfp_t mem_flags)
  * Note that this function can be called in any context, or so says
  * usb_hcd_unlink_urb()
  */
-int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
+static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status)
 {
unsigned long flags;
int ret, i;
@@ -1585,7 +1585,7 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb 
*urb, int status)
  * disabled, so there's no need for mutual exclusion to protect
  * the xhci->devs[slot_id] structure.
  */
-int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
+static int xhci_drop_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
struct usb_host_endpoint *ep)
 {
struct xhci_hcd *xhci;
@@ -1668,7 +1668,7 @@ int xhci_drop_endpoint(struct usb_hcd *hcd, struct 
usb_device *udev,
  * configuration or alt setting is installed in the device, so there's no need
  * for mutual exclusion to protect the xhci->devs[slot_id] structure.
  */
-int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
+static int xhci_add_endpoint(struct usb_hcd *hcd, struct usb_device *udev,
struct usb_host_endpoint *ep)
 {
struct xhci_hcd *xhci;
@@ -2339,7 +2339,7 @@ static unsigned int xhci_get_ss_bw_consumed(struct 
xhci_bw_info *ep_bw)
 
 }
 
-void xhci_drop_ep_from_interval_table(struct xhci_hcd *xhci,
+static void xhci_drop_ep_from_interval_table(struct xhci_hcd *xhci,
struct xhci_bw_info *ep_bw,
struct xhci_interval_bw_table *bw_table,
struct usb_device *udev,
@@ -2704,7 +2704,7 @@ static void xhci_check_bw_drop_ep_streams(struct xhci_hcd 
*xhci,
  * else should be touching the xhci->devs[slot_id] structure, so we
  * don't need to take the xhci->lock for manipulating that.
  */
-int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
+static int xhci_check_bandwidth(struct usb_hcd *hcd, struct usb_device *udev)
 {
int i;
int ret = 0;
@@ -2808,7 +2808,7 @@ int 

[PATCH 12/24] xhci: Do not halt the host until both HCD have disconnected their devices.

[Mathias Nyman]

From: Joel Stanley 

We can't halt the host controller immediately when first HCD is removed as
it will cause problems if we have devices attached to the second (primary)
HCD, like a keyboard.

We've been carrying this in our Linux-as-a-bootloader environment for a
little while now. The machines all have the same TI TUSB73x0 part,
and when we kexec the devices don't come back until a system power cycle.

[minor adjustments, code comments and remove HALT check  -Mathias]
Signed-off-by: Joel Stanley 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.c | 20 ++--
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index e523dbd..5ecba7c 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -692,21 +692,21 @@ void xhci_stop(struct usb_hcd *hcd)
 
mutex_lock(>mutex);
 
-   if (!(xhci->xhc_state & XHCI_STATE_HALTED)) {
-   spin_lock_irq(>lock);
-
-   xhci->xhc_state |= XHCI_STATE_HALTED;
-   xhci->cmd_ring_state = CMD_RING_STATE_STOPPED;
-   xhci_halt(xhci);
-   xhci_reset(xhci);
-   spin_unlock_irq(>lock);
-   }
-
+   /* Only halt host and free memory after both hcds are removed */
if (!usb_hcd_is_primary_hcd(hcd)) {
+   /* usb core will free this hcd shortly, unset pointer */
+   xhci->shared_hcd = NULL;
mutex_unlock(>mutex);
return;
}
 
+   spin_lock_irq(>lock);
+   xhci->xhc_state |= XHCI_STATE_HALTED;
+   xhci->cmd_ring_state = CMD_RING_STATE_STOPPED;
+   xhci_halt(xhci);
+   xhci_reset(xhci);
+   spin_unlock_irq(>lock);
+
xhci_cleanup_msix(xhci);
 
/* Deleting Compliance Mode Recovery Timer */
-- 
1.9.1

--
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 13/24] xhci: Rework how we handle unresponsive or hoptlug removed hosts

[Mathias Nyman]

Introduce a new xhci_hc_died() function that takes care of handling
pending commands and URBs if a host controller becomes unresponsive.

This addresses issues on hotpluggable xhci controllers that disappear
from the bus suddenly, often while the bus (PCI) remove function is
still being processed.

xhci_hc_died() sets a XHCI_STATUS_DYING flag to prevent new URBs and
commands or to be queued. The flag also ensures xhci_hc_died() will
give back pending commands and URBs once.

Host is considered dead if register read returns 0x, or host
fails to abort the command ring, or fails stopping an endpoint after
trying for 5 seconds.

Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c  |  12 +++--
 drivers/usb/host/xhci-ring.c | 118 +--
 drivers/usb/host/xhci.c  |  16 +-
 drivers/usb/host/xhci.h  |   1 +
 4 files changed, 80 insertions(+), 67 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 670c3ac..0e23775 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -1049,7 +1049,8 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, 
u16 wValue,
goto error;
wIndex--;
temp = readl(port_array[wIndex]);
-   if (temp == 0x) {
+   if (temp == ~(u32)0) {
+   xhci_hc_died(xhci);
retval = -ENODEV;
break;
}
@@ -1091,7 +1092,8 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, 
u16 wValue,
goto error;
wIndex--;
temp = readl(port_array[wIndex]);
-   if (temp == 0x) {
+   if (temp == ~(u32)0) {
+   xhci_hc_died(xhci);
retval = -ENODEV;
break;
}
@@ -1266,7 +1268,8 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, 
u16 wValue,
goto error;
wIndex--;
temp = readl(port_array[wIndex]);
-   if (temp == 0x) {
+   if (temp == ~(u32)0) {
+   xhci_hc_died(xhci);
retval = -ENODEV;
break;
}
@@ -1377,7 +1380,8 @@ int xhci_hub_status_data(struct usb_hcd *hcd, char *buf)
/* For each port, did anything change?  If so, set that bit in buf. */
for (i = 0; i < max_ports; i++) {
temp = readl(port_array[i]);
-   if (temp == 0x) {
+   if (temp == ~(u32)0) {
+   xhci_hc_died(xhci);
retval = -ENODEV;
break;
}
diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index d45f533..c8910fd 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -359,21 +359,19 @@ static int xhci_abort_cmd_ring(struct xhci_hcd *xhci, 
unsigned long flags)
xhci_write_64(xhci, temp_64 | CMD_RING_ABORT,
>op_regs->cmd_ring);
 
-   /* Section 4.6.1.2 of xHCI 1.0 spec says software should
-* time the completion od all xHCI commands, including
-* the Command Abort operation. If software doesn't see
-* CRR negated in a timely manner (e.g. longer than 5
-* seconds), then it should assume that the there are
-* larger problems with the xHC and assert HCRST.
+   /* Section 4.6.1.2 of xHCI 1.0 spec says software should also time the
+* completion of the Command Abort operation. If CRR is not negated in 5
+* seconds then driver handles it as if host died (-ENODEV).
+* In the future we should distinguish between -ENODEV and -ETIMEDOUT
+* and try to recover a -ETIMEDOUT with a host controller reset.
 */
ret = xhci_handshake(>op_regs->cmd_ring,
CMD_RING_RUNNING, 0, 5 * 1000 * 1000);
if (ret < 0) {
-   xhci_err(xhci,
-"Stop command ring failed, maybe the host is dead\n");
-   xhci->xhc_state |= XHCI_STATE_DYING;
+   xhci_err(xhci, "Abort failed to stop command ring: %d\n", ret);
xhci_halt(xhci);
-   return -ESHUTDOWN;
+   xhci_hc_died(xhci);
+   return ret;
}
/*
 * Writing the CMD_RING_ABORT bit should cause a cmd completion event,
@@ -873,6 +871,40 @@ static void xhci_kill_endpoint_urbs(struct xhci_hcd *xhci,
}
 }
 
+/*
+ * host controller died, register read returns 0x
+ * Complete pending commands, mark them ABORTED.
+ * URBs need to be given back as usb core might be waiting with device locks
+ * held for the URBs to finish during device disconnect, blocking host remove.
+ *
+ * Call with xhci->lock held.
+ 

[PATCH 15/24] usb: xhci: remove xhci_dbg_ep_rings()

[Mathias Nyman]

From: Lu Baolu 

xhci_dbg_ep_rings() isn't used in xhci driver anymore. Remove
it to reduce the module binary size.

Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-dbg.c | 24 
 drivers/usb/host/xhci.h |  3 ---
 2 files changed, 27 deletions(-)

diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index 4cfdd51..f6d3031 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -381,30 +381,6 @@ void xhci_debug_ring(struct xhci_hcd *xhci, struct 
xhci_ring *ring)
xhci_debug_segment(xhci, seg);
 }
 
-void xhci_dbg_ep_rings(struct xhci_hcd *xhci,
-   unsigned int slot_id, unsigned int ep_index,
-   struct xhci_virt_ep *ep)
-{
-   int i;
-   struct xhci_ring *ring;
-
-   if (ep->ep_state & EP_HAS_STREAMS) {
-   for (i = 1; i < ep->stream_info->num_streams; i++) {
-   ring = ep->stream_info->stream_rings[i];
-   xhci_dbg(xhci, "Dev %d endpoint %d stream ID %d:\n",
-   slot_id, ep_index, i);
-   xhci_debug_segment(xhci, ring->deq_seg);
-   }
-   } else {
-   ring = ep->ring;
-   if (!ring)
-   return;
-   xhci_dbg(xhci, "Dev %d endpoint ring %d:\n",
-   slot_id, ep_index);
-   xhci_debug_segment(xhci, ring->deq_seg);
-   }
-}
-
 void xhci_dbg_erst(struct xhci_hcd *xhci, struct xhci_erst *erst)
 {
u64 addr = erst->erst_dma_addr;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 3818504..5d771e3 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -1933,9 +1933,6 @@ static inline int xhci_link_trb_quirk(struct xhci_hcd 
*xhci)
 void xhci_dbg_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx *ctx, 
unsigned int last_ep);
 char *xhci_get_slot_state(struct xhci_hcd *xhci,
struct xhci_container_ctx *ctx);
-void xhci_dbg_ep_rings(struct xhci_hcd *xhci,
-   unsigned int slot_id, unsigned int ep_index,
-   struct xhci_virt_ep *ep);
 void xhci_dbg_trace(struct xhci_hcd *xhci, void (*trace)(struct va_format *),
const char *fmt, ...);
 
-- 
1.9.1

--
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 08/24] usb: host: xhci: extract xhci_slot_state_string()

[Mathias Nyman]

From: Felipe Balbi 

By extracting and exposing xhci_slot_state_string() in a header file, we
can re-use it to print Slot Context State from our tracepoints, which
can aid in tracking down problems related to command execution.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-dbg.c | 14 ++
 drivers/usb/host/xhci.h | 16 
 2 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/drivers/usb/host/xhci-dbg.c b/drivers/usb/host/xhci-dbg.c
index 2b4a00f..4cfdd51 100644
--- a/drivers/usb/host/xhci-dbg.c
+++ b/drivers/usb/host/xhci-dbg.c
@@ -451,19 +451,9 @@ char *xhci_get_slot_state(struct xhci_hcd *xhci,
struct xhci_container_ctx *ctx)
 {
struct xhci_slot_ctx *slot_ctx = xhci_get_slot_ctx(xhci, ctx);
+   int state = GET_SLOT_STATE(le32_to_cpu(slot_ctx->dev_state));
 
-   switch (GET_SLOT_STATE(le32_to_cpu(slot_ctx->dev_state))) {
-   case SLOT_STATE_ENABLED:
-   return "enabled/disabled";
-   case SLOT_STATE_DEFAULT:
-   return "default";
-   case SLOT_STATE_ADDRESSED:
-   return "addressed";
-   case SLOT_STATE_CONFIGURED:
-   return "configured";
-   default:
-   return "reserved";
-   }
+   return xhci_slot_state_string(state);
 }
 
 static void xhci_dbg_slot_ctx(struct xhci_hcd *xhci, struct xhci_container_ctx 
*ctx)
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index 675d552..44d031c 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2158,6 +2158,22 @@ static inline struct xhci_ring 
*xhci_urb_to_transfer_ring(struct xhci_hcd *xhci,
urb->stream_id);
 }
 
+static inline char *xhci_slot_state_string(u32 state)
+{
+   switch (state) {
+   case SLOT_STATE_ENABLED:
+   return "enabled/disabled";
+   case SLOT_STATE_DEFAULT:
+   return "default";
+   case SLOT_STATE_ADDRESSED:
+   return "addressed";
+   case SLOT_STATE_CONFIGURED:
+   return "configured";
+   default:
+   return "reserved";
+   }
+}
+
 static inline const char *xhci_decode_trb(u32 field0, u32 field1, u32 field2,
u32 field3)
 {
-- 
1.9.1

--
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 09/24] usb: host: xhci: add Slot and EP Context tracers

[Mathias Nyman]

From: Felipe Balbi 

With these, we can track what's happening with the HW while executing
each and every command. It will give us visibility into how the
different contexts are being modified by xHC which can bring insight
into problems while debugging.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c  |  42 
 drivers/usb/host/xhci-trace.h | 101 +
 drivers/usb/host/xhci.c   |  15 -
 drivers/usb/host/xhci.h   | 146 ++
 4 files changed, 302 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index a3309aa..2f700c9 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -689,6 +689,8 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, 
int slot_id,
struct xhci_virt_ep *ep;
struct xhci_td *cur_td = NULL;
struct xhci_td *last_unlinked_td;
+   struct xhci_ep_ctx *ep_ctx;
+   struct xhci_virt_device *vdev;
 
struct xhci_dequeue_state deq_state;
 
@@ -702,6 +704,11 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, 
int slot_id,
 
memset(_state, 0, sizeof(deq_state));
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3]));
+
+   vdev = xhci->devs[slot_id];
+   ep_ctx = xhci_get_ep_ctx(xhci, vdev->out_ctx, ep_index);
+   trace_xhci_handle_cmd_stop_ep(ep_ctx);
+
ep = >devs[slot_id]->eps[ep_index];
last_unlinked_td = list_last_entry(>cancelled_td_list,
struct xhci_td, cancelled_td_list);
@@ -1029,6 +1036,8 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd 
*xhci, int slot_id,
 
ep_ctx = xhci_get_ep_ctx(xhci, dev->out_ctx, ep_index);
slot_ctx = xhci_get_slot_ctx(xhci, dev->out_ctx);
+   trace_xhci_handle_cmd_set_deq(slot_ctx);
+   trace_xhci_handle_cmd_set_deq_ep(ep_ctx);
 
if (cmd_comp_code != COMP_SUCCESS) {
unsigned int ep_state;
@@ -1099,9 +1108,15 @@ static void xhci_handle_cmd_set_deq(struct xhci_hcd 
*xhci, int slot_id,
 static void xhci_handle_cmd_reset_ep(struct xhci_hcd *xhci, int slot_id,
union xhci_trb *trb, u32 cmd_comp_code)
 {
+   struct xhci_virt_device *vdev;
+   struct xhci_ep_ctx *ep_ctx;
unsigned int ep_index;
 
ep_index = TRB_TO_EP_INDEX(le32_to_cpu(trb->generic.field[3]));
+   vdev = xhci->devs[slot_id];
+   ep_ctx = xhci_get_ep_ctx(xhci, vdev->out_ctx, ep_index);
+   trace_xhci_handle_cmd_reset_ep(ep_ctx);
+
/* This command will only fail if the endpoint wasn't halted,
 * but we don't care.
 */
@@ -1143,10 +1158,15 @@ static void xhci_handle_cmd_enable_slot(struct xhci_hcd 
*xhci, int slot_id,
 static void xhci_handle_cmd_disable_slot(struct xhci_hcd *xhci, int slot_id)
 {
struct xhci_virt_device *virt_dev;
+   struct xhci_slot_ctx *slot_ctx;
 
virt_dev = xhci->devs[slot_id];
if (!virt_dev)
return;
+
+   slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->out_ctx);
+   trace_xhci_handle_cmd_disable_slot(slot_ctx);
+
if (xhci->quirks & XHCI_EP_LIMIT_QUIRK)
/* Delete default control endpoint resources */
xhci_free_device_endpoint_resources(xhci, virt_dev, true);
@@ -1158,6 +1178,7 @@ static void xhci_handle_cmd_config_ep(struct xhci_hcd 
*xhci, int slot_id,
 {
struct xhci_virt_device *virt_dev;
struct xhci_input_control_ctx *ctrl_ctx;
+   struct xhci_ep_ctx *ep_ctx;
unsigned int ep_index;
unsigned int ep_state;
u32 add_flags, drop_flags;
@@ -1182,6 +1203,9 @@ static void xhci_handle_cmd_config_ep(struct xhci_hcd 
*xhci, int slot_id,
/* Input ctx add_flags are the endpoint index plus one */
ep_index = xhci_last_valid_endpoint(add_flags) - 1;
 
+   ep_ctx = xhci_get_ep_ctx(xhci, virt_dev->out_ctx, ep_index);
+   trace_xhci_handle_cmd_config_ep(ep_ctx);
+
/* A usb_set_interface() call directly after clearing a halted
 * condition may race on this quirky hardware.  Not worth
 * worrying about, since this is prototype hardware.  Not sure
@@ -1206,9 +1230,26 @@ static void xhci_handle_cmd_config_ep(struct xhci_hcd 
*xhci, int slot_id,
return;
 }
 
+static void xhci_handle_cmd_addr_dev(struct xhci_hcd *xhci, int slot_id)
+{
+   struct xhci_virt_device *vdev;
+   struct xhci_slot_ctx *slot_ctx;
+
+   vdev = xhci->devs[slot_id];
+   slot_ctx = xhci_get_slot_ctx(xhci, vdev->out_ctx);
+   trace_xhci_handle_cmd_addr_dev(slot_ctx);
+}
+
 static void xhci_handle_cmd_reset_dev(struct xhci_hcd *xhci, int slot_id,
struct xhci_event_cmd *event)
 {
+   struct xhci_virt_device *vdev;
+   struct xhci_slot_ctx *slot_ctx;
+
+   

[PATCH 10/24] usb: host: xhci: fix up Control Transfer TRB decoder

[Mathias Nyman]

From: Felipe Balbi 

Format for each TRB in each control transfer stage differs. Let's make
sure we correctly pretty print these fields to avoid confusion.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.h | 59 +++--
 1 file changed, 37 insertions(+), 22 deletions(-)

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index cf79c1f..52153e1 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2217,31 +2217,46 @@ static inline const char *xhci_decode_trb(u32 field0, 
u32 field1, u32 field2,
 
break;
case TRB_SETUP:
-   sprintf(str,
-   "bRequestType %02x bRequest %02x wValue %02x%02x wIndex 
%02x%02x wLength %d length %d TD size %d intr %d type '%s' flags 
%c:%c:%c:%c:%c:%c:%c:%c",
-   field0 & 0xff,
-   (field0 & 0xff00) >> 8,
-   (field0 & 0xff00) >> 24,
-   (field0 & 0xff) >> 16,
-   (field1 & 0xff00) >> 8,
-   field1 & 0xff,
-   (field1 & 0xff00) >> 16 |
-   (field1 & 0xff) >> 16,
-   TRB_LEN(field2), GET_TD_SIZE(field2),
-   GET_INTR_TARGET(field2),
-   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
-   field3 & TRB_BEI ? 'B' : 'b',
-   field3 & TRB_IDT ? 'I' : 'i',
-   field3 & TRB_IOC ? 'I' : 'i',
-   field3 & TRB_CHAIN ? 'C' : 'c',
-   field3 & TRB_NO_SNOOP ? 'S' : 's',
-   field3 & TRB_ISP ? 'I' : 'i',
-   field3 & TRB_ENT ? 'E' : 'e',
-   field3 & TRB_CYCLE ? 'C' : 'c');
+   sprintf(str, "bRequestType %02x bRequest %02x wValue %02x%02x 
wIndex %02x%02x wLength %d length %d TD size %d intr %d type '%s' flags 
%c:%c:%c",
+   field0 & 0xff,
+   (field0 & 0xff00) >> 8,
+   (field0 & 0xff00) >> 24,
+   (field0 & 0xff) >> 16,
+   (field1 & 0xff00) >> 8,
+   field1 & 0xff,
+   (field1 & 0xff00) >> 16 |
+   (field1 & 0xff) >> 16,
+   TRB_LEN(field2), GET_TD_SIZE(field2),
+   GET_INTR_TARGET(field2),
+   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   field3 & TRB_IDT ? 'I' : 'i',
+   field3 & TRB_IOC ? 'I' : 'i',
+   field3 & TRB_CYCLE ? 'C' : 'c');
break;
-   case TRB_NORMAL:
case TRB_DATA:
+   sprintf(str, "Buffer %08x%08x length %d TD size %d intr %d type 
'%s' flags %c:%c:%c:%c:%c:%c:%c",
+   field1, field0, TRB_LEN(field2), 
GET_TD_SIZE(field2),
+   GET_INTR_TARGET(field2),
+   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   field3 & TRB_IDT ? 'I' : 'i',
+   field3 & TRB_IOC ? 'I' : 'i',
+   field3 & TRB_CHAIN ? 'C' : 'c',
+   field3 & TRB_NO_SNOOP ? 'S' : 's',
+   field3 & TRB_ISP ? 'I' : 'i',
+   field3 & TRB_ENT ? 'E' : 'e',
+   field3 & TRB_CYCLE ? 'C' : 'c');
+   break;
case TRB_STATUS:
+   sprintf(str, "Buffer %08x%08x length %d TD size %d intr %d type 
'%s' flags %c:%c:%c:%c",
+   field1, field0, TRB_LEN(field2), 
GET_TD_SIZE(field2),
+   GET_INTR_TARGET(field2),
+   xhci_trb_type_string(TRB_FIELD_TO_TYPE(field3)),
+   field3 & TRB_IOC ? 'I' : 'i',
+   field3 & TRB_CHAIN ? 'C' : 'c',
+   field3 & TRB_ENT ? 'E' : 'e',
+   field3 & TRB_CYCLE ? 'C' : 'c');
+   break;
+   case TRB_NORMAL:
case TRB_ISOC:
case TRB_EVENT_DATA:
case TRB_TR_NOOP:
-- 
1.9.1

--
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 07/24] usb: host: xhci: print device slot from URB tracers

[Mathias Nyman]

From: Felipe Balbi 

This will help us figuring out which device $this URB belongs to while
debugging.

Signed-off-by: Felipe Balbi 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-trace.h | 8 +---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/host/xhci-trace.h b/drivers/usb/host/xhci-trace.h
index 1ac2cdf..9d80366 100644
--- a/drivers/usb/host/xhci-trace.h
+++ b/drivers/usb/host/xhci-trace.h
@@ -231,6 +231,7 @@
__field(int, epnum)
__field(int, dir_in)
__field(int, type)
+   __field(int, slot_id)
),
TP_fast_assign(
__entry->urb = urb;
@@ -245,8 +246,9 @@
__entry->epnum = usb_endpoint_num(>ep->desc);
__entry->dir_in = usb_endpoint_dir_in(>ep->desc);
__entry->type = usb_endpoint_type(>ep->desc);
+   __entry->slot_id = urb->dev->slot_id;
),
-   TP_printk("ep%d%s-%s: urb %p pipe %u length %d/%d sgs %d/%d stream %d 
flags %08x",
+   TP_printk("ep%d%s-%s: urb %p pipe %u slot %d length %d/%d sgs %d/%d 
stream %d flags %08x",
__entry->epnum, __entry->dir_in ? "in" : "out",
({ char *s;
switch (__entry->type) {
@@ -264,8 +266,8 @@
break;
default:
s = "UNKNOWN";
-   } s; }), __entry->urb, __entry->pipe, __entry->actual,
-   __entry->length, __entry->num_mapped_sgs,
+   } s; }), __entry->urb, __entry->pipe, __entry->slot_id,
+   __entry->actual, __entry->length, 
__entry->num_mapped_sgs,
__entry->num_sgs, __entry->stream, __entry->flags
)
 );
-- 
1.9.1

--
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 06/24] usb: xhci: Add port test modes support for usb2.

[Mathias Nyman]

From: Guoqing Zhang 

For usb2 ports, the port test mode Test_J_State, Test_K_State,
Test_Packet, Test_SE0_NAK and Test_Force_En can be enabled
as described in usb2 spec.

USB2 test mode is a required hardware feature for system integrators
validating their hardware according to USB spec, regarding signal
strength and stuff. It is purely a hardware test feature.

Usually you need an oscilloscope and have to enable those test modes on
the hardware. This will send some specific test patterns on D+/D-. There
is no report available (in Linux itself) as it is purely externally
visible. Regular USB usage is not possible at that time.
Anyone (well access to e.g. /dev/bus/usb/001/001 provided) can use it by
sending appropriate USB_PORT_FEAT_TEST requests to the hub.

[Add better commit message by Alexander Stein  -Mathias]
Signed-off-by: Guoqing Zhang 
Cc: Alexander Stein 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c | 85 +
 drivers/usb/host/xhci.h |  2 ++
 2 files changed, 87 insertions(+)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index b8ea7c0..670c3ac 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -582,6 +582,77 @@ static void xhci_set_port_power(struct xhci_hcd *xhci, 
struct usb_hcd *hcd,
spin_lock_irqsave(>lock, flags);
 }
 
+static void xhci_port_set_test_mode(struct xhci_hcd *xhci,
+   u16 test_mode, u16 wIndex)
+{
+   u32 temp;
+   __le32 __iomem *addr;
+
+   /* xhci only supports test mode for usb2 ports, i.e. xhci->main_hcd */
+   addr = xhci_get_port_io_addr(xhci->main_hcd, wIndex);
+   temp = readl(addr + PORTPMSC);
+   temp |= test_mode << PORT_TEST_MODE_SHIFT;
+   writel(temp, addr + PORTPMSC);
+   xhci->test_mode = test_mode;
+   if (test_mode == TEST_FORCE_EN)
+   xhci_start(xhci);
+}
+
+static int xhci_enter_test_mode(struct xhci_hcd *xhci,
+   u16 test_mode, u16 wIndex)
+{
+   int i, retval;
+
+   /* Disable all Device Slots */
+   xhci_dbg(xhci, "Disable all slots\n");
+   for (i = 1; i <= HCS_MAX_SLOTS(xhci->hcs_params1); i++) {
+   retval = xhci_disable_slot(xhci, NULL, i);
+   if (retval)
+   xhci_err(xhci, "Failed to disable slot %d, %d. Enter 
test mode anyway\n",
+i, retval);
+   }
+   /* Put all ports to the Disable state by clear PP */
+   xhci_dbg(xhci, "Disable all port (PP = 0)\n");
+   /* Power off USB3 ports*/
+   for (i = 0; i < xhci->num_usb3_ports; i++)
+   xhci_set_port_power(xhci, xhci->shared_hcd, i, false);
+   /* Power off USB2 ports*/
+   for (i = 0; i < xhci->num_usb2_ports; i++)
+   xhci_set_port_power(xhci, xhci->main_hcd, i, false);
+   /* Stop the controller */
+   xhci_dbg(xhci, "Stop controller\n");
+   retval = xhci_halt(xhci);
+   if (retval)
+   return retval;
+   /* Disable runtime PM for test mode */
+   pm_runtime_forbid(xhci_to_hcd(xhci)->self.controller);
+   /* Set PORTPMSC.PTC field to enter selected test mode */
+   /* Port is selected by wIndex. port_id = wIndex + 1 */
+   xhci_dbg(xhci, "Enter Test Mode: %d, Port_id=%d\n",
+   test_mode, wIndex + 1);
+   xhci_port_set_test_mode(xhci, test_mode, wIndex);
+   return retval;
+}
+
+static int xhci_exit_test_mode(struct xhci_hcd *xhci)
+{
+   int retval;
+
+   if (!xhci->test_mode) {
+   xhci_err(xhci, "Not in test mode, do nothing.\n");
+   return 0;
+   }
+   if (xhci->test_mode == TEST_FORCE_EN &&
+   !(xhci->xhc_state & XHCI_STATE_HALTED)) {
+   retval = xhci_halt(xhci);
+   if (retval)
+   return retval;
+   }
+   pm_runtime_allow(xhci_to_hcd(xhci)->self.controller);
+   xhci->test_mode = 0;
+   return xhci_reset(xhci);
+}
+
 void xhci_set_link_state(struct xhci_hcd *xhci, __le32 __iomem **port_array,
int port_id, u32 link_state)
 {
@@ -937,6 +1008,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, u16 
wValue,
u16 link_state = 0;
u16 wake_mask = 0;
u16 timeout = 0;
+   u16 test_mode = 0;
 
max_ports = xhci_get_ports(hcd, _array);
bus_state = >bus_state[hcd_index(hcd)];
@@ -1010,6 +1082,8 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, 
u16 wValue,
link_state = (wIndex & 0xff00) >> 3;
if (wValue == USB_PORT_FEAT_REMOTE_WAKE_MASK)
wake_mask = wIndex & 0xff00;
+   if (wValue == USB_PORT_FEAT_TEST)
+   test_mode = (wIndex & 0xff00) >> 8;
   

[PATCH 11/24] xhci: add slot and endpoint numbers to debug messages in handle_tx_event

[Mathias Nyman]

From: Zhengjun Xing 

There's one annoyance in how xhci prints debug messages, we often
get logs with messages but it's hard to say from which device and
endpoint the message originates. Add slot_id, ep_index messages
in handle_tx_event.

Signed-off-by: Zhengjun Xing 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-ring.c | 80 ++--
 1 file changed, 55 insertions(+), 25 deletions(-)

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index 2f700c9..d45f533 100644
--- a/drivers/usb/host/xhci-ring.c
+++ b/drivers/usb/host/xhci-ring.c
@@ -2285,7 +2285,8 @@ static int handle_tx_event(struct xhci_hcd *xhci,
slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->flags));
xdev = xhci->devs[slot_id];
if (!xdev) {
-   xhci_err(xhci, "ERROR Transfer event pointed to bad slot\n");
+   xhci_err(xhci, "ERROR Transfer event pointed to bad slot %u\n",
+slot_id);
xhci_err(xhci, "@%016llx %08x %08x %08x %08x\n",
 (unsigned long long) xhci_trb_virt_to_dma(
 xhci->event_ring->deq_seg,
@@ -2305,8 +2306,9 @@ static int handle_tx_event(struct xhci_hcd *xhci,
ep_ring = xhci_dma_to_transfer_ring(ep, le64_to_cpu(event->buffer));
ep_ctx = xhci_get_ep_ctx(xhci, xdev->out_ctx, ep_index);
if (!ep_ring ||  GET_EP_CTX_STATE(ep_ctx) == EP_STATE_DISABLED) {
-   xhci_err(xhci, "ERROR Transfer event for disabled endpoint "
-   "or incorrect stream ring\n");
+   xhci_err(xhci,
+"ERROR Transfer event for disabled endpoint slot %u ep 
%u or incorrect stream ring\n",
+ slot_id, ep_index);
xhci_err(xhci, "@%016llx %08x %08x %08x %08x\n",
 (unsigned long long) xhci_trb_virt_to_dma(
 xhci->event_ring->deq_seg,
@@ -2340,45 +2342,62 @@ static int handle_tx_event(struct xhci_hcd *xhci,
trb_comp_code = COMP_SHORT_PACKET;
else
xhci_warn_ratelimited(xhci,
-   "WARN Successful completion on short 
TX: needs XHCI_TRUST_TX_LENGTH quirk?\n");
+ "WARN Successful completion on 
short TX for slot %u ep %u: needs XHCI_TRUST_TX_LENGTH quirk?\n",
+ slot_id, ep_index);
case COMP_SHORT_PACKET:
break;
case COMP_STOPPED:
-   xhci_dbg(xhci, "Stopped on Transfer TRB\n");
+   xhci_dbg(xhci, "Stopped on Transfer TRB for slot %u ep %u\n",
+slot_id, ep_index);
break;
case COMP_STOPPED_LENGTH_INVALID:
-   xhci_dbg(xhci, "Stopped on No-op or Link TRB\n");
+   xhci_dbg(xhci,
+"Stopped on No-op or Link TRB for slot %u ep %u\n",
+slot_id, ep_index);
break;
case COMP_STOPPED_SHORT_PACKET:
-   xhci_dbg(xhci, "Stopped with short packet transfer detected\n");
+   xhci_dbg(xhci,
+"Stopped with short packet transfer detected for slot 
%u ep %u\n",
+slot_id, ep_index);
break;
case COMP_STALL_ERROR:
-   xhci_dbg(xhci, "Stalled endpoint\n");
+   xhci_dbg(xhci, "Stalled endpoint for slot %u ep %u\n", slot_id,
+ep_index);
ep->ep_state |= EP_HALTED;
status = -EPIPE;
break;
case COMP_TRB_ERROR:
-   xhci_warn(xhci, "WARN: TRB error on endpoint\n");
+   xhci_warn(xhci,
+ "WARN: TRB error for slot %u ep %u on endpoint\n",
+ slot_id, ep_index);
status = -EILSEQ;
break;
case COMP_SPLIT_TRANSACTION_ERROR:
case COMP_USB_TRANSACTION_ERROR:
-   xhci_dbg(xhci, "Transfer error on endpoint\n");
+   xhci_dbg(xhci, "Transfer error for slot %u ep %u on endpoint\n",
+slot_id, ep_index);
status = -EPROTO;
break;
case COMP_BABBLE_DETECTED_ERROR:
-   xhci_dbg(xhci, "Babble error on endpoint\n");
+   xhci_dbg(xhci, "Babble error for slot %u ep %u on endpoint\n",
+slot_id, ep_index);
status = -EOVERFLOW;
break;
case COMP_DATA_BUFFER_ERROR:
-   xhci_warn(xhci, "WARN: HC couldn't access mem fast enough\n");
+   xhci_warn(xhci,
+ "WARN: HC couldn't access mem fast enough for slot %u 
ep %u\n",
+   

[PATCH 05/24] usb: xhci: Expose xhci_start() function.

[Mathias Nyman]

From: Guoqing Zhang 

Change the visability of xhci_start() so that it
can be used when enabling test mode.

Signed-off-by: Guoqing Zhang 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.c | 2 +-
 drivers/usb/host/xhci.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 8cfafdc..7b9d3ad 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -125,7 +125,7 @@ int xhci_halt(struct xhci_hcd *xhci)
 /*
  * Set the run bit and wait for the host to be running.
  */
-static int xhci_start(struct xhci_hcd *xhci)
+int xhci_start(struct xhci_hcd *xhci)
 {
u32 temp;
int ret;
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index d3485f8..4e12c8c 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2010,6 +2010,7 @@ void xhci_free_command(struct xhci_hcd *xhci,
 int xhci_handshake(void __iomem *ptr, u32 mask, u32 done, int usec);
 void xhci_quiesce(struct xhci_hcd *xhci);
 int xhci_halt(struct xhci_hcd *xhci);
+int xhci_start(struct xhci_hcd *xhci);
 int xhci_reset(struct xhci_hcd *xhci);
 int xhci_init(struct usb_hcd *hcd);
 int xhci_run(struct usb_hcd *hcd);
-- 
1.9.1

--
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 04/24] usb: xhci: Add helper function xhci_disable_slot().

[Mathias Nyman]

From: Guoqing Zhang 

Refactoring slot disable related code into a helper
function xhci_disable_slot() which can be used when
enabling test mode.

Signed-off-by: Guoqing Zhang 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.c | 49 +++--
 drivers/usb/host/xhci.h |  2 ++
 2 files changed, 33 insertions(+), 18 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index e3c7856..8cfafdc 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3561,8 +3561,6 @@ void xhci_free_dev(struct usb_hcd *hcd, struct usb_device 
*udev)
 {
struct xhci_hcd *xhci = hcd_to_xhci(hcd);
struct xhci_virt_device *virt_dev;
-   unsigned long flags;
-   u32 state;
int i, ret;
struct xhci_command *command;
 
@@ -3597,30 +3595,50 @@ void xhci_free_dev(struct usb_hcd *hcd, struct 
usb_device *udev)
del_timer_sync(_dev->eps[i].stop_cmd_timer);
}
 
+   xhci_disable_slot(xhci, command, udev->slot_id);
+   /*
+* Event command completion handler will free any data structures
+* associated with the slot.  XXX Can free sleep?
+*/
+}
+
+int xhci_disable_slot(struct xhci_hcd *xhci, struct xhci_command *command,
+   u32 slot_id)
+{
+   unsigned long flags;
+   u32 state;
+   int ret = 0;
+   struct xhci_virt_device *virt_dev;
+
+   virt_dev = xhci->devs[slot_id];
+   if (!virt_dev)
+   return -EINVAL;
+   if (!command)
+   command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
+   if (!command)
+   return -ENOMEM;
+
spin_lock_irqsave(>lock, flags);
/* Don't disable the slot if the host controller is dead. */
state = readl(>op_regs->status);
if (state == 0x || (xhci->xhc_state & XHCI_STATE_DYING) ||
(xhci->xhc_state & XHCI_STATE_HALTED)) {
-   xhci_free_virt_device(xhci, udev->slot_id);
+   xhci_free_virt_device(xhci, slot_id);
spin_unlock_irqrestore(>lock, flags);
kfree(command);
-   return;
+   return ret;
}
 
-   if (xhci_queue_slot_control(xhci, command, TRB_DISABLE_SLOT,
-   udev->slot_id)) {
+   ret = xhci_queue_slot_control(xhci, command, TRB_DISABLE_SLOT,
+   slot_id);
+   if (ret) {
spin_unlock_irqrestore(>lock, flags);
xhci_dbg(xhci, "FIXME: allocate a command ring segment\n");
-   return;
+   return ret;
}
xhci_ring_cmd_db(xhci);
spin_unlock_irqrestore(>lock, flags);
-
-   /*
-* Event command completion handler will free any data structures
-* associated with the slot.  XXX Can free sleep?
-*/
+   return ret;
 }
 
 /*
@@ -3727,15 +3745,10 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct 
usb_device *udev)
 
 disable_slot:
/* Disable slot, if we can do it without mem alloc */
-   spin_lock_irqsave(>lock, flags);
kfree(command->completion);
command->completion = NULL;
command->status = 0;
-   if (!xhci_queue_slot_control(xhci, command, TRB_DISABLE_SLOT,
-udev->slot_id))
-   xhci_ring_cmd_db(xhci);
-   spin_unlock_irqrestore(>lock, flags);
-   return 0;
+   return xhci_disable_slot(xhci, command, udev->slot_id);
 }
 
 /*
diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index da3eb69..d3485f8 100644
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2018,6 +2018,8 @@ void xhci_free_command(struct xhci_hcd *xhci,
 int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks);
 void xhci_init_driver(struct hc_driver *drv,
  const struct xhci_driver_overrides *over);
+int xhci_disable_slot(struct xhci_hcd *xhci,
+   struct xhci_command *command, u32 slot_id);
 
 #ifdef CONFIG_PM
 int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup);
-- 
1.9.1

--
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 00/24] xhci changes and fetures for usb-next

[Mathias Nyman]

Hi Greg

Among other small changes this series adds usb2 port test mode for xhci,
reworks how we handle unresponsive (hotplug removed) hosts,
and shifts debugging more towards tracing.

-Mathias

Andrew Bresticker (1):
  usb: xhci: plat: Enable async suspend/resume

Felipe Balbi (4):
  usb: host: xhci: print device slot from URB tracers
  usb: host: xhci: extract xhci_slot_state_string()
  usb: host: xhci: add Slot and EP Context tracers
  usb: host: xhci: fix up Control Transfer TRB decoder

Guoqing Zhang (4):
  usb: xhci: Add helper function xhci_set_power_on().
  usb: xhci: Add helper function xhci_disable_slot().
  usb: xhci: Expose xhci_start() function.
  usb: xhci: Add port test modes support for usb2.

Joel Stanley (1):
  xhci: Do not halt the host until both HCD have disconnected their
devices.

Lu Baolu (11):
  usb: xhci: clear EINT bit in status correctly
  usb: xhci: add xhci_log_ring trace events
  usb: xhci: remove xhci_dbg_ep_rings()
  usb: xhci: make several functions static
  usb: xhci: remove error messages for failed memory allocation
  usb: xhci: remove enq_updates and deq_updates from ring
  usb: xhci: remove ring debugging code
  usb: xhci: remove xhci_debug_trb()
  usb: xhci: remove xhci_dbg_ctx()
  usb: xhci: fix link trb decoding
  usb: xhci: refine xhci_decode_trb()

Mathias Nyman (1):
  xhci: Rework how we handle unresponsive or hoptlug removed hosts

Roger Quadros (1):
  usb: xhci: bInterval quirk for TI TUSB73x0

Zhengjun Xing (1):
  xhci: add slot and endpoint numbers to debug messages in
handle_tx_event

 drivers/usb/host/xhci-dbg.c   | 308 +--
 drivers/usb/host/xhci-hub.c   | 166 +
 drivers/usb/host/xhci-mem.c   |  19 ++-
 drivers/usb/host/xhci-pci.c   |   3 +
 drivers/usb/host/xhci-plat.c  |   2 +
 drivers/usb/host/xhci-ring.c  | 266 +-
 drivers/usb/host/xhci-trace.h | 174 +-
 drivers/usb/host/xhci.c   | 221 ++--
 drivers/usb/host/xhci.h   | 326 +-
 9 files changed, 830 insertions(+), 655 deletions(-)

-- 
1.9.1

--
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 02/24] usb: xhci: clear EINT bit in status correctly

[Mathias Nyman]

From: Lu Baolu 

EINT(Event Interrupt) is a write-1-to-clear type of bit in xhci
status register. It should be cleared by writing a 1. Writing 0
to this bit has no effect.

Xhci driver tries to clear this bit by writing 0 to it. This is
not the right way to go. This patch corrects this by reading the
register first, then clearing all RO/RW1C/RsvZ bits and setting
the clearing bit, and writing back the new value at last.

Xhci spec requires that software that uses EINT shall clear it
prior to clearing any IP flags in section 5.4.2. This is the
reason why this patch is CC'ed stable as well.

[old way didn't cause any issues, skip stable, send to next -Mathias]

Cc: Felipe Balbi 
Signed-off-by: Lu Baolu 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 643e31d..e3c7856 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -724,7 +724,7 @@ void xhci_stop(struct usb_hcd *hcd)
xhci_dbg_trace(xhci, trace_xhci_dbg_init,
"// Disabling event ring interrupts");
temp = readl(>op_regs->status);
-   writel(temp & ~STS_EINT, >op_regs->status);
+   writel((temp & ~0x1fff) | STS_EINT, >op_regs->status);
temp = readl(>ir_set->irq_pending);
writel(ER_IRQ_DISABLE(temp), >ir_set->irq_pending);
xhci_print_ir_set(xhci, 0);
@@ -1057,7 +1057,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated)
 
xhci_dbg(xhci, "// Disabling event ring interrupts\n");
temp = readl(>op_regs->status);
-   writel(temp & ~STS_EINT, >op_regs->status);
+   writel((temp & ~0x1fff) | STS_EINT, >op_regs->status);
temp = readl(>ir_set->irq_pending);
writel(ER_IRQ_DISABLE(temp), >ir_set->irq_pending);
xhci_print_ir_set(xhci, 0);
-- 
1.9.1

--
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 01/24] usb: xhci: plat: Enable async suspend/resume

[Mathias Nyman]

From: Andrew Bresticker 

USB host controllers can take a significant amount of time to suspend
and resume, adding several hundred miliseconds to the kernel resume
time. Since the XHCI controller has no outside dependencies (other than
clocks, which are suspended late/resumed early), allow it to suspend and
resume asynchronously.

Signed-off-by: Andrew Bresticker 
Tested-by: Andrew Bresticker 
Tested-by: Robert Foss 
Signed-off-by: Robert Foss 
Reviewed-by: Baolin Wang 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-plat.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/host/xhci-plat.c b/drivers/usb/host/xhci-plat.c
index f31b9dc..62e71b8 100644
--- a/drivers/usb/host/xhci-plat.c
+++ b/drivers/usb/host/xhci-plat.c
@@ -277,6 +277,8 @@ static int xhci_plat_probe(struct platform_device *pdev)
if (ret)
goto dealloc_usb2_hcd;
 
+   device_enable_async_suspend(>dev);
+
return 0;
 
 
-- 
1.9.1

--
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 03/24] usb: xhci: Add helper function xhci_set_power_on().

[Mathias Nyman]

From: Guoqing Zhang 

Refactoring port power on/off related code into
a helper function xhci_set_power_on() which can
be reused when enabling test mode.

Signed-off-by: Guoqing Zhang 
Signed-off-by: Mathias Nyman 
---
 drivers/usb/host/xhci-hub.c | 65 ++---
 1 file changed, 44 insertions(+), 21 deletions(-)

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index 3bddeaa..b8ea7c0 100644
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -540,6 +540,48 @@ static int xhci_get_ports(struct usb_hcd *hcd, __le32 
__iomem ***port_array)
return max_ports;
 }
 
+static __le32 __iomem *xhci_get_port_io_addr(struct usb_hcd *hcd, int index)
+{
+   __le32 __iomem **port_array;
+
+   xhci_get_ports(hcd, _array);
+   return port_array[index];
+}
+
+/*
+ * xhci_set_port_power() must be called with xhci->lock held.
+ * It will release and re-aquire the lock while calling ACPI
+ * method.
+ */
+static void xhci_set_port_power(struct xhci_hcd *xhci, struct usb_hcd *hcd,
+   u16 index, bool on)
+{
+   __le32 __iomem *addr;
+   u32 temp;
+   unsigned long flags = 0;
+
+   addr = xhci_get_port_io_addr(hcd, index);
+   temp = readl(addr);
+   if (on) {
+   /* Power on */
+   writel(temp | PORT_POWER, addr);
+   temp = readl(addr);
+   xhci_dbg(xhci, "set port power, actual port %d status  = 
0x%x\n",
+   index, temp);
+   } else {
+   /* Power off */
+   writel(temp & ~PORT_POWER, addr);
+   }
+
+   spin_unlock_irqrestore(>lock, flags);
+   temp = usb_acpi_power_manageable(hcd->self.root_hub,
+   index);
+   if (temp)
+   usb_acpi_set_power_state(hcd->self.root_hub,
+   index, on);
+   spin_lock_irqsave(>lock, flags);
+}
+
 void xhci_set_link_state(struct xhci_hcd *xhci, __le32 __iomem **port_array,
int port_id, u32 link_state)
 {
@@ -1092,18 +1134,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, 
u16 wValue,
 * However, hub_wq will ignore the roothub events until
 * the roothub is registered.
 */
-   writel(temp | PORT_POWER, port_array[wIndex]);
-
-   temp = readl(port_array[wIndex]);
-   xhci_dbg(xhci, "set port power, actual port %d status  
= 0x%x\n", wIndex, temp);
-
-   spin_unlock_irqrestore(>lock, flags);
-   temp = usb_acpi_power_manageable(hcd->self.root_hub,
-   wIndex);
-   if (temp)
-   usb_acpi_set_power_state(hcd->self.root_hub,
-   wIndex, true);
-   spin_lock_irqsave(>lock, flags);
+   xhci_set_port_power(xhci, hcd, wIndex, true);
break;
case USB_PORT_FEAT_RESET:
temp = (temp | PORT_RESET);
@@ -1207,15 +1238,7 @@ int xhci_hub_control(struct usb_hcd *hcd, u16 typeReq, 
u16 wValue,
port_array[wIndex], temp);
break;
case USB_PORT_FEAT_POWER:
-   writel(temp & ~PORT_POWER, port_array[wIndex]);
-
-   spin_unlock_irqrestore(>lock, flags);
-   temp = usb_acpi_power_manageable(hcd->self.root_hub,
-   wIndex);
-   if (temp)
-   usb_acpi_set_power_state(hcd->self.root_hub,
-   wIndex, false);
-   spin_lock_irqsave(>lock, flags);
+   xhci_set_port_power(xhci, hcd, wIndex, false);
break;
default:
goto error;
-- 
1.9.1

--
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 v2 2/4] reset: Add APIs to manage array of resets

[Philipp Zabel]

Hi Vivek,

On Tue, 2017-04-04 at 16:09 +0530, Vivek Gautam wrote:
[...]
> > I'd prefer to mirror the gpiod API a little, and to have the number
> > contained in the array structure, similar to struct gpio_descs:
[...]
> Alright, i can update this.
> I took regulator_bulk interface as the reference, but now i see
> gpio_descs has a smaller footprint.

Ok, understood.
I think the smaller footprint API is more convenient for the user.

[...]
> >> + * Returns 0 on success or error number on failure
> >> + */
> >> +static int reset_control_array_get(struct device *dev, int num_rsts,
> >> +  struct reset_control_array *resets,
> >> +  bool shared, bool optional)
> > Can we make this count and return a pointer to the newly created array?
> >
> > static struct reset_controls *
> > reset_control_array_get(struct device *dev, bool shared, bool optional)
> > {
> > ...
> >
> > That way the consumer doesn't have to care about counting and array
> > allocation.
> 
> Just trying to think again in line with the regulator bulk APIs.
> Can't a user provide a list of reset controls as data and thus
> request reset controls with a "id" and num_resets available.
> 
> e.g.
> const char * const rst_names[] = {
>   "rst1", "rst2" ...
> };
> xyz_data = {
>   .resets = rst_names;
>   .num = ARRAY_SIZE(rst_names);
> };
> and then the driver making use of reset_control_array APIs
> to request this list of reset controls and assert/deassert them.
> 
> I am assuming this case when one user driver is used across a bunch
> of platforms with different number of resets available.
> We may not want to rely solely on the device tree entries, since
> the resets can be mandatory in few cases and we may want to
> return failure.

The way I understood the array API was as a method of handling a list of
anonymous resets, specified as

resets = < 1>, < 2>, < 3>;
// reset-names = "rst1", "rst2", "rst3"; /* don't care */

in the device tree.

Now if you want to handle a partial list of those as an unordered list,
with special consideration for others, that could be added as a separate
API when there is use for it. But I expect that most potential users of
this array API will not have to make such a distinction.

> >> @@ -85,6 +107,39 @@ static inline struct reset_control 
> >> *__devm_reset_control_get(
> >>return -ENOTSUPP;
> >>   }
> >>   
> >> +static inline int reset_control_array_assert(int num_rsts,
> >> +  struct reset_control_array *resets)
> >> +{
> >> +  return 0;
> >> +}
> >> +
> >> +static inline int reset_control_array_deassert(int num_rsts,
> >> +  struct reset_control_array *resets)
> >> +{
> >> +  return 0;
> >> +}
> > Is this missing a stub for reset_control_array_get?
> 
> No, that's supposed to be a static function.

Oh right, it is static. Please ignore.

regards
Philipp


--
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: usb: musb: regression since 4.9 on omap4-panda-es (caused by d8e5f0eca1e8)

[Bin Liu]

On Tue, Apr 04, 2017 at 10:09:50AM +0300, Peter Ujfalusi wrote:
> Tony,
> 
> since 4.9 (4.8 was fine) I can not boot omap4-panda-es if the musb
> is compiled in. The kernel will stuck printing:
> 
> ** 206 printk messages dropped ** [8.926727] musb_bus_suspend
> 2584: trying to suspend as a_idle while active

Does it sound a similar issue to
http://marc.info/?l=linux-usb=149036531809506=2

Regards,
-Bin.

> 
> The bisect (log is [1]) points to:
> d8e5f0eca1e8 usb: musb: Fix hardirq-safe hardirq-unsafe lock order error
> 
> and reverting the d8e5f0eca1e8 makes the board to boot up fine
> (Linux 4.11-rc5 and next-20170331).
> 
> any idea on how to fix this w/o reverting the commit?
> 
> [1] https://pastebin.com/Z2HJY229
> 
> - Péter
--
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: Fw: New Defects reported by Coverity Scan for Linux

[Bjørn Mork]

Oliver Neukum  writes:
> Am Montag, den 03.04.2017, 12:48 -0700 schrieb Stephen Hemminger:
>> Looks like new warnings in usbnet
>> 
>> 
>> ** CID 751368:  Null pointer dereferences  (FORWARD_NULL)
>> /drivers/net/usb/usbnet.c: 1925 in __usbnet_read_cmd()
>
> Hi Stephen,
>
> I am afraid I don't see the problem. Yes, we are passing
> a NULL pointer down, but only if size==0. In that case it
> is allowed.
> Could you shed some light on the meaning of the report?
> I might be overlooking something, but to me it looks
> like a false positive.

I wonder if the problem is the unvalidated assumption that
"data == NULL" implies "size == 0".

We don't actually test for "size == 0".  Maybe we should?



Bjørn
--
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: Ragentek usb-serial driver

[Oliver Neukum]

Am Dienstag, den 04.04.2017, 13:02 +0200 schrieb Alexander Kozyrev:
> $ lsusb -s 1:57 -v
> 
> Bus 001 Device 057: ID 0bb4:0c01 HTC (High Tech Computer Corp.) Dream /
> ADP1 / G1 / Magic / Tattoo
> Device Descriptor:
>   bLength18
>   bDescriptorType 1
>   bcdUSB   2.00
>   bDeviceClass0 (Defined at Interface level)
>   bDeviceSubClass 0 
>   bDeviceProtocol 0 
>   bMaxPacketSize064
>   idVendor   0x0bb4 HTC (High Tech Computer Corp.)
>   idProduct  0x0c01 Dream / ADP1 / G1 / Magic / Tattoo
>   bcdDevice1.00
>   iManufacturer   1 MediaTek
>   iProduct2 Android
>   iSerial 3 X560I1ZR5A241388
>   bNumConfigurations  1
>   Configuration Descriptor:
> bLength 9
> bDescriptorType 2
> wTotalLength   32
> bNumInterfaces  1
> bConfigurationValue 1
> iConfiguration  0 
> bmAttributes 0x80
>   (Bus Powered)
> MaxPower  256mA
> Interface Descriptor:
>   bLength 9
>   bDescriptorType 4
>   bInterfaceNumber0
>   bAlternateSetting   0
>   bNumEndpoints   2
>   bInterfaceClass   255 Vendor Specific Class
>   bInterfaceSubClass 66 
>   bInterfaceProtocol  3 
>   iInterface  4 fastboot
>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x01  EP 1 OUT
> bmAttributes2
>   Transfer TypeBulk
>   Synch Type   None
>   Usage Type   Data
> wMaxPacketSize 0x0200  1x 512 bytes


> bInterval   0
>   Endpoint Descriptor:
> bLength 7
> bDescriptorType 5
> bEndpointAddress 0x81  EP 1 IN
> bmAttributes2
>   Transfer TypeBulk
>   Synch Type   None
>   Usage Type   Data
> wMaxPacketSize 0x0200  1x 512 bytes
> bInterval   1
> Device Status: 0x0001
>   Self Powered
> 
Hi,

the generic driver should bind. Please ramp up the log level
and activate dynamic debugging for it and repost the log

Regards
Oliver

PS: Please don't drop the list from CC#

--
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 2/2] usb: misc: refactor code

[Gustavo A. R. Silva]

Hello,

Quoting Felipe Balbi :


Hi,

"Gustavo A. R. Silva"  writes:

Code refactoring to make the flow easier to follow.

Cc: Alan Stern 
Cc: Greg Kroah-Hartman 
Signed-off-by: Gustavo A. R. Silva 
---
 drivers/usb/misc/usbtest.c | 67  
+-

 1 file changed, 30 insertions(+), 37 deletions(-)

diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
index 7bfb6b78..382491e 100644
--- a/drivers/usb/misc/usbtest.c
+++ b/drivers/usb/misc/usbtest.c
@@ -124,18 +124,32 @@ static struct usb_device  
*testdev_to_usbdev(struct usbtest_dev *test)


  
/*-*/


+static inline void endpoint_update(int edi,
+  struct usb_host_endpoint **in,
+  struct usb_host_endpoint **out,
+  struct usb_host_endpoint *e)
+{
+   if (edi) {
+   if (!*in)
+   *in = e;
+   } else {
+   if (!*out)
+   *out = e;
+   }
+}
+
 static int
 get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)
 {
-   int tmp;
-   struct usb_host_interface   *alt;
-   struct usb_host_endpoint*in, *out;
-   struct usb_host_endpoint*iso_in, *iso_out;
-   struct usb_host_endpoint*int_in, *int_out;
-   struct usb_device   *udev;
+   int tmp;
+   struct usb_host_interface   *alt;
+   struct usb_host_endpoint*in, *out;
+   struct usb_host_endpoint*iso_in, *iso_out;
+   struct usb_host_endpoint*int_in, *int_out;
+   struct usb_device   *udev;


unnecessary change



for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
-   unsignedep;
+   unsignedep;


unnecessary change



in = out = NULL;
iso_in = iso_out = NULL;
@@ -150,48 +164,27 @@ get_endpoints(struct usbtest_dev *dev, struct  
usb_interface *intf)

 * ignore other endpoints and altsettings.
 */
for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) {
-   struct usb_host_endpoint*e;
+   struct usb_host_endpoint*e;


unnecessary change



I already sent the version 2 of this patch: https://lkml.org/lkml/2017/4/3/856

Thanks
--
Gustavo A. R. Silva





--
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: Ragentek usb-serial driver

[Oliver Neukum]

Am Dienstag, den 04.04.2017, 12:36 +0200 schrieb Alexander Kozyrev:
> Hi all!
> 
> Have troubles with Ragentek chip in Mediatek Leagoo Android phone. It's
> tries to connect as ttyACM0 device and ttyUSB0 - unsuccessful.

Hi,

please post

lsusb -v

Regards
Oliver

--
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

Ragentek usb-serial driver

[Alexander Kozyrev]

Hi all!

Have troubles with Ragentek chip in Mediatek Leagoo Android phone. It's
tries to connect as ttyACM0 device and ttyUSB0 - unsuccessful.

[2.219806] usb 1-1: new high-speed USB device number 36 using xhci_hcd
[2.385119] usb 1-1: New USB device found, idVendor=0e8d, idProduct=2008
[2.385125] usb 1-1: New USB device strings: Mfr=2, Product=3,
SerialNumber=4
[2.385128] usb 1-1: Product: j5808
[2.385130] usb 1-1: Manufacturer: Ragentek
[2.385132] usb 1-1: SerialNumber: 0123456789ABCDEF
[3.368257] usb 1-1: USB disconnect, device number 36
[3.742359] usb 1-1: new high-speed USB device number 37 using xhci_hcd
[3.908089] usb 1-1: New USB device found, idVendor=0e8d, idProduct=201c
[3.908095] usb 1-1: New USB device strings: Mfr=2, Product=3,
SerialNumber=4
[3.908097] usb 1-1: Product: j5808


[3.908099] usb 1-1: Manufacturer: Ragentek
[3.908101] usb 1-1: SerialNumber: 0123456789ABCDEF
[6.436388] usb 1-1: USB disconnect, device number 37
[8.691575] usb 1-1: new high-speed USB device number 38 using xhci_hcd
[8.856068] usb 1-1: New USB device found, idVendor=0e8d, idProduct=2000
[8.856073] usb 1-1: New USB device strings: Mfr=1, Product=2,
SerialNumber=0
[8.856077] usb 1-1: Product: MT65xx Preloader
[8.856080] usb 1-1: Manufacturer: MediaTek
[8.878070] usbserial_generic 1-1:1.0: The "generic" usb-serial driver is
only for testing and one-off prototypes.
[8.878074] usbserial_generic 1-1:1.0: Tell linux-usb@vger.kernel.org to
add your device to a proper driver.
[8.878076] usbserial_generic 1-1:1.0: generic converter detected
[8.878333] usb 1-1: generic converter now attached to ttyUSB0
[8.918180] usbserial_generic 1-1:1.1: Generic device with no bulk out,
not allowed.
[8.918191] usbserial_generic: probe of 1-1:1.1 failed with error -5
[8.918207] cdc_acm: probe of 1-1:1.1 failed with error -16
[11.358968] usb 1-1: USB disconnect, device number 38
[11.359186] generic ttyUSB0: generic converter now disconnected from
ttyUSB0
[71.359205] usbserial_generic 1-1:1.0: device disconnected
[73.499064] usb 1-1: new high-speed USB device number 39 using xhci_hcd
[73.663695] usb 1-1: New USB device found, idVendor=0bb4, idProduct=0c01
[73.663716] usb 1-1: New USB device strings: Mfr=1, Product=2,
SerialNumber=3
[73.663718] usb 1-1: Product: Android
[73.663720] usb 1-1: Manufacturer: MediaTek
[73.663722] usb 1-1: SerialNumber: X560I1ZR5A241388

[root@gigabyte flash-kernel]# ll /dev/ttyUSB0
ls: cannot access /dev/ttyUSB0: No such file or directory


Linux gigabyte.postmet.de 3.19.8-100.fc20.i686 #1 SMP Tue May 12
17:42:35 UTC 2015 i686 i686 i386 GNU/Linux
Fedora release 20 (Heisenbug)



--
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 v2 2/4] reset: Add APIs to manage array of resets

[Vivek Gautam]

Hi Philipp,


On 04/03/2017 10:06 PM, Philipp Zabel wrote:

Hi Vivek,

thank you for the patch series. A few comments below: I'd like to reduce
the API surface a bit and include the counting and array allocation in
the _get functions, if possible.


Thank you for reviewing the patch. Please find my comments inline.



On Mon, 2017-04-03 at 19:12 +0530, Vivek Gautam wrote:

Many devices may want to request a bunch of resets
and control them. So it's better to manage them as an
array. Add APIs to _get(), _assert(), and _deassert()
an array of reset_control.

Cc: Philipp Zabel 
Signed-off-by: Vivek Gautam 
---

Changes since v1:
  - New patch added to the series.

  drivers/reset/core.c  | 169 ++
  include/linux/reset.h | 108 
  2 files changed, 277 insertions(+)

diff --git a/drivers/reset/core.c b/drivers/reset/core.c
index 66db061165cb..fb908aa4f69e 100644
--- a/drivers/reset/core.c
+++ b/drivers/reset/core.c
@@ -477,3 +477,172 @@ int of_reset_control_get_count(struct device_node *node)
return count;
  }
  EXPORT_SYMBOL_GPL(of_reset_control_get_count);
+
+/*
+ * APIs to manage an array of reset controllers
+ */
+/**
+ * reset_control_array_assert: assert a list of resets
+ *
+ * @resets: reset control array holding info about a list of resets
+ * @num_rsts: number of resets to be asserted.

This should mention the API doesn't make any guarantees that the reset
lines controlled by the reset array are asserted or deasserted in any
particular order.


Sure, will add this comment.




+ * Returns 0 on success or error number on failure.
+ */
+int reset_control_array_assert(int num_rsts,
+   struct reset_control_array *resets)

I'd prefer to mirror the gpiod API a little, and to have the number
contained in the array structure, similar to struct gpio_descs:

struct reset_control_array {
unsigned int num_rstcs;
struct reset_control *rstc[];
};

int reset_control_array_assert(struct reset_control_array *resets)
{
...


Alright, i can update this.
I took regulator_bulk interface as the reference, but now i see
gpio_descs has a smaller footprint.


+{
+   int ret, i;
+
+   if (WARN_ON(IS_ERR_OR_NULL(resets)))
+   return -EINVAL;
+
+   for (i = 0; i < num_rsts; i++) {
+   ret = reset_control_assert(resets[i].rst);
+   if (ret)
+   return ret;
+   }
+
+   return 0;
+}
+EXPORT_SYMBOL_GPL(reset_control_array_assert);
+
+/**
+ * reset_control_array_deassert: deassert a list of resets
+ *
+ * @resets: reset control array holding info about a list of resets
+ * @num_rsts: number of resets to be deasserted.

Same here, no guarantees on the order in which the resets are
deasserted.


sure.




+ * Returns 0 on success or error number on failure.
+ */
+int reset_control_array_deassert(int num_rsts,
+   struct reset_control_array *resets)
+{
+   int ret, i;
+
+   if (WARN_ON(IS_ERR_OR_NULL(resets)))
+   return -EINVAL;
+
+   for (i = 0; i < num_rsts; i++)
+   ret = reset_control_deassert(resets[i].rst);
+   if (ret)
+   goto err;
+
+   return 0;
+
+err:
+   while (--i >= 0)
+   reset_control_assert(resets[i].rst);
+   return ret;
+}
+EXPORT_SYMBOL_GPL(reset_control_array_deassert);
+
+/**
+ * reset_control_array_get - Get a list of reset controllers

A list of "reset controls".


right, will update this.




+ *
+ * @dev: device that requests the list of reset controllers
+ * @num_rsts: number of reset controllers requested
+ * @resets: reset control array holding info about a list of resets
+ * @shared: whether reset controllers are shared or not
+ * @optional: whether it is optional to get the reset controllers
+ *

This should mention that the array API is intended for a list of resets
that just have to be asserted or deasserted, without any requirements on
the order.


Sure, will mention that.




+ * Returns 0 on success or error number on failure
+ */
+static int reset_control_array_get(struct device *dev, int num_rsts,
+   struct reset_control_array *resets,
+   bool shared, bool optional)

Can we make this count and return a pointer to the newly created array?

static struct reset_controls *
reset_control_array_get(struct device *dev, bool shared, bool optional)
{
...

That way the consumer doesn't have to care about counting and array
allocation.


Just trying to think again in line with the regulator bulk APIs.
Can't a user provide a list of reset controls as data and thus
request reset controls with a "id" and num_resets available.

e.g.
   const char * const rst_names[] = {
 "rst1", "rst2" ...
   };
   xyz_data = {
 .resets = rst_names;
 .num = 

Re: Fw: New Defects reported by Coverity Scan for Linux

[Oliver Neukum]

Am Montag, den 03.04.2017, 12:48 -0700 schrieb Stephen Hemminger:
> Looks like new warnings in usbnet
> 
> 
> ** CID 751368:  Null pointer dereferences  (FORWARD_NULL)
> /drivers/net/usb/usbnet.c: 1925 in __usbnet_read_cmd()

Hi Stephen,

I am afraid I don't see the problem. Yes, we are passing
a NULL pointer down, but only if size==0. In that case it
is allowed.
Could you shed some light on the meaning of the report?
I might be overlooking something, but to me it looks
like a false positive.

Regards
Oliver

--
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] usb: dwc3: gadget: skip Set/Clear Halt when invalid

[Felipe Balbi]

Hi John,

Felipe Balbi  writes:
> At least macOS seems to be sending
> ClearFeature(ENDPOINT_HALT) to endpoints which
> aren't Halted. This makes DWC3's CLEARSTALL command
> time out which causes several issues for the driver.
>
> Instead, let's just return 0 and bail out early.
>
> Cc: 
> Signed-off-by: Felipe Balbi 
> ---
>
> this falls into "has never worked before" category, so I'll be sending
> it together with other patches for v4.11 merge window. Still, it's a
> valid bug that's likely needed for stable trees.
>
>  drivers/usb/dwc3/gadget.c | 5 +
>  1 file changed, 5 insertions(+)
>
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 6faf484e5dfc..0a664d8eba3f 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1379,6 +1379,9 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int 
> value, int protocol)
>   unsigned transfer_in_flight;
>   unsigned started;
>  
> + if (dep->flags & DWC3_EP_STALL)
> + return 0;
> +
>   if (dep->number > 1)
>   trb = dwc3_ep_prev_trb(dep, dep->trb_enqueue);
>   else
> @@ -1400,6 +1403,8 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int 
> value, int protocol)
>   else
>   dep->flags |= DWC3_EP_STALL;
>   } else {
> + if (!(dep->flags & DWC3_EP_STALL))
> + return 0;
>  
>   ret = dwc3_send_clear_stall_ep_cmd(dep);
>   if (ret)

Reviving this old thread here. While $subject allowed dwc3 to work when
attached to macOS Host, I fear that we might have more issues than not
in the future. The reason is that USB20 spec allows hosts to use
ClearFeature(ENDPOINT_HALT) as a "Reset Data Toggle/SeqN" hint.

With this, we're basically blocking that possibility. Still, without
$subject, ClearStall commands were timing out. I'll try to do a local
revert here and check what happens in this case, but would you have any
idea why ClearStall would time out like that?

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v3 1/3] usb: udc: allow adding and removing the same gadget device

[Felipe Balbi]

Hi,

Alan Stern  writes:
> On Mon, 3 Apr 2017, Roger Quadros wrote:
>
>> allow usb_del_gadget_udc() and usb add_gadget_udc() to be called
>> repeatedly on the same gadget->dev structure.
>> 
>> We need to clear the gadget->dev structure so that kobject_init()
>> doesn't complain about already initialized object.
>> 
>> Signed-off-by: Roger Quadros 
>> ---
>>  drivers/usb/gadget/udc/core.c | 1 +
>>  1 file changed, 1 insertion(+)
>> 
>> diff --git a/drivers/usb/gadget/udc/core.c b/drivers/usb/gadget/udc/core.c
>> index d685d82..efce68e 100644
>> --- a/drivers/usb/gadget/udc/core.c
>> +++ b/drivers/usb/gadget/udc/core.c
>> @@ -1273,6 +1273,7 @@ void usb_del_gadget_udc(struct usb_gadget *gadget)
>>  flush_work(>work);
>>  device_unregister(>dev);
>>  device_unregister(>dev);
>> +memset(>dev, 0x00, sizeof(gadget->dev));
>>  }
>>  EXPORT_SYMBOL_GPL(usb_del_gadget_udc);
>
> Isn't this dangerous?  It's quite possible that the device_unregister() 

not on the gadget API, no.

> call on the previous line invokes the gadget->dev.release callback, 
> which might deallocate gadget.  If that happens, your new memset will 
> oops.

that won't happen. struct usb_gadget is a member of the UDC's private
structure, like this:

struct dwc3 {
[...]
struct usb_gadget   gadget;
struct usb_gadget_driver *gadget_driver;
[...]
};

I'm actually thinking that struct usb_gadget shouldn't have a struct
device at all. Just a pointer to a device, that would solve all these
issues.

-- 
balbi


signature.asc
Description: PGP signature

[PATCH v4 3/3] usb: dwc3: Add dual-role support

[Roger Quadros]

If dr_mode is "otg" then support dual role mode of operation.
Currently this mode is only supported when an extcon handle is
present in the dwc3 device tree node. This is needed to
get the ID status events of the port.

We're using a workqueue to manage the dual-role state transitions
as the extcon notifier (dwc3_drd_notifier) is called in an atomic
context by extcon_sync() and this doesn't go well with
usb_del_gadget_udc() causing a lockdep and softirq warning.

Signed-off-by: Roger Quadros 
---
v4:
-fix build error if CONFIG_USB_DWC3_DUAL_ROLE is not set

 drivers/usb/dwc3/Makefile |   4 ++
 drivers/usb/dwc3/core.c   |  15 +
 drivers/usb/dwc3/core.h   |  19 ++
 drivers/usb/dwc3/drd.c| 167 ++
 4 files changed, 193 insertions(+), 12 deletions(-)
 create mode 100644 drivers/usb/dwc3/drd.c

diff --git a/drivers/usb/dwc3/Makefile b/drivers/usb/dwc3/Makefile
index ffca340..f15fabb 100644
--- a/drivers/usb/dwc3/Makefile
+++ b/drivers/usb/dwc3/Makefile
@@ -17,6 +17,10 @@ ifneq ($(filter y,$(CONFIG_USB_DWC3_GADGET) 
$(CONFIG_USB_DWC3_DUAL_ROLE)),)
dwc3-y  += gadget.o ep0.o
 endif
 
+ifneq ($(CONFIG_USB_DWC3_DUAL_ROLE),)
+   dwc3-y  += drd.o
+endif
+
 ifneq ($(CONFIG_USB_DWC3_ULPI),)
dwc3-y  += ulpi.o
 endif
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index bf917c2..15c05a1 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -864,12 +864,10 @@ static int dwc3_core_init_mode(struct dwc3 *dwc)
}
break;
case USB_DR_MODE_OTG:
-   /* start in peripheral role by default */
-   dwc3_set_mode(dwc, DWC3_GCTL_PRTCAP_DEVICE);
-   ret = dwc3_gadget_init(dwc);
+   ret = dwc3_drd_init(dwc);
if (ret) {
if (ret != -EPROBE_DEFER)
-   dev_err(dev, "failed to initialize gadget\n");
+   dev_err(dev, "failed to initialize 
dual-role\n");
return ret;
}
break;
@@ -891,14 +889,7 @@ static void dwc3_core_exit_mode(struct dwc3 *dwc)
dwc3_host_exit(dwc);
break;
case USB_DR_MODE_OTG:
-   /* role might have changed since start */
-   if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_HOST) {
-   dwc3_host_exit(dwc);
-   /* Add back UDC to match dwc3_gadget_exit() */
-   usb_add_gadget_udc(dwc->dev, >gadget);
-   }
-
-   dwc3_gadget_exit(dwc);
+   dwc3_drd_exit(dwc);
break;
default:
/* do nothing */
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index adb04af..b1be4a3 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -786,6 +786,10 @@ struct dwc3_scratchpad_array {
  * @revision: revision register contents
  * @dr_mode: requested mode of operation
  * @current_dr_role: current role of operation when in dual-role mode
+ * @edev: extcon handle
+ * @edev_nb: extcon notifier
+ * @drd_work: dual-role work
+ * @drd_prevent_change: flag to prevent dual-role state change
  * @hsphy_mode: UTMI phy mode, one of following:
  * - USBPHY_INTERFACE_MODE_UTMI
  * - USBPHY_INTERFACE_MODE_UTMIW
@@ -903,6 +907,11 @@ struct dwc3 {
 
enum usb_dr_modedr_mode;
u32 current_dr_role;
+   struct extcon_dev   *edev;
+   struct notifier_block   edev_nb;
+   booldrd_prevent_change;
+   struct work_struct  drd_work;
+
enum usb_phy_interface  hsphy_mode;
 
u32 fladj;
@@ -1225,6 +1234,16 @@ static inline int 
dwc3_send_gadget_generic_command(struct dwc3 *dwc,
 { return 0; }
 #endif
 
+#if IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
+int dwc3_drd_init(struct dwc3 *dwc);
+void dwc3_drd_exit(struct dwc3 *dwc);
+#else
+static inline int dwc3_drd_init(struct dwc3 *dwc)
+{ return 0; }
+static inline void dwc3_drd_exit(struct dwc3 *dwc)
+{ }
+#endif
+
 /* power management interface */
 #if !IS_ENABLED(CONFIG_USB_DWC3_HOST)
 int dwc3_gadget_suspend(struct dwc3 *dwc);
diff --git a/drivers/usb/dwc3/drd.c b/drivers/usb/dwc3/drd.c
new file mode 100644
index 000..c9b02a3
--- /dev/null
+++ b/drivers/usb/dwc3/drd.c
@@ -0,0 +1,167 @@
+/**
+ * drd.c - DesignWare USB3 DRD Controller Dual-role support
+ *
+ * Copyright (C) 2017 Texas Instruments Incorporated - http://www.ti.com
+ *
+ * Authors: Roger Quadros 
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2  of
+ * the License as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be 

Re: [PATCH 2/2] usb: misc: refactor code

[Felipe Balbi]

Hi,

"Gustavo A. R. Silva"  writes:
> Code refactoring to make the flow easier to follow.
>
> Cc: Alan Stern 
> Cc: Greg Kroah-Hartman 
> Signed-off-by: Gustavo A. R. Silva 
> ---
>  drivers/usb/misc/usbtest.c | 67 
> +-
>  1 file changed, 30 insertions(+), 37 deletions(-)
>
> diff --git a/drivers/usb/misc/usbtest.c b/drivers/usb/misc/usbtest.c
> index 7bfb6b78..382491e 100644
> --- a/drivers/usb/misc/usbtest.c
> +++ b/drivers/usb/misc/usbtest.c
> @@ -124,18 +124,32 @@ static struct usb_device *testdev_to_usbdev(struct 
> usbtest_dev *test)
>  
>  /*-*/
>  
> +static inline void endpoint_update(int edi,
> +struct usb_host_endpoint **in,
> +struct usb_host_endpoint **out,
> +struct usb_host_endpoint *e)
> +{
> + if (edi) {
> + if (!*in)
> + *in = e;
> + } else {
> + if (!*out)
> + *out = e;
> + }
> +}
> +
>  static int
>  get_endpoints(struct usbtest_dev *dev, struct usb_interface *intf)
>  {
> - int tmp;
> - struct usb_host_interface   *alt;
> - struct usb_host_endpoint*in, *out;
> - struct usb_host_endpoint*iso_in, *iso_out;
> - struct usb_host_endpoint*int_in, *int_out;
> - struct usb_device   *udev;
> + int tmp;
> + struct usb_host_interface   *alt;
> + struct usb_host_endpoint*in, *out;
> + struct usb_host_endpoint*iso_in, *iso_out;
> + struct usb_host_endpoint*int_in, *int_out;
> + struct usb_device   *udev;

unnecessary change

>  
>   for (tmp = 0; tmp < intf->num_altsetting; tmp++) {
> - unsignedep;
> + unsignedep;

unnecessary change

>  
>   in = out = NULL;
>   iso_in = iso_out = NULL;
> @@ -150,48 +164,27 @@ get_endpoints(struct usbtest_dev *dev, struct 
> usb_interface *intf)
>* ignore other endpoints and altsettings.
>*/
>   for (ep = 0; ep < alt->desc.bNumEndpoints; ep++) {
> - struct usb_host_endpoint*e;
> + struct usb_host_endpoint*e;

unnecessary change

-- 
balbi


signature.asc
Description: PGP signature

Re: [PATCH v3 3/3] usb: dwc3: Add dual-role support

[Roger Quadros]

Felipe,

I'll send a revised patch to fix this.

cheers,
-roger

On 03/04/17 22:21, kbuild test robot wrote:
> Hi Roger,
> 
> [auto build test ERROR on balbi-usb/next]
> [also build test ERROR on v4.11-rc5 next-20170403]
> [if your patch is applied to the wrong git tree, please drop us a note to 
> help improve the system]
> 
> url:
> https://github.com/0day-ci/linux/commits/Roger-Quadros/usb-dwc3-dual-role-support/20170404-022401
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb.git next
> config: i386-randconfig-x010-201714 (attached as .config)
> compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901
> reproduce:
> # save the attached .config to linux build tree
> make ARCH=i386 
> 
> All error/warnings (new ones prefixed by >>):
> 
>In file included from drivers/usb/dwc3/core.c:44:0:
>>> drivers/usb/dwc3/core.h:1243:1: error: expected identifier or '(' before 
>>> '{' token
> { }
> ^
>>> drivers/usb/dwc3/core.h:1242:13: warning: 'dwc3_drd_exit' used but never 
>>> defined
> static void dwc3_drd_exit(struct dwc3 *dwc);
> ^
> --
>In file included from drivers/usb/dwc3/trace.h:27:0,
> from drivers/usb/dwc3/trace.c:19:
>>> drivers/usb/dwc3/core.h:1243:1: error: expected identifier or '(' before 
>>> '{' token
> { }
> ^
>In file included from drivers/usb/dwc3/trace.h:27:0,
> from drivers/usb/dwc3/trace.c:19:
>drivers/usb/dwc3/core.h:1242:13: warning: 'dwc3_drd_exit' declared 
> 'static' but never defined [-Wunused-function]
> static void dwc3_drd_exit(struct dwc3 *dwc);
> ^
> 
> vim +1243 drivers/usb/dwc3/core.h
> 
>   1236#if IS_ENABLED(CONFIG_USB_DWC3_DUAL_ROLE)
>   1237int dwc3_drd_init(struct dwc3 *dwc);
>   1238void dwc3_drd_exit(struct dwc3 *dwc);
>   1239#else
>   1240static inline int dwc3_drd_init(struct dwc3 *dwc)
>   1241{ return 0; }
>> 1242 static void dwc3_drd_exit(struct dwc3 *dwc);
>> 1243 { }
>   1244#endif
>   1245
>   1246/* power management interface */
> 
> ---
> 0-DAY kernel test infrastructureOpen Source Technology Center
> https://lists.01.org/pipermail/kbuild-all   Intel Corporation
> 
--
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: dwc2_hc_chhltd_intr_dma - ChHltd set errors?

[Felipe Balbi]

Hi,

Minas Harutyunyan  writes:
>>   We've noticed that when using usb ethernet adapters on HiKey, we
>> occasionally see errors like:
>>
>> dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 0 - ChHltd set,
>> but reason is unknown
>> dwc2 f72c.usb: hcint 0x0002, intsts 0x06200029
>>
>> dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 11 - ChHltd set,
>> but reason is unknown
>> dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
>>
>> Sometimes followed up by a usb error in the driver, something like:
>> asix 1-1.2:1.0 eth0: asix_rx_fixup() Bad Header Length 0x36000807, 
>> offset 68
>>
>> Curious if you've seen any reports like this?
>>>
>>> The core version is less than 2.71a, am I right?
>>
>> So it looks like its reporting 0x4f54300a for hsotg->regs + GSNPSID
>> which looks like DWC2_CORE_REV_3_00a
>>
>>> Please send full debug log to do more investigation.
>>
>> Full dmesg, or is there special debugging you want me to enable?
>
> Full dmesg around issue.
>>
>>> Also send us regdump after connecting ethernet adapter.
>>
>> Sorry, can you clarify how to generate this?
>
> cat regdump. To locate dwc2 regdump file: cd /; find -name regdump

this won't work if his distro doesn't mount debugfs. Please give
complete instructions ;-)

# mkdir -p /d
# mount -t debugfs none /d
# cd /d
# find . -name regdump

The directory name is the same name as the dwc2 device name, AFAICT. So,
check your DTS for the name of the device.

-- 
balbi


signature.asc
Description: PGP signature

Re: dwc2_hc_chhltd_intr_dma - ChHltd set errors?

[Minas Harutyunyan]

Hi,

On 4/4/2017 7:04 AM, John Stultz wrote:
> On Mon, Apr 3, 2017 at 5:54 AM, Minas Harutyunyan
>  wrote:
>> On 4/3/2017 9:23 AM, John Youn wrote:
>>> On 03/31/2017 04:04 PM, John Stultz wrote:
 On Thu, Mar 2, 2017 at 12:00 PM, John Stultz  
 wrote:
> Hey John,
>   We've noticed that when using usb ethernet adapters on HiKey, we
> occasionally see errors like:
>
> dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 0 - ChHltd set,
> but reason is unknown
> dwc2 f72c.usb: hcint 0x0002, intsts 0x06200029
>
> dwc2 f72c.usb: dwc2_hc_chhltd_intr_dma: Channel 11 - ChHltd set,
> but reason is unknown
> dwc2 f72c.usb: hcint 0x0002, intsts 0x04200029
>
> Sometimes followed up by a usb error in the driver, something like:
> asix 1-1.2:1.0 eth0: asix_rx_fixup() Bad Header Length 0x36000807, offset 
> 68
>
> Curious if you've seen any reports like this?
>>
>> The core version is less than 2.71a, am I right?
>
> So it looks like its reporting 0x4f54300a for hsotg->regs + GSNPSID
> which looks like DWC2_CORE_REV_3_00a
>
>> Please send full debug log to do more investigation.
>
> Full dmesg, or is there special debugging you want me to enable?

Full dmesg around issue.
>
>> Also send us regdump after connecting ethernet adapter.
>
> Sorry, can you clarify how to generate this?

cat regdump. To locate dwc2 regdump file: cd /; find -name regdump

>
> thanks
> -john
>

Thanks,
Minas
--
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

usb: musb: regression since 4.9 on omap4-panda-es (caused by d8e5f0eca1e8)

[Peter Ujfalusi]

Tony,

since 4.9 (4.8 was fine) I can not boot omap4-panda-es if the musb is 
compiled in. The kernel will stuck printing:


** 206 printk messages dropped ** [8.926727] musb_bus_suspend 2584: 
trying to suspend as a_idle while active


The bisect (log is [1]) points to:
d8e5f0eca1e8 usb: musb: Fix hardirq-safe hardirq-unsafe lock order error

and reverting the d8e5f0eca1e8 makes the board to boot up fine (Linux 
4.11-rc5 and next-20170331).


any idea on how to fix this w/o reverting the commit?

[1] https://pastebin.com/Z2HJY229

- Péter
--
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] ARM: davinci: Add the clock for the CPPI 4.1 DMA engine

[Sekhar Nori]

On Wednesday 29 March 2017 09:39 PM, Alexandre Bailon wrote:
> The CPPI 4.1 DMA is sharing its clock with the USB OTG,
> and most of the time, the clock will be enabled by USB.
> But during the init of the DMA, USB is not enabled (waiting for DMA),
> and then we must enable the clock before to do anything.
> Add the clock for the CPPI 4.1 DMA engine.
> 
> Note:
> This patch is to apply instead of:
> "ARM: davinci: Make the usb20 clock available to PM runtime"

Okay, but I still liked the fact that that patch was using NULL as
con_id for MUSB clock. That makes sense because MUSB gets a single clock
input. I think you should still make that change. If not for v4.12, then
for v4.13.

> 
> Signed-off-by: Alexandre Bailon 
> ---
>  arch/arm/mach-davinci/da830.c| 1 +
>  arch/arm/mach-davinci/da850.c| 1 +
>  arch/arm/mach-davinci/da8xx-dt.c | 2 ++
>  3 files changed, 4 insertions(+)
> 
> diff --git a/arch/arm/mach-davinci/da830.c b/arch/arm/mach-davinci/da830.c
> index 073c458..ae4a8a5 100644
> --- a/arch/arm/mach-davinci/da830.c
> +++ b/arch/arm/mach-davinci/da830.c
> @@ -413,6 +413,7 @@ static struct clk_lookup da830_clks[] = {
>   CLK("davinci-mcasp.1",  NULL,   _clk),
>   CLK("davinci-mcasp.2",  NULL,   _clk),
>   CLK("musb-da8xx",   "usb20",_clk),
> + CLK("cppi41-dmaengine", NULL,   _clk),

Did you try reading /sys/kernel/debug/davinci_clocks after this patch?
It will hang because of the loop created here.

Looks like what you want is cppi4.1 dma clock to be a child of MUSB
clock. That way, even if DMA is enabled before MUSB, it still works.

Thanks,
Sekhar
--
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