[PATCH] usb: musb: Cancel delayed work on host controller rmmod

2019-09-15 Thread Tony Lindgren
If we rmmod the musb host controller with gadgets configured,
musb_gadget_stop() gets called. We need to also flush the delayed
work.

Fixes: 2bff3916fda9 ("usb: musb: Fix PM for hub disconnect")
Signed-off-by: Tony Lindgren 
---
 drivers/usb/musb/musb_gadget.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/musb/musb_gadget.c b/drivers/usb/musb/musb_gadget.c
--- a/drivers/usb/musb/musb_gadget.c
+++ b/drivers/usb/musb/musb_gadget.c
@@ -1910,6 +1910,7 @@ static int musb_gadget_stop(struct usb_gadget *g)
 
/* Force check of devctl register for PM runtime */
schedule_delayed_work(&musb->irq_work, 0);
+   flush_delayed_work(&musb->irq_work);
 
pm_runtime_mark_last_busy(musb->controller);
pm_runtime_put_autosuspend(musb->controller);
-- 
2.23.0


RE: EHSET with hub and PCIe root hub

2019-09-15 Thread Peter Chen
 
> > >I should add that the USB 2.0 spec includes the following text (from 
> > >section
> 11.24.2.13):
> > >
> > >Test mode of a downstream facing port can only be used in
> > >a well defined sequence of hub states. This sequence is
> > >defined as follows:
> > >
> > >1)  All enabled downstream facing ports of the hub containing
> > >the port to be tested must be (selectively) suspended via
> > >the SetPortFeature(PORT_SUSPEND) request.  Each downstream
> > >facing port of the hub must be in the disabled,
> > >disconnected, or suspended state (see Figure 11-9).
> > >
> > >So you can see the hub probably failed the request because a
> > >non-suspended device was connected to port 3.  (And who knows what
> > >was attached to the other ports -- the usbmon trace doesn't say.)
> > >
> > >Alan Stern
> >
> > This was very helpful.
> >
> > I was able to get the USB3503 to generate test packets by adding a
> SetPortFeature(PORT_SUSPEND) request to suspend the port before setting the
> PORT_TEST feature. Is there a way to tell that a device is a hub but not a 
> root hub
> so ports on root hub ports aren't suspended prior to calling
> SetPortFeature(PORT_TEST)?
> >
> > I tried to use hub_udev->maxchild to determine if something was a hub but 
> > this
> appears misguided since root hubs can have multiple children, nothing else in 
> the
> usb_device structure jumped out as being directly related to a hub.
> 
> That's a perfectly good way to see that the device really is a hub.  In 
> addition, if
> hub_udev->parent == NULL then hub_udev is a root hub, otherwise it isn't.
> 

Hi Allen & Alan,

Good finding.

Besides, if you would like to generate a formal patch, per 7.1.20 Test Mode 
Support, you may
support Test_SE0_NAK/Test_J/Test_K/Test_Packet all for non-root hub. The other 
three test
modes should be embedded host required.

Peter



Re: [PATCH 0/4] musb host improvments mostly for omap2430 glue

2019-09-15 Thread Tony Lindgren
Hi,

* Pavel Machek  [190903 08:08]:
> On Mon 2019-09-02 09:06:51, Tony Lindgren wrote:
> #2 definitely has _something_, because it does work in original
>  android. But not even original android provides VBUS (5V on USB) in
>  that configuration. It also looks like hardware _can_ provide at
>  least VBAT on VBUS, because I seen that during some of the crashes.

I started seeing the musb hang issue you reported today
on one droid4 but not on an other one :) Turns out we have
racy .set_vbus still around that might get called.

The following patch is needed in preparation for this $subject
series as otherwise patch "[PATCH 3/4] usb: musb: Add
musb_set_host and peripheral and use them for omap2430" can
cause a hang with "scheduling while atomic" if .set_vbus
gets called from musb_stage0_irq() path.

Regards,

Tony

8< -
>From tony Mon Sep 17 00:00:00 2001
From: Tony Lindgren 
Date: Sun, 15 Sep 2019 17:16:58 -0700
Subject: [PATCH] usb: musb: Get rid of musb_set_vbus()

We currently have musb_set_vbus() called from two different paths. Mostly
it gets called from the USB PHY via omap_musb_set_mailbox(), but in some
cases it can get also called from musb_stage0_irq() rather via .set_vbus:

(musb_set_host [musb_hdrc])
(omap2430_musb_set_vbus [omap2430])
(musb_stage0_irq [musb_hdrc])
(musb_interrupt [musb_hdrc])
(omap2430_musb_interrupt [omap2430])

This is racy and will not work with introducing generic helper functions
for musb_set_host() and musb_set_peripheral(). We want to get rid of the
busy loops in favor of usleep_range().

Let's just get rid of .set_vbus for omap2430 glue layer and let the PHY
code handle VBUS with musb_set_vbus(). Note that in the follow-up patch
we can completely remove omap2430_musb_set_vbus(), but let's do it in a
separate patch as this change may actually turn out to be needed as a
fix.

Reported-by: Pavel Machek 
Signed-off-by: Tony Lindgren 
---
 drivers/usb/musb/omap2430.c | 2 --
 1 file changed, 2 deletions(-)

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
@@ -312,8 +312,6 @@ static const struct musb_platform_ops omap2430_ops = {
.init   = omap2430_musb_init,
.exit   = omap2430_musb_exit,
 
-   .set_vbus   = omap2430_musb_set_vbus,
-
.enable = omap2430_musb_enable,
.disable= omap2430_musb_disable,
 
-- 
2.23.0


Re: [PATCH 0/4] musb host improvments mostly for omap2430 glue

2019-09-15 Thread Tony Lindgren
* Tony Lindgren  [190916 01:21]:
> Let's just get rid of .set_vbus for omap2430 glue layer and let the PHY
> code handle VBUS with musb_set_vbus(). Note that in the follow-up patch
> we can completely remove omap2430_musb_set_vbus(), but let's do it in a
> separate patch as this change may actually turn out to be needed as a
> fix.

FYI, below is a quick take at dropping omap2430_musb_set_vbus(),
needs more testing though. I'm still seeing some errors on
disconnect with this one like:

VBUS_ERROR in b_idle (80,  A_IDLE and jumping right to B_IDLE
- * as set by musb_set_peripheral().
- */
-static void omap2430_musb_set_vbus(struct musb *musb, int is_on)
-{
-   struct usb_otg *otg = musb->xceiv->otg;
-   int error;
-
-   if (is_on) {
-   switch (musb->xceiv->otg->state) {
-   case OTG_STATE_A_IDLE:
-   error = musb_set_host(musb);
-   if (!error) {
-   musb->xceiv->otg->state = 
OTG_STATE_A_WAIT_VRISE;
-   otg_set_vbus(otg, 1);
-   }
-   break;
-   default:
-   otg_set_vbus(otg, 1);
-   break;
-   }
-   } else {
-   error = musb_set_peripheral(musb);
-   otg_set_vbus(otg, 0);
-   }
-
-   dev_dbg(musb->controller, "VBUS %s, devctl %02x\n",
-   usb_otg_state_string(musb->xceiv->otg->state),
-   musb_readb(musb->mregs, MUSB_DEVCTL));
-}
-
 static inline void omap2430_low_level_exit(struct musb *musb)
 {
u32 l;
@@ -112,27 +77,42 @@ static int omap2430_musb_mailbox(enum musb_vbus_id_status 
status)
return 0;
 }
 
+/*
+ * HDRC controls CPEN, but beware current surges during device connect.
+ * They can trigger transient overcurrent conditions that must be ignored.
+ *
+ * Note that we're skipping A_WAIT_VFALL -> A_IDLE and jumping right to B_IDLE
+ * as set by musb_set_peripheral().
+ */
 static void omap_musb_set_mailbox(struct omap2430_glue *glue)
 {
struct musb *musb = glue_to_musb(glue);
-   struct musb_hdrc_platform_data *pdata =
-   dev_get_platdata(musb->controller);
-   struct omap_musb_board_data *data = pdata->board_data;
+   int error;
 
pm_runtime_get_sync(musb->controller);
+
+   dev_dbg(musb->controller, "VBUS %s, devctl %02x\n",
+   usb_otg_state_string(musb->xceiv->otg->state),
+   musb_readb(musb->mregs, MUSB_DEVCTL));
+
switch (glue->status) {
case MUSB_ID_GROUND:
dev_dbg(musb->controller, "ID GND\n");
switch (musb->xceiv->otg->state) {
+   case OTG_STATE_A_IDLE:
+   error = musb_set_host(musb);
+   if (error)
+   break;
+   musb->xceiv->otg->state = OTG_STATE_A_WAIT_VRISE;
+   /* Fall through */
case OTG_STATE_A_WAIT_VRISE:
case OTG_STATE_A_WAIT_BCON:
case OTG_STATE_A_HOST:
-   case OTG_STATE_A_IDLE:
/*
 * On multiple ID ground interrupts just keep enabling
 * VBUS. At least cpcap VBUS shuts down otherwise.
 */
-   omap2430_musb_set_vbus(musb, 1);
+   otg_set_vbus(musb->xceiv->otg, 1);
break;
default:
musb->xceiv->otg->state = OTG_STATE_A_IDLE;
@@ -140,7 +120,7 @@ static void omap_musb_set_mailbox(struct omap2430_glue 
*glue)
if (musb->gadget_driver) {
omap_control_usb_set_mode(glue->control_otghs,
  USB_MODE_HOST);
-   omap2430_musb_set_vbus(musb, 1);
+   otg_set_vbus(musb->xceiv->otg, 1);
}
break;
}
@@ -159,14 +139,10 @@ static void omap_musb_set_mailbox(struct omap2430_glue 
*glue)
dev_dbg(musb->controller, "VBUS Disconnect\n");
 
musb->xceiv->last_event = USB_EVENT_NONE;
-   if (musb->gadget_driver)
-   omap2430_musb_set_vbus(musb, 0);
-
-   if (data->interface_type == MUSB_INTERFACE_UTMI)
-   otg_set_vbus(musb->xceiv->otg, 0);
-
+   otg_set_vbus(musb->xceiv->otg, 0);
omap_control_usb_set_mode(glue->control_otghs,
USB_MODE_DISCONNECT);
+   musb_set_peripheral(musb);
break;
default:
dev_dbg(musb->controller, "ID float\n");
-- 
2.23.0


Re: [v5,2/4] usb: chipidea: imx: add HSIC support

2019-09-15 Thread Peter Chen
On 19-09-13 10:47:58, André Draszik wrote:
> Hi Peter,
> 
> On Tue, 2018-12-11 at 02:08 +, Peter Chen wrote:
> > To support imx HSIC, there are some special requirement:
> > - The HSIC pad is 1.2v, it may need to supply from external
> > - The data/strobe pin needs to be pulled down first, and after
> >   host mode is initialized, the strobe pin needs to be pulled up
> > - During the USB suspend/resume, special setting is needed
> 
> I have an imx7d based board that is using the USBH/HSIC port.
> 
> This USB-port isn't working with this commit because the pinctrl
> is non-optional (as opposed to the NXP kernel 4.19.35 where
> the pinctrl is optional and it appears to work fine for us
> without).
> 
> Now, I'd like to make it work with your patch here, but I
> am not sure the relevant pinmux setting is documented in
> the IMX7D applications processor reference manual
> IMX7DRM.pdf that I have.

Hi Andre,

Thanks for reporting it. i.MX7D uses different HSIC controller
compares with imx6, and there are dedicated HSIC pins which
doesn't need configure pinmux. So the suitable solution is
getting pinmux optional, a patch to improve it is welcome.

Peter

> 
> As far as I understand, I need to provide sth like the following:
> 
> &usbh {
> ...
> pinctrl-names = "idle", "active";
> pinctrl-0 = <&pinctrl_usbh_idle>;
> pinctrl-1 = <&pinctrl_usbh_active>;
> ...
> };
> 
> pinctrl_usbh_idle: usbhgrp-idle {
> fsl,pins = <
> MX7D_PAD_USB_H_DATA__USB_H_DATA 0x
> MX7D_PAD_USB_H_STROBE__USB_H_STROBE 0x
> >;
> };
> 
> pinctrl_usbh_active: usbhgrp-active {
> fsl,pins = <
> MX7D_PAD_USB_H_DATA__USB_H_DATA 0x
> MX7D_PAD_USB_H_STROBE__USB_H_STROBE 0x
>  >;
> };
> 
> 
> Can you help or point me to the relevant documentation please for
> defining MX7D_PAD_USB_H_DATA__USB_H_DATA &
> MX7D_PAD_USB_H_STROBE__USB_H_STROBE and their register settings?
> 
> Given USB_H_DATA and USB_H_STROBE don't appear to be board specific
> 'No muxing' is mentioned in IMX7DRM), should any such configuration
> really go into imx7s.dtsi?
> 
> Alternatively, given that this works without the extra pinctrl
> dance in the i.MX specific kernel, is it really needed on i.MX7,
> or can the driver be changed to make this optional here as well?
> What is the right approach?
> 
> 
> Cheers,
> Andre'
> 
> 
> 
> 
> > 
> > Reviewed-by: Frieder Schrempf 
> > Tested-by: Frieder Schrempf 
> > Signed-off-by: Peter Chen 
> > ---
> >  drivers/usb/chipidea/ci_hdrc_imx.c | 140 
> > -
> >  drivers/usb/chipidea/ci_hdrc_imx.h |   9 ++-
> >  drivers/usb/chipidea/usbmisc_imx.c | 140 
> > +
> >  3 files changed, 270 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/usb/chipidea/ci_hdrc_imx.c 
> > b/drivers/usb/chipidea/ci_hdrc_imx.c
> > index 09b37c0d075d..56781c329db0 100644
> > --- a/drivers/usb/chipidea/ci_hdrc_imx.c
> > +++ b/drivers/usb/chipidea/ci_hdrc_imx.c
> > @@ -14,6 +14,7 @@
> >  #include 
> >  #include 
> >  #include 
> > +#include 
> >  
> >  #include "ci.h"
> >  #include "ci_hdrc_imx.h"
> > @@ -85,6 +86,9 @@ struct ci_hdrc_imx_data {
> > bool supports_runtime_pm;
> > bool override_phy_control;
> > bool in_lpm;
> > +   struct pinctrl *pinctrl;
> > +   struct pinctrl_state *pinctrl_hsic_active;
> > +   struct regulator *hsic_pad_regulator;
> > /* SoC before i.mx6 (except imx23/imx28) needs three clks */
> > bool need_three_clks;
> > struct clk *clk_ipg;
> > @@ -245,19 +249,49 @@ static void imx_disable_unprepare_clks(struct device 
> > *dev)
> > }
> >  }
> >  
> > +static int ci_hdrc_imx_notify_event(struct ci_hdrc *ci, unsigned int event)
> > +{
> > +   struct device *dev = ci->dev->parent;
> > +   struct ci_hdrc_imx_data *data = dev_get_drvdata(dev);
> > +   int ret = 0;
> > +
> > +   switch (event) {
> > +   case CI_HDRC_IMX_HSIC_ACTIVE_EVENT:
> > +   ret = pinctrl_select_state(data->pinctrl,
> > +   data->pinctrl_hsic_active);
> > +   if (ret)
> > +   dev_err(dev, "hsic_active select failed, err=%d\n",
> > +   ret);
> > +   break;
> > +   case CI_HDRC_IMX_HSIC_SUSPEND_EVENT:
> > +   ret = imx_usbmisc_hsic_set_connect(data->usbmisc_data);
> > +   if (ret)
> > +   dev_err(dev,
> > +   "hsic_set_connect failed, err=%d\n", ret);
> > +   break;
> > +   default:
> > +   break;
> > +   }
> > +
> > +   return ret;
> > +}
> > +
> >  static int ci_hdrc_imx_probe(struct platform_device *pdev)
> >  {
> > struct ci_hdrc_imx_data *data;
> > struct ci_hdrc_platform_data pdata = {
> > .name   = dev_name(&pdev->dev),
> > .capoffset  = DEF_CAPOFFSET,
> > +   .notify_event  

No SuperSpeedPlus on ASM2142

2019-09-15 Thread Loïc Yhuel
Hello,

I'm trying to get Gen 2 working on this controller.
It drives 2 USB ports on the back panel of an ASUS Prime X399-A (latest BIOS).
ASM2142 FW is 170308_70_02_00 (seen with ASM2142A_MPTool on Windows).
On Windows 10 it uses the Microsoft xhci driver, and Gen 2 works.

On 5.3, I get :
[1.008270] xhci_hcd :08:00.0: Host supports USB 3.0 SuperSpeed
...
[1.333145] usb 4-1: new SuperSpeed Gen 1 USB device number 2 using xhci_hcd

lsusb shows 10 Gbps support in "SuperSpeedPlus USB Device Capability" for
both the root hub and the device.

For the root hub, commit ddd57980 broke the detection, since
xhci->usb3_rhub.min_rev is 0x1 instead of expected 0x10 (SBRN is 0x30).
Reverting it changes to "Host supports USB 3.1 Enhanced SuperSpeed", and the
speed of the root hub is 1 in sysfs.
However, I only got the device detected as "SuperSpeedPlus Gen 2 USB" once,
and the performance didn't increase, so even if the "speed" in sysfs was 1,
I think it didn't work. After a reboot, it reverted to being detected as Gen 1.

The device (JMS580 USB Gen 2 to SATA bridge, with an SSD) seems to have a
performance issue on Gen 1 (doesn't depend on the controller or the OS), with
about 280MB/s read (almost the same without UAS).
But Gen 2 on Windows gives 510MB/s read, so even the only time Linux reported
10 Gbps speed, if it was working "hdparm -t" should have improved.

As a side note, the runtime power management doesn't seem to work either, but
since it isn't the default configuration, unless this controller is
used on laptops
it probably doesn't matter.
If the power/control of the PCIe device and its two root hubs are all set to
"auto", it is suspended if there is no USB device, and doesn't wake up on plug.
Bus 004 Device 002: ID 0578:0578 Intrinsix Corp. JMS580
Device Descriptor:
  bLength18
  bDescriptorType 1
  bcdUSB   3.10
  bDeviceClass0 
  bDeviceSubClass 0 
  bDeviceProtocol 0 
  bMaxPacketSize0 9
  idVendor   0x0578 Intrinsix Corp.
  idProduct  0x0578 
  bcdDevice   41.01
  iManufacturer   1 JMicron
  iProduct2 JMS580
  iSerial 3 1114030541CB
  bNumConfigurations  1
  Configuration Descriptor:
bLength 9
bDescriptorType 2
wTotalLength   0x0079
bNumInterfaces  1
bConfigurationValue 1
iConfiguration  0 
bmAttributes 0xc0
  Self Powered
MaxPower8mA
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   0
  bNumEndpoints   2
  bInterfaceClass 8 Mass Storage
  bInterfaceSubClass  6 SCSI
  bInterfaceProtocol 80 Bulk-Only
  iInterface  0 
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x81  EP 1 IN
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0400  1x 1024 bytes
bInterval   0
bMaxBurst  15
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x02  EP 2 OUT
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0400  1x 1024 bytes
bInterval   0
bMaxBurst  15
Interface Descriptor:
  bLength 9
  bDescriptorType 4
  bInterfaceNumber0
  bAlternateSetting   1
  bNumEndpoints   4
  bInterfaceClass 8 Mass Storage
  bInterfaceSubClass  6 SCSI
  bInterfaceProtocol 98 
  iInterface 10 MSC USB Attached SCSI
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x01  EP 1 OUT
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0400  1x 1024 bytes
bInterval   0
bMaxBurst   0
Command pipe (0x01)
  Endpoint Descriptor:
bLength 7
bDescriptorType 5
bEndpointAddress 0x82  EP 2 IN
bmAttributes2
  Transfer TypeBulk
  Synch Type   None
  Usage Type   Data
wMaxPacketSize 0x0400  1x 1024 bytes
bInterval   0
bMaxBurst   0
MaxStreams 32
Status pipe (0x02)
  Endpoint Descri