All of lore.kernel.org
 help / color / mirror / Atom feed
* Use device tree to disable U1/U2 in gadget devices based on DWC3
@ 2019-04-24 16:26 Claus H. Stovgaard
  2019-04-24 18:33 ` Alan Stern
  0 siblings, 1 reply; 7+ messages in thread
From: Claus H. Stovgaard @ 2019-04-24 16:26 UTC (permalink / raw)
  To: balbi; +Cc: linux-usb, mnarani, Michal Simek

Hi Balbi and other USB developers.

I am developing camera devices based on Xilinx ZynqMP, using the gadget framework and the build-in dwc3 core of the ZynqMP as USB3 controller and the build-in Cirrus SERDES as phy.
Testing with a number of hosts and Windows 7, has shown sporadic reconnects when leaving U2/U1, caused by failing link training, where the host resets the bus. Sometime it also means it reconnect via USB2.

So to overcome this, I will like to have the option for disabling U1/U2 on the core when working with those hosts.

Currently I have made a hack in ep0.c  where I return EINVAL in dwc3_ep0_handle_u1 and dwc3_ep0_handle_u2 together with not setting DWC3_DCTL_ACCEPTU1ENA and DWC3_DCTL_ACCEPTU2ENA in dwc3_ep0_set_config
Will though prefer a solution possible to upstream, so was thinking about adding two devicetree bindings.

* snps,u1_disable_as_gadget: When set the core will not enable U1 if requested from host, nor initiate U1.
* snps,u2_disable_as_gadget: When set the core will not enable U2 if requested from host, nor initiate U2.

If you think this might be something which can be upstreamed I will prepare the code and send a patch for discussion.
On the other hand, if you think that disabling U1/U2 via device tree as suggested should not be a feature no need for me to try making it a feature.

Kind regards / Med venlig hilsen 

Claus Stovgaard 
Firmware Design Engineer 
Phase One

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Use device tree to disable U1/U2 in gadget devices based on DWC3
  2019-04-24 16:26 Claus H. Stovgaard
@ 2019-04-24 18:33 ` Alan Stern
  2019-04-26 12:44   ` Claus H. Stovgaard
  0 siblings, 1 reply; 7+ messages in thread
From: Alan Stern @ 2019-04-24 18:33 UTC (permalink / raw)
  To: Claus H. Stovgaard; +Cc: balbi, linux-usb, mnarani, Michal Simek

On Wed, 24 Apr 2019, Claus H. Stovgaard wrote:

> Hi Balbi and other USB developers.
> 
> I am developing camera devices based on Xilinx ZynqMP, using the
> gadget framework and the build-in dwc3 core of the ZynqMP as USB3
> controller and the build-in Cirrus SERDES as phy.
> Testing with a number of hosts and Windows 7, has shown sporadic
> reconnects when leaving U2/U1, caused by failing link training, where
> the host resets the bus. Sometime it also means it reconnect via
> USB2.
> 
> So to overcome this, I will like to have the option for disabling
> U1/U2 on the core when working with those hosts.
> 
> Currently I have made a hack in ep0.c where I return EINVAL in
> dwc3_ep0_handle_u1 and dwc3_ep0_handle_u2 together with not setting
> DWC3_DCTL_ACCEPTU1ENA and DWC3_DCTL_ACCEPTU2ENA in
> dwc3_ep0_set_config
> Will though prefer a solution possible to upstream, so was thinking
> about adding two devicetree bindings.
> 
> * snps,u1_disable_as_gadget: When set the core will not enable U1 if
> requested from host, nor initiate U1.
> * snps,u2_disable_as_gadget: When set the core will not enable U2 if
> requested from host, nor initiate U2.
> 
> If you think this might be something which can be upstreamed I will
> prepare the code and send a patch for discussion.
> On the other hand, if you think that disabling U1/U2 via device tree
> as suggested should not be a feature no need for me to try making it
> a feature.

Speaking as an interested bystander, I would first wonder why your
hardware fails during link training.  If it is properly designed, that
should not happen.  The fact that it does happen suggests your devices
might also experience problems during normal data transport.

Second, the bindings you suggested would not be accepted by the 
devicetree maintainers.  The whole idea behind devicetree is that it 
describes the hardware, not the software.  Things like 
u1_disable_as_gadget are software concepts and so do not belong in a 
devicetree description.

You might be better off creating sysfs attribute files for disabling
entry into U1 and U2.  Alternatively, if you really want to do it
through devicetree, you should create a binding that does describe your
hardware properties, such as "u1-and-u2-are-broken", or
"broken-link-training", or something else along those lines.

Alan Stern


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Use device tree to disable U1/U2 in gadget devices based on DWC3
@ 2019-04-25 20:11 ` Rob Weber
  0 siblings, 0 replies; 7+ messages in thread
From: Rob Weber @ 2019-04-25 20:11 UTC (permalink / raw)
  To: Alan Stern; +Cc: Claus H. Stovgaard, balbi, linux-usb, mnarani, Michal Simek

Hi Everyone,

On Wed, Apr 24, 2019 at 02:33:48PM -0400, Alan Stern wrote:
> On Wed, 24 Apr 2019, Claus H. Stovgaard wrote:
> 
> > Hi Balbi and other USB developers.
> > 
> > I am developing camera devices based on Xilinx ZynqMP, using the
> > gadget framework and the build-in dwc3 core of the ZynqMP as USB3
> > controller and the build-in Cirrus SERDES as phy.
> > Testing with a number of hosts and Windows 7, has shown sporadic
> > reconnects when leaving U2/U1, caused by failing link training, where
> > the host resets the bus. Sometime it also means it reconnect via
> > USB2.
> > 
> > So to overcome this, I will like to have the option for disabling
> > U1/U2 on the core when working with those hosts.
> > 
> > Currently I have made a hack in ep0.c where I return EINVAL in
> > dwc3_ep0_handle_u1 and dwc3_ep0_handle_u2 together with not setting
> > DWC3_DCTL_ACCEPTU1ENA and DWC3_DCTL_ACCEPTU2ENA in
> > dwc3_ep0_set_config
> > Will though prefer a solution possible to upstream, so was thinking
> > about adding two devicetree bindings.
> > 
> > * snps,u1_disable_as_gadget: When set the core will not enable U1 if
> > requested from host, nor initiate U1.
> > * snps,u2_disable_as_gadget: When set the core will not enable U2 if
> > requested from host, nor initiate U2.
> > 
> > If you think this might be something which can be upstreamed I will
> > prepare the code and send a patch for discussion.
> > On the other hand, if you think that disabling U1/U2 via device tree
> > as suggested should not be a feature no need for me to try making it
> > a feature.
> 
> Speaking as an interested bystander, I would first wonder why your
> hardware fails during link training.  If it is properly designed, that
> should not happen.  The fact that it does happen suggests your devices
> might also experience problems during normal data transport.

I was recently debugging a similar problem where my device would
improperly transition from U2 to U0. This caused the connection to reset
and the device to be re-enumerated as a USB2 device. Our design uses an
Intel CherryTrail z8550 SoC that has an internal dwc3 USB device
controller. I worked with Felipe a couple of weeks ago [1] to come up with
the same workaround you mentioned above. I disable device-initiated U1/U2
in ep0_set_config and also return -EINVAL when handling SetFeature
requests from the USB host. I've attached my patch for kernel 4.9.115
below for reference.

Although we have applied this workaround, we still have not identified a
root cause of the issue. Intel has no known errata related to the symptoms
we are experiencing. We've done quite a bit of analysis on our design and
are pretty confident we've followed all design guidelines for USB.

Cheers,
Rob Weber

[1] https://marc.info/?t=155349935600001&r=1&w=2
---
drivers/usb/dwc3/ep0.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)

--

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 2331469f943d..78b75d65b435 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -433,10 +433,13 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,
                return -EINVAL;

            reg = dwc3_readl(dwc->regs, DWC3_DCTL);
-            if (set)
-                reg |= DWC3_DCTL_INITU1ENA;
-            else
+            if (set) {
+                /* reg |= DWC3_DCTL_INITU1ENA; */
+                dwc3_trace(trace_dwc3_ep0, "GBX: Skipping U1 Enable");
+                return -EINVAL;
+            } else {
                reg &= ~DWC3_DCTL_INITU1ENA;
+            }
            dwc3_writel(dwc->regs, DWC3_DCTL, reg);
            break;

@@ -448,10 +451,13 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,
                return -EINVAL;

            reg = dwc3_readl(dwc->regs, DWC3_DCTL);
-            if (set)
-                reg |= DWC3_DCTL_INITU2ENA;
-            else
+            if (set) {
+                /* reg |= DWC3_DCTL_INITU2ENA; */
+                dwc3_trace(trace_dwc3_ep0, "GBX: Skipping U2 Enable");
+                return -EINVAL;
+            } else {
                reg &= ~DWC3_DCTL_INITU2ENA;
+            }
            dwc3_writel(dwc->regs, DWC3_DCTL, reg);
            break;

@@ -567,7 +573,7 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
    enum usb_device_state state = dwc->gadget.state;
    u32 cfg;
    int ret;
-    u32 reg;
+    /* u32 reg; */

    cfg = le16_to_cpu(ctrl->wValue);

@@ -594,9 +600,9 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
             * Enable transition to U1/U2 state when
             * nothing is pending from application.
             */
-            reg = dwc3_readl(dwc->regs, DWC3_DCTL);
-            reg |= (DWC3_DCTL_ACCEPTU1ENA | DWC3_DCTL_ACCEPTU2ENA);
-            dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+            /* reg = dwc3_readl(dwc->regs, DWC3_DCTL); */
+            /* reg |= (DWC3_DCTL_ACCEPTU1ENA | DWC3_DCTL_ACCEPTU2ENA); */
+            /* dwc3_writel(dwc->regs, DWC3_DCTL, reg); */
        }
        break;


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: Use device tree to disable U1/U2 in gadget devices based on DWC3
@ 2019-04-25 20:11 ` Rob Weber
  0 siblings, 0 replies; 7+ messages in thread
From: Rob Weber @ 2019-04-25 20:11 UTC (permalink / raw)
  To: Alan Stern; +Cc: Claus H. Stovgaard, balbi, linux-usb, mnarani, Michal Simek

Hi Everyone,

On Wed, Apr 24, 2019 at 02:33:48PM -0400, Alan Stern wrote:
> On Wed, 24 Apr 2019, Claus H. Stovgaard wrote:
> 
> > Hi Balbi and other USB developers.
> > 
> > I am developing camera devices based on Xilinx ZynqMP, using the
> > gadget framework and the build-in dwc3 core of the ZynqMP as USB3
> > controller and the build-in Cirrus SERDES as phy.
> > Testing with a number of hosts and Windows 7, has shown sporadic
> > reconnects when leaving U2/U1, caused by failing link training, where
> > the host resets the bus. Sometime it also means it reconnect via
> > USB2.
> > 
> > So to overcome this, I will like to have the option for disabling
> > U1/U2 on the core when working with those hosts.
> > 
> > Currently I have made a hack in ep0.c where I return EINVAL in
> > dwc3_ep0_handle_u1 and dwc3_ep0_handle_u2 together with not setting
> > DWC3_DCTL_ACCEPTU1ENA and DWC3_DCTL_ACCEPTU2ENA in
> > dwc3_ep0_set_config
> > Will though prefer a solution possible to upstream, so was thinking
> > about adding two devicetree bindings.
> > 
> > * snps,u1_disable_as_gadget: When set the core will not enable U1 if
> > requested from host, nor initiate U1.
> > * snps,u2_disable_as_gadget: When set the core will not enable U2 if
> > requested from host, nor initiate U2.
> > 
> > If you think this might be something which can be upstreamed I will
> > prepare the code and send a patch for discussion.
> > On the other hand, if you think that disabling U1/U2 via device tree
> > as suggested should not be a feature no need for me to try making it
> > a feature.
> 
> Speaking as an interested bystander, I would first wonder why your
> hardware fails during link training.  If it is properly designed, that
> should not happen.  The fact that it does happen suggests your devices
> might also experience problems during normal data transport.

I was recently debugging a similar problem where my device would
improperly transition from U2 to U0. This caused the connection to reset
and the device to be re-enumerated as a USB2 device. Our design uses an
Intel CherryTrail z8550 SoC that has an internal dwc3 USB device
controller. I worked with Felipe a couple of weeks ago [1] to come up with
the same workaround you mentioned above. I disable device-initiated U1/U2
in ep0_set_config and also return -EINVAL when handling SetFeature
requests from the USB host. I've attached my patch for kernel 4.9.115
below for reference.

Although we have applied this workaround, we still have not identified a
root cause of the issue. Intel has no known errata related to the symptoms
we are experiencing. We've done quite a bit of analysis on our design and
are pretty confident we've followed all design guidelines for USB.

Cheers,
Rob Weber

[1] https://marc.info/?t=155349935600001&r=1&w=2

---
drivers/usb/dwc3/ep0.c | 26 ++++++++++++++++----------
1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 2331469f943d..78b75d65b435 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -433,10 +433,13 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,
                return -EINVAL;

            reg = dwc3_readl(dwc->regs, DWC3_DCTL);
-            if (set)
-                reg |= DWC3_DCTL_INITU1ENA;
-            else
+            if (set) {
+                /* reg |= DWC3_DCTL_INITU1ENA; */
+                dwc3_trace(trace_dwc3_ep0, "GBX: Skipping U1 Enable");
+                return -EINVAL;
+            } else {
                reg &= ~DWC3_DCTL_INITU1ENA;
+            }
            dwc3_writel(dwc->regs, DWC3_DCTL, reg);
            break;

@@ -448,10 +451,13 @@ static int dwc3_ep0_handle_feature(struct dwc3 *dwc,
                return -EINVAL;

            reg = dwc3_readl(dwc->regs, DWC3_DCTL);
-            if (set)
-                reg |= DWC3_DCTL_INITU2ENA;
-            else
+            if (set) {
+                /* reg |= DWC3_DCTL_INITU2ENA; */
+                dwc3_trace(trace_dwc3_ep0, "GBX: Skipping U2 Enable");
+                return -EINVAL;
+            } else {
                reg &= ~DWC3_DCTL_INITU2ENA;
+            }
            dwc3_writel(dwc->regs, DWC3_DCTL, reg);
            break;

@@ -567,7 +573,7 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
    enum usb_device_state state = dwc->gadget.state;
    u32 cfg;
    int ret;
-    u32 reg;
+    /* u32 reg; */

    cfg = le16_to_cpu(ctrl->wValue);

@@ -594,9 +600,9 @@ static int dwc3_ep0_set_config(struct dwc3 *dwc, struct usb_ctrlrequest *ctrl)
             * Enable transition to U1/U2 state when
             * nothing is pending from application.
             */
-            reg = dwc3_readl(dwc->regs, DWC3_DCTL);
-            reg |= (DWC3_DCTL_ACCEPTU1ENA | DWC3_DCTL_ACCEPTU2ENA);
-            dwc3_writel(dwc->regs, DWC3_DCTL, reg);
+            /* reg = dwc3_readl(dwc->regs, DWC3_DCTL); */
+            /* reg |= (DWC3_DCTL_ACCEPTU1ENA | DWC3_DCTL_ACCEPTU2ENA); */
+            /* dwc3_writel(dwc->regs, DWC3_DCTL, reg); */
        }
        break;

--

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: Use device tree to disable U1/U2 in gadget devices based on DWC3
  2019-04-24 18:33 ` Alan Stern
@ 2019-04-26 12:44   ` Claus H. Stovgaard
  0 siblings, 0 replies; 7+ messages in thread
From: Claus H. Stovgaard @ 2019-04-26 12:44 UTC (permalink / raw)
  To: Alan Stern; +Cc: balbi, linux-usb, mnarani, Michal Simek

On Wed, 2019-04-24 at 14:33 -0400, Alan Stern wrote:
> On Wed, 24 Apr 2019, Claus H. Stovgaard wrote:
> 
> > 
> > Hi Balbi and other USB developers.
> > 
> > I am developing camera devices based on Xilinx ZynqMP, using the
> > gadget framework and the build-in dwc3 core of the ZynqMP as USB3
> > controller and the build-in Cirrus SERDES as phy.
> > Testing with a number of hosts and Windows 7, has shown sporadic
> > reconnects when leaving U2/U1, caused by failing link training,
> > where
> > the host resets the bus. Sometime it also means it reconnect via
> > USB2.
> > 
> > So to overcome this, I will like to have the option for disabling
> > U1/U2 on the core when working with those hosts.
> > 
> > Currently I have made a hack in ep0.c where I return EINVAL in
> > dwc3_ep0_handle_u1 and dwc3_ep0_handle_u2 together with not setting
> > DWC3_DCTL_ACCEPTU1ENA and DWC3_DCTL_ACCEPTU2ENA in
> > dwc3_ep0_set_config
> > Will though prefer a solution possible to upstream, so was thinking
> > about adding two devicetree bindings.
> > 
> > * snps,u1_disable_as_gadget: When set the core will not enable U1
> > if
> > requested from host, nor initiate U1.
> > * snps,u2_disable_as_gadget: When set the core will not enable U2
> > if
> > requested from host, nor initiate U2.
> > 
> > If you think this might be something which can be upstreamed I will
> > prepare the code and send a patch for discussion.
> > On the other hand, if you think that disabling U1/U2 via device
> > tree
> > as suggested should not be a feature no need for me to try making
> > it
> > a feature.
> Speaking as an interested bystander, I would first wonder why your
> hardware fails during link training.  If it is properly designed,
> that
> should not happen.  The fact that it does happen suggests your
> devices
> might also experience problems during normal data transport.

Unfortunately I don't know the root cause of the failing link training
when leaving U2 yet. I can see the link training fail with a beagle
analyzer, but have not had the possibility for looking at the signals
when it happens. Have tested our hardware with a keysight setup
(DSAV134A) resulting it being compliant according to the keysight USB
3.1 compliance test software (U7243C-1TP/SR).

To set the error rate into perspective, running several of our 100
mega-pixel camera (above 100 MB/s for each) in parallel for a 19 hour
test, resulted in 1 unit having no reconnects, 1 having 2 reconnects
and 1 having 4 reconnects. Here we constant enters and exit U0/U1/U2.

As our industrial cameras often work in pretty noisy environment like
an airplane, means we also is trying to reduce insertion loss and
improve cabling etc. to help if the issue is caused by a signal
integrity element.

In short, I am trying to figure out the root cause for the link
training failing, but as it is very hard to find, I need to pursue the
option for disabling U1/U2, as it works very effective.


> Second, the bindings you suggested would not be accepted by the 
> devicetree maintainers.  The whole idea behind devicetree is that it 
> describes the hardware, not the software.  Things like 
> u1_disable_as_gadget are software concepts and so do not belong in a 
> devicetree description.
> 
> You might be better off creating sysfs attribute files for disabling
> entry into U1 and U2.  Alternatively, if you really want to do it
> through devicetree, you should create a binding that does describe
> your
> hardware properties, such as "u1-and-u2-are-broken", or
> "broken-link-training", or something else along those lines.

Thanks. A very good point, and exactly the kind of input I hoped to
get.
Using a sysfs attribute also fit very nice with the
usb3_hardware_lpm_u1 and usb3_hardware_lpm_u2 sysfs files for enabling
/ disabling u1/u2 from the host side.
Will look into setting it via 2 files inside the configfs / usb_gadget
interface.

/Claus


> 
> Alan Stern
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Use device tree to disable U1/U2 in gadget devices based on DWC3
@ 2019-04-26 13:15 ` Claus H. Stovgaard
  0 siblings, 0 replies; 7+ messages in thread
From: Claus H. Stovgaard @ 2019-04-26 13:15 UTC (permalink / raw)
  To: Rob Weber, Alan Stern; +Cc: balbi, linux-usb, mnarani, Michal Simek

On Thu, 2019-04-25 at 13:11 -0700, Rob Weber wrote:
> Hi Everyone,
> 
> On Wed, Apr 24, 2019 at 02:33:48PM -0400, Alan Stern wrote:
> > 
> > On Wed, 24 Apr 2019, Claus H. Stovgaard wrote:
> > 
> > > 
> > > Hi Balbi and other USB developers.
> > > 
> > > I am developing camera devices based on Xilinx ZynqMP, using the
> > > gadget framework and the build-in dwc3 core of the ZynqMP as USB3
> > > controller and the build-in Cirrus SERDES as phy.
> > > Testing with a number of hosts and Windows 7, has shown sporadic
> > > reconnects when leaving U2/U1, caused by failing link training,
> > > where
> > > the host resets the bus. Sometime it also means it reconnect via
> > > USB2.
> > > 
> > > So to overcome this, I will like to have the option for disabling
> > > U1/U2 on the core when working with those hosts.
> > > 
> > > Currently I have made a hack in ep0.c where I return EINVAL in
> > > dwc3_ep0_handle_u1 and dwc3_ep0_handle_u2 together with not
> > > setting
> > > DWC3_DCTL_ACCEPTU1ENA and DWC3_DCTL_ACCEPTU2ENA in
> > > dwc3_ep0_set_config
> > > Will though prefer a solution possible to upstream, so was
> > > thinking
> > > about adding two devicetree bindings.
> > > 
> > > * snps,u1_disable_as_gadget: When set the core will not enable U1
> > > if
> > > requested from host, nor initiate U1.
> > > * snps,u2_disable_as_gadget: When set the core will not enable U2
> > > if
> > > requested from host, nor initiate U2.
> > > 
> > > If you think this might be something which can be upstreamed I
> > > will
> > > prepare the code and send a patch for discussion.
> > > On the other hand, if you think that disabling U1/U2 via device
> > > tree
> > > as suggested should not be a feature no need for me to try making
> > > it
> > > a feature.
> > Speaking as an interested bystander, I would first wonder why your
> > hardware fails during link training.  If it is properly designed,
> > that
> > should not happen.  The fact that it does happen suggests your
> > devices
> > might also experience problems during normal data transport.
> I was recently debugging a similar problem where my device would
> improperly transition from U2 to U0. This caused the connection to
> reset
> and the device to be re-enumerated as a USB2 device. Our design uses
> an
> Intel CherryTrail z8550 SoC that has an internal dwc3 USB device
> controller. I worked with Felipe a couple of weeks ago [1] to come up
> with
> the same workaround you mentioned above. I disable device-initiated
> U1/U2
> in ep0_set_config and also return -EINVAL when handling SetFeature
> requests from the USB host. I've attached my patch for kernel 4.9.115
> below for reference.
> 
> Although we have applied this workaround, we still have not
> identified a
> root cause of the issue. Intel has no known errata related to the
> symptoms
> we are experiencing. We've done quite a bit of analysis on our design
> and
> are pretty confident we've followed all design guidelines for USB.
> 
> Cheers,
> Rob Weber
> 
> [1] https://marc.info/?t=155349935600001&r=1&w=2

Thanks for this info, reading through your thread and can see many
similarities.
FYI we are also using a Type-C interface (Using the Cypress CCG4 as
controller), and using a redriver / mux. We are using the tusb1042i

Have previously asked for information from Xilinx for the dwc3 core
(the synopsys documentation), and info regarding the PHY. The serdes
module used as phy is pretty undocumented in the Xilinx documentation,
with many registers left undocumented.

Please keep me posted if you find the root cause for your problem, and
I will do the same.

/Claus

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Use device tree to disable U1/U2 in gadget devices based on DWC3
@ 2019-04-26 13:15 ` Claus H. Stovgaard
  0 siblings, 0 replies; 7+ messages in thread
From: Claus H. Stovgaard @ 2019-04-26 13:15 UTC (permalink / raw)
  To: Rob Weber, Alan Stern; +Cc: balbi, linux-usb, mnarani, Michal Simek

On Thu, 2019-04-25 at 13:11 -0700, Rob Weber wrote:
> Hi Everyone,
> 
> On Wed, Apr 24, 2019 at 02:33:48PM -0400, Alan Stern wrote:
> > 
> > On Wed, 24 Apr 2019, Claus H. Stovgaard wrote:
> > 
> > > 
> > > Hi Balbi and other USB developers.
> > > 
> > > I am developing camera devices based on Xilinx ZynqMP, using the
> > > gadget framework and the build-in dwc3 core of the ZynqMP as USB3
> > > controller and the build-in Cirrus SERDES as phy.
> > > Testing with a number of hosts and Windows 7, has shown sporadic
> > > reconnects when leaving U2/U1, caused by failing link training,
> > > where
> > > the host resets the bus. Sometime it also means it reconnect via
> > > USB2.
> > > 
> > > So to overcome this, I will like to have the option for disabling
> > > U1/U2 on the core when working with those hosts.
> > > 
> > > Currently I have made a hack in ep0.c where I return EINVAL in
> > > dwc3_ep0_handle_u1 and dwc3_ep0_handle_u2 together with not
> > > setting
> > > DWC3_DCTL_ACCEPTU1ENA and DWC3_DCTL_ACCEPTU2ENA in
> > > dwc3_ep0_set_config
> > > Will though prefer a solution possible to upstream, so was
> > > thinking
> > > about adding two devicetree bindings.
> > > 
> > > * snps,u1_disable_as_gadget: When set the core will not enable U1
> > > if
> > > requested from host, nor initiate U1.
> > > * snps,u2_disable_as_gadget: When set the core will not enable U2
> > > if
> > > requested from host, nor initiate U2.
> > > 
> > > If you think this might be something which can be upstreamed I
> > > will
> > > prepare the code and send a patch for discussion.
> > > On the other hand, if you think that disabling U1/U2 via device
> > > tree
> > > as suggested should not be a feature no need for me to try making
> > > it
> > > a feature.
> > Speaking as an interested bystander, I would first wonder why your
> > hardware fails during link training.  If it is properly designed,
> > that
> > should not happen.  The fact that it does happen suggests your
> > devices
> > might also experience problems during normal data transport.
> I was recently debugging a similar problem where my device would
> improperly transition from U2 to U0. This caused the connection to
> reset
> and the device to be re-enumerated as a USB2 device. Our design uses
> an
> Intel CherryTrail z8550 SoC that has an internal dwc3 USB device
> controller. I worked with Felipe a couple of weeks ago [1] to come up
> with
> the same workaround you mentioned above. I disable device-initiated
> U1/U2
> in ep0_set_config and also return -EINVAL when handling SetFeature
> requests from the USB host. I've attached my patch for kernel 4.9.115
> below for reference.
> 
> Although we have applied this workaround, we still have not
> identified a
> root cause of the issue. Intel has no known errata related to the
> symptoms
> we are experiencing. We've done quite a bit of analysis on our design
> and
> are pretty confident we've followed all design guidelines for USB.
> 
> Cheers,
> Rob Weber
> 
> [1] https://marc.info/?t=155349935600001&r=1&w=2

Thanks for this info, reading through your thread and can see many
similarities.
FYI we are also using a Type-C interface (Using the Cypress CCG4 as
controller), and using a redriver / mux. We are using the tusb1042i

Have previously asked for information from Xilinx for the dwc3 core
(the synopsys documentation), and info regarding the PHY. The serdes
module used as phy is pretty undocumented in the Xilinx documentation,
with many registers left undocumented.

Please keep me posted if you find the root cause for your problem, and
I will do the same.

/Claus

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-04-26 13:15 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-25 20:11 Use device tree to disable U1/U2 in gadget devices based on DWC3 Rob Weber
2019-04-25 20:11 ` Rob Weber
  -- strict thread matches above, loose matches on Subject: below --
2019-04-26 13:15 Claus H. Stovgaard
2019-04-26 13:15 ` Claus H. Stovgaard
2019-04-24 16:26 Claus H. Stovgaard
2019-04-24 18:33 ` Alan Stern
2019-04-26 12:44   ` Claus H. Stovgaard

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.