[PATCH] usb: core: usbport: fix "BUG: key not in .data" when lockdep is enabled

[Christian Lamparter]

This patch fixes a splat that happens if CONFIG_DEBUG_LOCK_ALLOC
is enabled and the ledtrig_usbport is loaded. (on a device that
has some usb ports).

[   60.695479] BUG: key c53f8420 not in .data!
[   60.695521] [ cut here ]
[   60.698542] WARNING: CPU: 1 PID: 854 at kernel/locking/lockdep.c:3134 
__kernfs_create_file+0x5c/0xc0
[   60.703355] DEBUG_LOCKS_WARN_ON(1)
[   60.712534] Modules linked in:
[   60.944078] CPU: 1 PID: 854 Comm: S96led Not tainted 4.9.44 #0
[   60.944329] Hardware name: Generic DT based system
[   60.950106] [] (unwind_backtrace) from [] 
(show_stack+0x10/0x14)
[   60.954878] [] (show_stack) from [] 
(dump_stack+0x7c/0x9c)
[   60.962772] [] (dump_stack) from [] (__warn+0xbc/0xec)
[   60.969799] [] (__warn) from [] 
(warn_slowpath_fmt+0x34/0x44)
[   60.976656] [] (warn_slowpath_fmt)
[   60.984210] [] (__kernfs_create_file)
[   60.992712] [] (sysfs_add_file_mode_ns)
[   61.002090] [] (sysfs_add_file) from
[   61.010619] [] (sysfs_add_file_to_group)
[   61.019263] [] (usbport_trig_add_usb_dev_ports [ledtrig_usbport])
[   61.031002] [] (bus_for_each_dev)
[   61.042106] [] (usb_for_each_dev)
[   61.050375] [] (usbport_trig_activate [ledtrig_usbport])
[   61.060685] [] (led_trigger_set) from []
[...]

Signed-off-by: Christian Lamparter <chunk...@googlemail.com>
---
 drivers/usb/core/ledtrig-usbport.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/usb/core/ledtrig-usbport.c 
b/drivers/usb/core/ledtrig-usbport.c
index 16c19a31dad1..910ea063caf8 100644
--- a/drivers/usb/core/ledtrig-usbport.c
+++ b/drivers/usb/core/ledtrig-usbport.c
@@ -205,6 +205,7 @@ static int usbport_trig_add_port(struct usbport_trig_data 
*usbport_data,
}
snprintf(port->port_name, len, "%s-port%d", hub_name, portnum);
 
+   sysfs_attr_init(>attr.attr);
port->attr.attr.name = port->port_name;
port->attr.attr.mode = S_IRUSR | S_IWUSR;
port->attr.show = usbport_trig_port_show;
-- 
2.14.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] usb: dwc2: Add binding for AHB burst

[Christian Lamparter]

On Tuesday, January 10, 2017 3:23:24 PM CET John Youn wrote:
> On 1/10/2017 3:03 PM, Christian Lamparter wrote:
> > On Tuesday, January 10, 2017 1:46:56 PM CET John Youn wrote:
> >> On 12/19/2016 6:49 AM, Christian Lamparter wrote:
> >>> (Lot's of old stuff, that doesn't matter anymore)
> > 
> > Hello John,
> >  
> >> This should be fixed against the latest dwc2 param rework series [1]
> >> which i hope to get queued for 4.11. If you can give it a test, that
> >> would be great.
> > 
> > oh Ok. I see you added it to
> > "[PATCH 16/21] usb: dwc2: Remove platform static params" [0].
> > Yes, I think this should work nicely. Thank you very much for
> > your time, even though it's just for like "one old board" :-).
> 
> No problem. Sorry for the delay getting the param stuff sorted.
> 
> > 
> > Do you have a public git tree with your patches that I can
> > clone/checkout?  If not, I'll take some time on the weekend
> > for this and write back on monday. But yeah, this should 
> > work.
> Yes check here on branch 'next'
> 
> https://github.com/synopsys-usb/linux.git
Ok thanks. I cloned it and built a new kernel for the thing. 

>From the (attached) bootlog:
GAHBCFG   @0xD1210008 : 0x002E
0x2E = Bit 5 | (Bit 3 | Bit 2 | Bit 1)
 = GAHBCFG_DMA_EN |
   (GAHBCFG_HBSTLEN_INCR16 << GAHBCFG_HBSTLEN_SHIFT)

I've attached an old 1GiB USB-Stick to the dwc2 managed port
and it does work as expected. The same goes for a usb-3.0 HDD
and a USB 2.0 11n WLAN stick. (All while the DWC SATA is
copying data).

Tested-by: Christian Lamparter <chunk...@googlemail.com>

Again, thank you for your work!

Regards,
Christian
---
These are the kernel messages.

dwc2 4bff8.usbotg: mapped PA bff8 to VA d121
dwc2 4bff8.usbotg: registering common handler for irq35
dwc2 4bff8.usbotg: Forcing mode to host
dwc2 4bff8.usbotg: Core Release: 2.90a (snpsid=4f54290a)
dwc2 4bff8.usbotg: Forcing mode to host
dwc2 4bff8.usbotg: DWC OTG HCD INIT
dwc2 4bff8.usbotg: hcfg=0200
dwc2 4bff8.usbotg: dwc2_core_init(ca890810)
dwc2 4bff8.usbotg: HS UTMI+ PHY selected
dwc2 4bff8.usbotg: Internal DMA Mode
dwc2 4bff8.usbotg: host_dma:1 dma_desc_enable:0
dwc2 4bff8.usbotg: Using Buffer DMA mode
dwc2 4bff8.usbotg: Host Mode
dwc2 4bff8.usbotg: DWC OTG Controller
dwc2 4bff8.usbotg: new USB bus registered, assigned bus number 1
dwc2 4bff8.usbotg: irq 35, io mem 0x
dwc2 4bff8.usbotg: DWC OTG HCD START
dwc2 4bff8.usbotg: dwc2_core_host_init(ca890810)
dwc2 4bff8.usbotg: Initializing HCFG.FSLSPClkSel to 
dwc2 4bff8.usbotg: initial grxfsiz=0213
dwc2 4bff8.usbotg: new grxfsiz=0213
dwc2 4bff8.usbotg: initial gnptxfsiz=01000213
dwc2 4bff8.usbotg: new gnptxfsiz=01000213
dwc2 4bff8.usbotg: initial hptxfsiz=01000313
dwc2 4bff8.usbotg: new hptxfsiz=01000313
dwc2 4bff8.usbotg: dwc2_core_host_init: Halt channel 0
dwc2 4bff8.usbotg: dwc2_core_host_init: Halt channel 1
dwc2 4bff8.usbotg: dwc2_core_host_init: Halt channel 2
dwc2 4bff8.usbotg: dwc2_core_host_init: Halt channel 3
dwc2 4bff8.usbotg: Init: Port Power? op_state=9
dwc2 4bff8.usbotg: Init: Power Port (0)
dwc2 4bff8.usbotg: dwc2_enable_host_interrupts()
dwc2 4bff8.usbotg: DWC OTG HCD Has Root Hub
dwc2 4bff8.usbotg: DWC OTG HCD EP RESET: bEndpointAddress=0x81
hub 1-0:1.0: USB hub found
dwc2 4bff8.usbotg: GetHubDescriptor
hub 1-0:1.0: 1 port detected
dwc2 4bff8.usbotg: GetHubStatus
dwc2 4bff8.usbotg: SetPortFeature
dwc2 4bff8.usbotg: 
dwc2 4bff8.usbotg: 

dwc2 4bff8.usbotg: HCD State:
dwc2 4bff8.usbotg:   Num channels: 4
dwc2 4bff8.usbotg:   Channel 0:
dwc2 4bff8.usbotg: dev_addr: 0, ep_num: 0, ep_is_in: 0
dwc2 4bff8.usbotg: speed: 0
dwc2 4bff8.usbotg: ep_type: 0
dwc2 4bff8.usbotg: max_packet: 0
dwc2 4bff8.usbotg: data_pid_start: 0
dwc2 4bff8.usbotg: multi_count: 0
dwc2 4bff8.usbotg: xfer_started: 0
dwc2 4bff8.usbotg: xfer_buf:   (null)
dwc2 4bff8.usbotg: xfer_dma: 
dwc2 4bff8.usbotg: xfer_len: 0
dwc2 4bff8.usbotg: xfer_count: 0
dwc2 4bff8.usbotg: halt_on_queue: 0
dwc2 4bff8.usbotg: halt_pending: 0
dwc2 4bff8.usbotg: halt_status: 0
dwc2 4bff8.usbotg: do_split: 0
dwc2 4bff8.usbotg: complete_split: 0
dwc2 4bff8.usbotg: hub_addr: 0
dwc2 4bff8.usbotg: hub_port: 0
dwc2 4bff8.usbotg: xact_pos: 0
dwc2 4bff8.usbotg: requests: 0
dwc2 4bff8.usbotg: qh:   (null)
dwc2 4bff8.usbotg:   Channel 1:
dwc2 4bff8.usbotg: dev_addr: 0, ep_num: 0, ep_is_in: 0
dwc2 4bff8.usbotg: speed: 0
dwc2 4bff8.us

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Christian Lamparter]

On Tuesday, January 10, 2017 1:46:56 PM CET John Youn wrote:
> On 12/19/2016 6:49 AM, Christian Lamparter wrote:
> > (Lot's of old stuff, that doesn't matter anymore)

Hello John,
 
> This should be fixed against the latest dwc2 param rework series [1]
> which i hope to get queued for 4.11. If you can give it a test, that
> would be great.

oh Ok. I see you added it to
"[PATCH 16/21] usb: dwc2: Remove platform static params" [0].
Yes, I think this should work nicely. Thank you very much for
your time, even though it's just for like "one old board" :-).

Do you have a public git tree with your patches that I can
clone/checkout?  If not, I'll take some time on the weekend
for this and write back on monday. But yeah, this should 
work.

Regards,
Christian

[0] <https://www.spinics.net/lists/linux-usb/msg151709.html>
--
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] usb: dwc2: Add binding for AHB burst

[Christian Lamparter]

Hello John, hello Felipe

On Monday, November 28, 2016 7:32:20 PM CET John Youn wrote:
> On 11/22/2016 12:51 PM, Christian Lamparter wrote:
> > On Monday, November 21, 2016 7:32:30 PM CET John Youn wrote:
> >> On 11/21/2016 1:10 PM, Christian Lamparter wrote:
> >>> On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
> >>>> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
> >>>>> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
> >>>>>> Also, perhaps you should allow that the compatible string can define 
> >>>>>> the 
> >>>>>> default.
> >>>>>>
> >>>>> I hoped you would say that :).
> >>>>>
> >>>>> I've attached a patch (on top of John Youn changes) [...]
> >>>>> ---
> >>>>> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for 
> >>>>> amcc,dwc-otg
> >>>>> [...]
> >>>>> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
> >>>>> +/* [...] */
> >>>>> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
> >>>>> +   {
> >>>>> +   .compatible = "amcc,dwc-otg",
> >>>>> +   .data = (void *) GAHBCFG_HBSTLEN_INCR16,
> >>>>> +   },
> >>>>> +};
> >> [...]
> >>>>>> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct 
> >>>>>> dwc2_hsotg *hsotg)
> >>>>> ret = device_property_read_string(hsotg->dev, "snps,ahb-burst", 
> >>>>> );
> >>>>> if (ret < 0) {
> >>>>> +   const struct of_device_id *match;
> >>>>> +
> >>>>> +   match = of_match_node(dwc2_compat_ahb_bursts, node);
> >>>>> +   if (match)
> >>>>> +   ret = (int)match->data;
> >>>>> +
> >> [...]
> >>>> I'd prefer if you use the binding which requires no extra code in
> >>>> dwc2.
> >>> I'm fine with either option. However it think that this would require
> >>> that either Mark or Rob would allow an exception to the "keep existing
> >>> dts the way they are) and ack the following change to the 
> >>> canyonlands.dts. 
> >> [...]
>
> Ok thanks for clearing that up. I understand.
> 
> For now we can just set the property to "INCR16" based on the
> compatible string. Perhaps in the future do this from a glue-layer
> driver which binds to all compatible strings other than "snps,dwc2".
> 
> I won't be able to do anything with this until next week though.
Ok, I think enough time has passed. I would like to see this
patch series (v3 [0]) being queued for 4.11+ together with
"usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg" [1].

Felipe, if you want I can resend the series and add the
"amcc,dwc-otg" patch to it as well. Just let me know what you
prefer here.

Regards,
Christian

[0] <https://www.mail-archive.com/linux-usb@vger.kernel.org/msg83401.html>
[1] <https://www.spinics.net/lists/linux-usb/msg149663.html>
--
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] usb: dwc2: Add binding for AHB burst

[Christian Lamparter]

On Monday, November 21, 2016 7:32:30 PM CET John Youn wrote:
> On 11/21/2016 1:10 PM, Christian Lamparter wrote:
> > On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
> >> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
> >>> On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
> >>>> Also, perhaps you should allow that the compatible string can define the 
> >>>> default.
> >>>>
> >>> I hoped you would say that :).
> >>>
> >>> I've attached a patch (on top of John Youn changes) [...]
> >>> ---
> >>> Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for 
> >>> amcc,dwc-otg
> >>> [...]
> >>> @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
> >>> +/* [...] */
> >>> +static const struct of_device_id dwc2_compat_ahb_bursts[] = {
> >>> + {
> >>> + .compatible = "amcc,dwc-otg",
> >>> + .data = (void *) GAHBCFG_HBSTLEN_INCR16,
> >>> + },
> >>> +};
> [...]
> > >>> @@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct 
> > >>> dwc2_hsotg *hsotg)
> >>>   ret = device_property_read_string(hsotg->dev, "snps,ahb-burst", );
> >>>   if (ret < 0) {
> >>> + const struct of_device_id *match;
> >>> +
> >>> + match = of_match_node(dwc2_compat_ahb_bursts, node);
> >>> + if (match)
> >>> + ret = (int)match->data;
> >>> +
> [...]
> >> I'd prefer if you use the binding which requires no extra code in
> >> dwc2.
> > I'm fine with either option. However it think that this would require
> > that either Mark or Rob would allow an exception to the "keep existing
> > dts the way they are) and ack the following change to the canyonlands.dts. 
> 
> I don't know about that. Under what circumstance can the dts change?
As far as I know, the justification for not changing the DTS is that a
compiled DTB might be stored in an read-only ROM on a board. So it would
be impossible to update it. Hence, the driver have work with the existing
(and sometimes buggy or incomplete) information to stay compatible.

(Note: Thankfully, the canyonlands dtb is stored in flash, it's possible
to update it. But it is an extra step that's not done automatically
with make install). 

> The canyonlands dts was binding to an external vendor driver. So it
> wasn't documented nor expected to work with dwc2 until your recent
> patch adding the compatible string.

Oh, no that's not what happend. Let me explain why there was no "external 
vendor driver": AMCC/APM were planing to upstream their hole platform. And
in fact, the devs tried very hard to include their driver back in 2011 [0].
But this driver was denied inclusion back then due to:

"[...]
I would also like to point out that the same Synopsys USB controller
is used in a number of other SoCs (especially ARM chips), and
supported by other drivers, some of these even in mainline.

See http://thread.gmane.org/gmane.linux.usb.general/61714/focus=62139
for a related thread.

Instead of trying to add a completely new driver to mainline (and one
which has been repeatedly been rejected), I vote for focusing on the
existing driver code that is already in mainline, and testing and
improving this so we can use a single implementation of this driver
code for all SoCs that use the same IP block." [1]

Of course: The listed link goes the "USB Host driver for i.MX28" driver.
And this is an ehci-hcd like driver... Which is as you are well aware not
that similar to the dwc2 OTG. And as far as I can tell: AMCC abandoned
the patch series right there. 

Note: AMCC did however succeed in pushing your employer's Synopsys
DesignWare SATA and DMA drivers to the kernel back then. And I'm happy
to report that both drivers are still around and working fine for the 460EX 
(sata_dwc_460ex.c[2] and the DW AHB DMA [3]). (The drivers also work for
different platforms than the original PPC. I know that because I helped
Andy Shevchenko with testing and pushing some fixes to it when he was
adding support for the Intel Quark SoC, which uses the DWC SATA and DMA).

So Please?
> Systems that use the vendor driver will still work with the dts. If
> you remove the vendor driver and configure it to use dwc2, it won't
> work due to a quirk of the canyonlands hardware, for which you need to
> add a dts property.
Sadly, there is no up to date vendor driver. The canyonlands.dts binding
is still in place and the hardware works fine. I'm interested in this
platform since it is a cheap BigEndian system which is useful for usb
driver d

Re: [PATCH v2 2/4] usb: dwc2: Add binding for AHB burst

[Christian Lamparter]

Hello John,

On Monday, November 21, 2016 12:16:31 PM CET John Youn wrote:
> On 11/18/2016 12:18 PM, Christian Lamparter wrote:
> > On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
> >> On Thu, Nov 17, 2016 at 04:35:10PM +0100, Stefan Wahren wrote:
> >>> Hi John,
> >>>
> >>> Am 17.11.2016 um 00:47 schrieb John Youn:
> >>>> Add the "snps,ahb-burst" binding and read it in.
> >>>>
> >>>> This property controls which burst type to perform on the AHB bus as a
> >>>> master in internal DMA mode. This overrides the legacy param value,
> >>>> which we need to keep around for now since several platforms use it.
> >>>>
> >>>> Some platforms may see better or worse performance based on this
> >>>> value. The HAPS platform is one example where all INCRx have worse
> >>>> performance than INCR.
> >>>>
> >>>> Other platforms (such as the Canyonlands board) report that the default
> >>>> value causes system hangs.
> >>>>
> >>>> Signed-off-by: John Youn <johny...@synopsys.com>
> >>>> Cc: Christian Lamparter <chunk...@googlemail.com>
> >>>> ---
> >>>>  Documentation/devicetree/bindings/usb/dwc2.txt |  2 +
> >>>>  drivers/usb/dwc2/core.h|  9 +
> >>>>  drivers/usb/dwc2/params.c  | 56 
> >>>> ++
> >>>>  3 files changed, 67 insertions(+)
> >>>>
> >>>> diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt 
> >>>> b/Documentation/devicetree/bindings/usb/dwc2.txt
> >>>> index 6c7c2bce..9e7b4b4 100644
> >>>> --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> >>>> +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> >>>
> >>> according to Documentation/devicetree/bindings/submitting-patches.txt
> >>> this change should be a separate patch.
> >>>
> >>>> @@ -26,6 +26,8 @@ Optional properties:
> >>>>  Refer to phy/phy-bindings.txt for generic phy consumer properties
> >>>>  - dr_mode: shall be one of "host", "peripheral" and "otg"
> >>>>Refer to usb/generic.txt
> >>>> +- snps,ahb-burst: specifies the ahb burst length. Valid arguments are:
> >>>> +  "SINGLE", "INCR", "INCR4", "INCR8", "INCR16". Defaults to "INCR4".
> >>>
> >>> This doesn't apply in case of the bcm2835. I would prefer this option is
> >>> ignored in that case with a dev_warn("snps,ahb-burst is not supported on
> >>> this platform").
> >>
> >> Also, perhaps you should allow that the compatible string can define the 
> >> default.
> >>
> > I hoped you would say that :).
> > 
> > I've attached a patch (on top of John Youn changes) that does
> > just that for the amcc,dwc-otg. I put the GAHBCFG_HBSTLEN_INCR
> > value into the .data, if that's a problem, I can certainly 
> > respin the patch and put it in a dedicated struct.
> > 
> > Regards
> > 
> > Christian
> > ---
> > From 4c31a029dde714828810b1c3e61a5b1412ac939a Mon Sep 17 00:00:00 2001
> > From: Christian Lamparter <chunk...@gmail.com>
> > Date: Fri, 18 Nov 2016 21:03:19 +0100
> > Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg
> > 
> > This patch adds a of_device_id table which can be used by
> > existing devices to supply a ahb-burst value for the platform
> > without having to add a "snps,ahb-burst" entry to the dts.
> > 
> > Note: Adding new devices to this table is discouraged.
> >   please consider adding the "snps,ahb-burst" property
> >   with the correct configuration to your device tree
> >   file instead.
> > 
> > Signed-off-by: Christian Lamparter <chunk...@gmail.com>
> > ---
> >  drivers/usb/dwc2/params.c | 22 ++
> >  1 file changed, 22 insertions(+)
> > 
> > diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> > index e0fc9aa..51be266 100644
> > --- a/drivers/usb/dwc2/params.c
> > +++ b/drivers/usb/dwc2/params.c
> > @@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
> > [GAHBCFG_HBSTLEN_INCR16]= "INCR16",
> >  };
> >  
> > +/*
> >

Re: [PATCH v3 0/4] usb: dwc2: Add AHB burst configuration

[Christian Lamparter]

On Thursday, November 17, 2016 12:52:14 PM CET John Youn wrote:
> This series adds a binding for AHB burst, reads it in, and configures
> the controller for the specified burst type.
> 
> Tested on HAPS platform with DWC_hsotg IP version 3.30a.
> 
> v3:
> * Split out binding documentation
> * Squash bcm2835 comment with binding implementation
> * Add warning for bcm2835
> 
> v2:
> * Don't remove the bcm2835 ahbcfg param and document why.
> 
> 
> John Youn (4):
>   Documentation: devictree: dwc2: Add AHB burst binding
>   usb: dwc2: Read in the AHB burst property
>   usb: dwc2: Use the ahb_burst param
>   usb: dwc2: pci: Add AHB burst property for HAPS
> 
>  Documentation/devicetree/bindings/usb/dwc2.txt |  2 +
>  drivers/usb/dwc2/core.h|  9 +++
>  drivers/usb/dwc2/gadget.c  |  2 +-
>  drivers/usb/dwc2/hcd.c |  8 +--
>  drivers/usb/dwc2/params.c  | 79 
> ++
>  drivers/usb/dwc2/pci.c     |  1 +
>  6 files changed, 85 insertions(+), 16 deletions(-)
> 
Tested-by: Christian Lamparter <chunk...@gmail.com>


--
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] usb: dwc2: Add binding for AHB burst

[Christian Lamparter]

On Friday, November 18, 2016 8:16:08 AM CET Rob Herring wrote:
> On Thu, Nov 17, 2016 at 04:35:10PM +0100, Stefan Wahren wrote:
> > Hi John,
> > 
> > Am 17.11.2016 um 00:47 schrieb John Youn:
> > > Add the "snps,ahb-burst" binding and read it in.
> > >
> > > This property controls which burst type to perform on the AHB bus as a
> > > master in internal DMA mode. This overrides the legacy param value,
> > > which we need to keep around for now since several platforms use it.
> > >
> > > Some platforms may see better or worse performance based on this
> > > value. The HAPS platform is one example where all INCRx have worse
> > > performance than INCR.
> > >
> > > Other platforms (such as the Canyonlands board) report that the default
> > > value causes system hangs.
> > >
> > > Signed-off-by: John Youn <johny...@synopsys.com>
> > > Cc: Christian Lamparter <chunk...@googlemail.com>
> > > ---
> > >  Documentation/devicetree/bindings/usb/dwc2.txt |  2 +
> > >  drivers/usb/dwc2/core.h|  9 +
> > >  drivers/usb/dwc2/params.c  | 56 
> > > ++
> > >  3 files changed, 67 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt 
> > > b/Documentation/devicetree/bindings/usb/dwc2.txt
> > > index 6c7c2bce..9e7b4b4 100644
> > > --- a/Documentation/devicetree/bindings/usb/dwc2.txt
> > > +++ b/Documentation/devicetree/bindings/usb/dwc2.txt
> > 
> > according to Documentation/devicetree/bindings/submitting-patches.txt
> > this change should be a separate patch.
> > 
> > > @@ -26,6 +26,8 @@ Optional properties:
> > >  Refer to phy/phy-bindings.txt for generic phy consumer properties
> > >  - dr_mode: shall be one of "host", "peripheral" and "otg"
> > >Refer to usb/generic.txt
> > > +- snps,ahb-burst: specifies the ahb burst length. Valid arguments are:
> > > +  "SINGLE", "INCR", "INCR4", "INCR8", "INCR16". Defaults to "INCR4".
> > 
> > This doesn't apply in case of the bcm2835. I would prefer this option is
> > ignored in that case with a dev_warn("snps,ahb-burst is not supported on
> > this platform").
> 
> Also, perhaps you should allow that the compatible string can define the 
> default.
> 
I hoped you would say that :).

I've attached a patch (on top of John Youn changes) that does
just that for the amcc,dwc-otg. I put the GAHBCFG_HBSTLEN_INCR
value into the .data, if that's a problem, I can certainly 
respin the patch and put it in a dedicated struct.

Regards

Christian
---
>From 4c31a029dde714828810b1c3e61a5b1412ac939a Mon Sep 17 00:00:00 2001
From: Christian Lamparter <chunk...@gmail.com>
Date: Fri, 18 Nov 2016 21:03:19 +0100
Subject: [PATCH] usb: dwc2: add a default ahb-burst setting for amcc,dwc-otg

This patch adds a of_device_id table which can be used by
existing devices to supply a ahb-burst value for the platform
without having to add a "snps,ahb-burst" entry to the dts.

Note: Adding new devices to this table is discouraged.
  please consider adding the "snps,ahb-burst" property
  with the correct configuration to your device tree
  file instead.

Signed-off-by: Christian Lamparter <chunk...@gmail.com>
---
 drivers/usb/dwc2/params.c | 22 ++
 1 file changed, 22 insertions(+)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index e0fc9aa..51be266 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1097,6 +1097,22 @@ static const char *const ahb_bursts[] = {
[GAHBCFG_HBSTLEN_INCR16]= "INCR16",
 };
 
+/*
+ * This table provides AHB burst configuration for existing
+ * device tree bindings that work poorly with the default setting.
+ *
+ * Note: Adding new devices to this table is discouraged.
+ *  please consider adding the "snps,ahb-burst" property
+ *  with the correct configuration to your device tree
+ *  file instead.
+ */
+static const struct of_device_id dwc2_compat_ahb_bursts[] = {
+   {
+   .compatible = "amcc,dwc-otg",
+   .data = (void *) GAHBCFG_HBSTLEN_INCR16,
+   },
+};
+
 static int dwc2_get_property_ahb_burst(struct dwc2_hsotg *hsotg)
 {
struct device_node *node = hsotg->dev->of_node;
@@ -1107,6 +1123,12 @@ static int dwc2_get_property_ahb_burst(struct dwc2_hsotg 
*hsotg)
ret = device_property_read_string(hsotg->dev,
  "snps,ahb-burst", );
if (ret < 0) {
+   const struct of_device_id *match;
+
+   match = of_match_node(dwc2_compat_ahb_bursts, node);
+   if (match)
+   ret = (int)match->data;
+
return ret;
} else if (of_device_is_compatible(node, "brcm,bcm2835-usb")) {
dev_warn(hsotg->dev,
-- 
2.10.2





--
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 1/2] usb: dwc2: add amcc,dwc-otg support

[Christian Lamparter]

On Friday, November 11, 2016 2:20:42 PM CET John Youn wrote:
> On 11/11/2016 2:05 PM, Christian Lamparter wrote:
> > On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
> >> On 11/11/2016 12:59 PM, Christian Lamparter wrote:
> >>> This patch adds support for the "amcc,usb-otg" device
> >>> which is found in the PowerPC Canyonlands' dts.
> >>>
> >>> The device definition was added by:
> >>> commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands 
> >>> board")'
> >>> but without any driver support as the dwc2 driver wasn't
> >>> available at that time.
> >>>
> >>> Note: The system can't use the generic "snps,dwc2" compatible
> >>> because of the special ahbcfg configuration. The default
> >>> GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
> >>> when the USB and SATA is used concurrently.
> >>
> >> I don't want to add any more of these param structures to the driver
> >> unless really necessary. We're trying to remove usage of them in favor
> >> of using auto-detected defaults and device properties to override
> >> them.
> > Ok, thanks. I think that would work. I've attached an updated patch.
> > Can it be applied/queued now? Or do you want me to resent it later?
> > 
> >> The AHB Burst is actually one of the ones we were going to do next
> >> because our platform also doesn't work well with INCR4. In fact I'm
> >> thinking of making the default INCR.
> > Is that actually possible to change the default still? This would
> > require to re-evaluate all existing archs/platforms that use 
> > "snps,dwc2" for INCR16 compatibility. 
> 
> INCR, not INCR16, but you're right, so we may not change it even
> though though INCR is usually the right choice over INCR4.
What about making a device-tree property?

Recommended properties:
 - g-ahb-bursts : specifies the ahb bursts length, should be one of
   "single", "INCRx", "INCR4", "INCR8", or "INCR16". If not specified
   the safer but inefficient "INCR4" is used. The optimal setting is
   "INCRx".

Would this work? If so, I can make a patch over the weekend.
> Anyways, with the binding, can't you just set the compatible string to
> snps,dwc2?

Ah, let me explain. I had a discussion with Mark Rutland and Rob Herring
a while back about device-tree bindings.

They made it very clear to me, that they don't want any generic "catch all
compatible" strings:

"Bindings should be for hardware (either specific device models, or for
classes), and not for Linux drivers. The latter is subject to arbitrary
changes while the former is not, as old hardware continues to exist and
does not change while drivers get completely reworked." [0]

Furthermore, this is an existing binding in kernel's canyonlands.dts [1]
and this binding can't be easily changed. Rob Herring explained this in
the context of the "basic-mmio-gpio" patch [2] when I was editing the dts
to make them work with the changes I made:

"You can't remove the old drivers as they are needed to work with 
old dtbs, so there is no gain.

You would need to match on existing compatibles such as
moxa,moxart-gpio and provide a match data struct that has all the info
you are adding here (e.g. data register offset). Then additionally you
could add "basic-mmio-gpio" (I would drop "basic" part) and the
additional data associated with it. But it has to be new properties,
not changing properties. Changing the reg values doesn't work."

So, for this to work with the existing canyonlands.dts, I need to have
the "amcc,dwc-otg" compatible string.

Of course, it would be great to hear from Rob Herring and/or Mark Rutland
about this case.

Regards,
Christian

[0] <https://patchwork.kernel.org/patch/8976221/>
[1] 
<http://lxr.free-electrons.com/source/arch/powerpc/boot/dts/canyonlands.dts#L181>
[2] <http://www.spinics.net/lists/devicetree/msg124538.html>

 
> > 
> > From what I can tell based would be:
> > bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
> > stratix10, meson-gxbb, rt3050 and some Altera FPGAs.
> > 
> >> If that's all you need then a devicetree binding should be enough
> >> right?
> > Yes. The device is working fine so far.
> > 
> > Regards,
> > Christian
> > 
> > ---
> > From 70dd4be016b89655a56bc8260f04683b50f07644 Mon Sep 17 00:00:00 2001
> > From: Christian Lamparter <chunk...@gmail.com>
> > Date: Sun, 6 Nov 2016 00:39:24 +0100
> > Subject: [PATCH] usb: dwc2: add amcc,dwc-otg 

Re: [PATCH 1/2] usb: dwc2: add amcc,dwc-otg support

[Christian Lamparter]

Hello,

On Friday, November 11, 2016 1:22:16 PM CET John Youn wrote:
> On 11/11/2016 12:59 PM, Christian Lamparter wrote:
> > This patch adds support for the "amcc,usb-otg" device
> > which is found in the PowerPC Canyonlands' dts.
> > 
> > The device definition was added by:
> > commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands 
> > board")'
> > but without any driver support as the dwc2 driver wasn't
> > available at that time.
> > 
> > Note: The system can't use the generic "snps,dwc2" compatible
> > because of the special ahbcfg configuration. The default
> > GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
> > when the USB and SATA is used concurrently.
> 
> I don't want to add any more of these param structures to the driver
> unless really necessary. We're trying to remove usage of them in favor
> of using auto-detected defaults and device properties to override
> them.
Ok, thanks. I think that would work. I've attached an updated patch.
Can it be applied/queued now? Or do you want me to resent it later?

> The AHB Burst is actually one of the ones we were going to do next
> because our platform also doesn't work well with INCR4. In fact I'm
> thinking of making the default INCR.
Is that actually possible to change the default still? This would
require to re-evaluate all existing archs/platforms that use 
"snps,dwc2" for INCR16 compatibility. 

>From what I can tell based would be:
bcm11351, bcm21664, bcm23550, exynos3250, stm32f429, rk3xxx,
stratix10, meson-gxbb, rt3050 and some Altera FPGAs.

> If that's all you need then a devicetree binding should be enough
> right?
Yes. The device is working fine so far.

Regards,
Christian

---
>From 70dd4be016b89655a56bc8260f04683b50f07644 Mon Sep 17 00:00:00 2001
From: Christian Lamparter <chunk...@gmail.com>
Date: Sun, 6 Nov 2016 00:39:24 +0100
Subject: [PATCH] usb: dwc2: add amcc,dwc-otg support

This patch adds support for the "amcc,usb-otg" device
which is found in the PowerPC Canyonlands' dts.

The device definition was added by:
commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
but without any driver support as the dwc2 driver wasn't
available at that time.

Note: The system can't use the generic "snps,dwc2" compatible
because of the special ahbcfg configuration. The default
GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
when the USB and SATA is used concurrently.

Cc: Felipe Balbi <felipe.ba...@linux.intel.com>
Cc: John Youn <johny...@synopsys.com>
Signed-off-by: Christian Lamparter <chunk...@gmail.com>
---
v1->v2:
- moved definitons to params.c
- removed dma_enable / host_dma parameter
- added dma_desc_fs_enable parameter
v2->v3:
- removed parameters

Please queue this patch until GAHBCFG_HBSTLEN_INCR16 is the default
for ahbcfg.
---
 Documentation/devicetree/bindings/usb/dwc2.txt | 1 +
 drivers/usb/dwc2/params.c  | 1 +
 2 files changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt 
b/Documentation/devicetree/bindings/usb/dwc2.txt
index 10a2a4b..6ccfe85 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -12,6 +12,7 @@ Required properties:
   - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
   - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b 
SoCs;
   - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 
SoCs;
+  - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX 
SoCs;
   - snps,dwc2: A generic DWC2 USB controller with default parameters.
 - reg : Should contain 1 register range (address and length)
 - interrupts : Should contain 1 interrupt
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 64d5c66..9506ab0 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -239,6 +239,7 @@ const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
{ .compatible = "amlogic,meson8b-usb", .data = _amlogic },
{ .compatible = "amlogic,meson-gxbb-usb", .data = _amlogic },
+   { .compatible = "amcc,dwc-otg", .data = NULL },
{},
 };
 MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
-- 
2.10.2


--
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 2/2] usb: dwc2: fixes host_dma logic

[Christian Lamparter]

This patch moves the the host_dma initialization
before dwc2_set_param_dma_desc_enable and
dwc2_set_param_dma_desc_fs_enable. The reason being
that both function need it.

Fixes: 1205489cee75bf39 ("usb: dwc2: Get host DMA device properties")

Cc: John Youn <johny...@synopsys.com>
Cc: Felipe Balbi <felipe.ba...@linux.intel.com>
Signed-off-by: Christian Lamparter <chunk...@gmail.com>
---
 drivers/usb/dwc2/params.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 5d822c5..222a83c 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -1157,9 +1157,6 @@ static void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
bool dma_capable = !(hw->arch == GHWCFG2_SLAVE_ONLY_ARCH);
 
dwc2_set_param_otg_cap(hsotg, params->otg_cap);
-   dwc2_set_param_dma_desc_enable(hsotg, params->dma_desc_enable);
-   dwc2_set_param_dma_desc_fs_enable(hsotg, params->dma_desc_fs_enable);
-
if ((hsotg->dr_mode == USB_DR_MODE_HOST) ||
(hsotg->dr_mode == USB_DR_MODE_OTG)) {
bool disable;
@@ -1174,6 +1171,8 @@ static void dwc2_set_parameters(struct dwc2_hsotg *hsotg,
!disable, false,
dma_capable);
}
+   dwc2_set_param_dma_desc_enable(hsotg, params->dma_desc_enable);
+   dwc2_set_param_dma_desc_fs_enable(hsotg, params->dma_desc_fs_enable);
 
dwc2_set_param_host_support_fs_ls_low_power(hsotg,
params->host_support_fs_ls_low_power);
-- 
2.10.2

--
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 1/2] usb: dwc2: add amcc,dwc-otg support

[Christian Lamparter]

This patch adds support for the "amcc,usb-otg" device
which is found in the PowerPC Canyonlands' dts.

The device definition was added by:
commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands board")'
but without any driver support as the dwc2 driver wasn't
available at that time.

Note: The system can't use the generic "snps,dwc2" compatible
because of the special ahbcfg configuration. The default
GAHBCFG_HBSTLEN_INCR4 of snps,dwc2 can cause a system hang
when the USB and SATA is used concurrently.

Cc: Felipe Balbi <felipe.ba...@linux.intel.com>
Cc: John Youn <johny...@synopsys.com>
Signed-off-by: Christian Lamparter <chunk...@gmail.com>
---
v1->v2
- moved definitons to params.c
- removed dma_enable / host_dma parameter
- added dma_desc_fs_enable parameter
---
 Documentation/devicetree/bindings/usb/dwc2.txt |  1 +
 drivers/usb/dwc2/params.c  | 33 ++
 2 files changed, 34 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt 
b/Documentation/devicetree/bindings/usb/dwc2.txt
index 10a2a4b..6ccfe85 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -12,6 +12,7 @@ Required properties:
   - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
   - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b 
SoCs;
   - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 
SoCs;
+  - "amcc,dwc-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX 
SoCs;
   - snps,dwc2: A generic DWC2 USB controller with default parameters.
 - reg : Should contain 1 register range (address and length)
 - interrupts : Should contain 1 interrupt
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 64d5c66..5d822c5 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -192,6 +192,38 @@ static const struct dwc2_core_params params_amlogic = {
.hibernation= -1,
 };
 
+static const struct dwc2_core_params params_amcc_dwc_otg = {
+   .otg_cap= DWC2_CAP_PARAM_HNP_SRP_CAPABLE,
+   .otg_ver= -1,
+   .dma_desc_enable= -1,
+   .dma_desc_fs_enable = -1,
+   .speed  = -1,
+   .enable_dynamic_fifo= -1,
+   .en_multiple_tx_fifo= -1,
+   .host_rx_fifo_size  = -1,
+   .host_nperio_tx_fifo_size   = -1,
+   .host_perio_tx_fifo_size= -1,
+   .max_transfer_size  = -1,
+   .max_packet_count   = -1,
+   .host_channels  = -1,
+   .phy_type   = -1,
+   .phy_utmi_width = -1,
+   .phy_ulpi_ddr   = -1,
+   .phy_ulpi_ext_vbus  = -1,
+   .i2c_enable = -1,
+   .ulpi_fs_ls = -1,
+   .host_support_fs_ls_low_power   = -1,
+   .host_ls_low_power_phy_clk  = -1,
+   .ts_dline   = -1,
+   .reload_ctl = -1,
+   /* Avoid system hang during concurrently using USB and SATA */
+   .ahbcfg = GAHBCFG_HBSTLEN_INCR16 <<
+ GAHBCFG_HBSTLEN_SHIFT,
+   .uframe_sched   = -1,
+   .external_id_pin_ctl= -1,
+   .hibernation= -1,
+};
+
 static const struct dwc2_core_params params_default = {
.otg_cap= -1,
.otg_ver= -1,
@@ -239,6 +271,7 @@ const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
{ .compatible = "amlogic,meson8b-usb", .data = _amlogic },
{ .compatible = "amlogic,meson-gxbb-usb", .data = _amlogic },
+   { .compatible = "amcc,dwc-otg", .data = _amcc_dwc_otg },
{},
 };
 MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
-- 
2.10.2

--
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] usb: dwc2: add amcc,dwc-otg device tree definition

[Christian Lamparter]

This patch adds support for the "amcc,usb-otg" device
which is found in the PowerPC Canyonlands' dts.

The device definition was added by:
commit c89b3458d8cc ("powerpc/44x: Add USB DWC DTS entry to Canyonlands 
board")'.
AMCC produced a standalone driver that was sent to the
linuxppc-dev at the time. However, it was never integrated.

Signed-off-by: Christian Lamparter <chunk...@gmail.com>
---
For anyone interested: the driver was sent to the ML multiple times back
in 2012 [0], [1].

[0] <https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-May/097847.html>
[1] <https://lists.ozlabs.org/pipermail/linuxppc-dev/2012-May/097938.html>
---
 Documentation/devicetree/bindings/usb/dwc2.txt |  1 +
 drivers/usb/dwc2/platform.c| 33 ++
 2 files changed, 34 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc2.txt 
b/Documentation/devicetree/bindings/usb/dwc2.txt
index 2c30a54..73c4dc5 100644
--- a/Documentation/devicetree/bindings/usb/dwc2.txt
+++ b/Documentation/devicetree/bindings/usb/dwc2.txt
@@ -12,6 +12,7 @@ Required properties:
   - "lantiq,xrx200-usb": The DWC2 USB controller instance in Lantiq XRX SoCs;
   - "amlogic,meson8b-usb": The DWC2 USB controller instance in Amlogic Meson8b 
SoCs;
   - "amlogic,meson-gxbb-usb": The DWC2 USB controller instance in Amlogic S905 
SoCs;
+  - "amcc,usb-otg": The DWC2 USB controller instance in AMCC Canyonlands 460EX 
SoCs;
   - snps,dwc2: A generic DWC2 USB controller with default parameters.
 - reg : Should contain 1 register range (address and length)
 - interrupts : Should contain 1 interrupt
diff --git a/drivers/usb/dwc2/platform.c b/drivers/usb/dwc2/platform.c
index dde08d5..8230037 100644
--- a/drivers/usb/dwc2/platform.c
+++ b/drivers/usb/dwc2/platform.c
@@ -214,6 +214,38 @@ static const struct dwc2_core_params params_amlogic = {
.hibernation= -1,
 };
 
+static const struct dwc2_core_params params_amcc_dwc_otg = {
+   .otg_cap= DWC2_CAP_PARAM_HNP_SRP_CAPABLE,
+   .otg_ver= -1,
+   .dma_enable = -1,
+   .dma_desc_enable= -1,
+   .speed  = -1,
+   .enable_dynamic_fifo= -1,
+   .en_multiple_tx_fifo= -1,
+   .host_rx_fifo_size  = -1,
+   .host_nperio_tx_fifo_size   = -1,
+   .host_perio_tx_fifo_size= -1,
+   .max_transfer_size  = -1,
+   .max_packet_count   = -1,
+   .host_channels  = -1,
+   .phy_type   = -1,
+   .phy_utmi_width = -1,
+   .phy_ulpi_ddr   = -1,
+   .phy_ulpi_ext_vbus  = -1,
+   .i2c_enable = -1,
+   .ulpi_fs_ls = -1,
+   .host_support_fs_ls_low_power   = -1,
+   .host_ls_low_power_phy_clk  = -1,
+   .ts_dline   = -1,
+   .reload_ctl = -1,
+   /* Avoid system hang during concurrently using USB and SATA */
+   .ahbcfg = GAHBCFG_HBSTLEN_INCR16 <<
+ GAHBCFG_HBSTLEN_SHIFT,
+   .uframe_sched   = -1,
+   .external_id_pin_ctl= -1,
+   .hibernation= -1,
+};
+
 /*
  * Check the dr_mode against the module configuration and hardware
  * capabilities.
@@ -521,6 +553,7 @@ static const struct of_device_id dwc2_of_match_table[] = {
{ .compatible = "samsung,s3c6400-hsotg", .data = NULL},
{ .compatible = "amlogic,meson8b-usb", .data = _amlogic },
{ .compatible = "amlogic,meson-gxbb-usb", .data = _amlogic },
+   { .compatible = "amcc,dwc-otg", .data = _amcc_dwc_otg },
{},
 };
 MODULE_DEVICE_TABLE(of, dwc2_of_match_table);
-- 
2.10.2

--
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: xhci: handle uPD720201 and uPD720202 w/o ROM

[Christian Lamparter]

Hello,

On Tuesday, June 21, 2016 05:56:58 AM Yoshihiro Shimoda wrote:
> > From: Christian Lamparter
> > Sent: Tuesday, June 21, 2016 12:32 AM
> > 
> > On Wednesday, June 08, 2016 12:14:57 AM Christian Lamparter wrote:
> > > This patch adds a firmware check for the uPD720201K8-711-BAC-A
> > > and uPD720202K8-711-BAA-A variant. Both of these chips are listed
> > > in Renesas' R19UH0078EJ0500 Rev.5.00 "User's Manual: Hardware" as
> > > devices which need a firmware in order to work as they do not have
> > > support to load the firmware from an external ROM.
> > >
> > > Currently, the xhci-pci driver is unable to initialize the hcd in
> > > this case. Instead it will wait for 30 seconds and cause a timeout
> > > in xhci_handshake() and fails.
> > >
> > > [5.116990] xhci_hcd :45:00.0: new USB bus registered ...
> > > [   32.335215] xhci_hcd :45:00.0: can't setup: -110
> > > [   32.340179] xhci_hcd :45:00.0: USB bus 2 deregistered
> > > [   32.345587] xhci_hcd :45:00.0: init :45:00.0 fail, -110
> > > [   32.351496] xhci_hcd: probe of :45:00.0 failed with error -110
> > >
> > > Cc: Yoshihiro Shimoda <yoshihiro.shimoda...@renesas.com>
> > > Signed-off-by: Christian Lamparter <chunk...@gmail.com>
> > Hello?
> > 
> > Are there any news on this? Or is there anything else that I'm missing
> > which blocks this patch? If it's because the device won't be working
> > either with this patch, then please let me know.
> 
> Thank you for the patch with CC me.
> However, I'm afraid but I don't know the detail of the Renesas xHCI PCI 
> controller.
> (My job is for R-Car environment for now.)
Oh. I see. Still, I would like to thank you for your input. I looked at
the rcar loader and firmware: The firmware header (aa55 + firmware version
pointer at 0x14) are carbon-copies of the pci parts (or vice versa?). I guess
I shouldn't be surprised, I bet the rcar and uPD both have a similar SuperH.
The firmware upload mechanism itself seems to be a bit different though, the
spec for the pci devices lists much more stuff to check and verify. Also, the
pci variant uploades 2 x 32bit packages at a time (via data0 and data1 in the
pci config space). However, the procedure is still somewhat identical.

Anyway, if you want to take a look: I went ahead and wrote a firmware loader
for the uPD720201 and uPD720202: I have attached in the patch below. The device
now works for this APM82181 (PowerPC 464 - Big Endian).

xhci_hcd :45:00.0: xHCI Host Controller
xhci_hcd :45:00.0: new USB bus registered, assigned bus number 2
xhci_hcd :45:00.0: hcc params 0x014051cf hci version 0x100 quirks 0x0190
xhci_hcd :45:00.0: xHCI Host Controller
xhci_hcd :45:00.0: new USB bus registered, assigned bus number 3
usb usb3: We don't know the algorithms for LPM for this host, disabling LPM.
hub 3-0:1.0: USB hub found
hub 3-0:1.0: 2 ports detected
usb 3-1: new SuperSpeed USB device number 2 using xhci_hcd
usb-storage 3-1:1.0: USB Mass Storage device detected
scsi host1: usb-storage 3-1:1.0
scsi 1:0:0:0: Direct-Access Intenso  Slim LinePMAP PQ: 0 ANSI: 6
sd 1:0:0:0: [sdb] 15466496 512-byte logical blocks: (7.92 GB/7.38 GiB)
...

> By the way, the issue seems similar with R-Car environment though :)
> http://thread.gmane.org/gmane.linux.kernel.stable/175457/focus=140699
> 
> and fix patch for it:
> http://thread.gmane.org/gmane.linux.kernel.stable/177524
Yes :). Same thing here. Load the firmware and the device is happy.
I know this is a big ask: Do you know or can you give me a lead
to whom would be willing to help/support the uPD720201/2? It would
be great if the firmware could be added to linux-firmware.git.

Note: This is one way to do it. But there are many more... I think adding
a pci_enable quirk to drivers/usb/host/pci-quirk.c is the least invasive
way... unless of course, someone knows a even better method... So please:
let me know!

Regards,
Christian
---
>From a0dc613140bab907a3d5787a7ae7b0638bf674d0 Mon Sep 17 00:00:00 2001
From: Christian Lamparter <chunk...@gmail.com>
Date: Thu, 23 Jun 2016 20:28:20 +0200
Subject: [PATCH] usb: xhci: add firmware loader quirk for  uPD720201 and
 uPD720202

This patch adds a firmware loader for the uPD720201K8-711-BAC-A
and uPD720202K8-711-BAA-A variant. Both of these chips are listed
in Renesas' R19UH0078EJ0500 Rev.5.00 "User's Manual: Hardware" as
devices which need a firmware in order to work as they do not have
support to load the firmware from an external ROM.

Currently, the xhci-pci driver is unable to initialize the hcd in
this case. Instead it will wait for 30 seconds and cause a timeout
in xhci_handshake() and fail.

[5.116990] xhci_hcd :45:00.0: new USB bus 

Re: [PATCH] usb: xhci: handle uPD720201 and uPD720202 w/o ROM

[Christian Lamparter]

On Wednesday, June 08, 2016 12:14:57 AM Christian Lamparter wrote:
> This patch adds a firmware check for the uPD720201K8-711-BAC-A
> and uPD720202K8-711-BAA-A variant. Both of these chips are listed
> in Renesas' R19UH0078EJ0500 Rev.5.00 "User's Manual: Hardware" as
> devices which need a firmware in order to work as they do not have
> support to load the firmware from an external ROM.
> 
> Currently, the xhci-pci driver is unable to initialize the hcd in
> this case. Instead it will wait for 30 seconds and cause a timeout
> in xhci_handshake() and fails.
> 
> [5.116990] xhci_hcd :45:00.0: new USB bus registered ...
> [   32.335215] xhci_hcd :45:00.0: can't setup: -110
> [   32.340179] xhci_hcd :45:00.0: USB bus 2 deregistered
> [   32.345587] xhci_hcd :45:00.0: init :45:00.0 fail, -110
> [   32.351496] xhci_hcd: probe of :45:00.0 failed with error -110
> 
> Cc: Yoshihiro Shimoda <yoshihiro.shimoda...@renesas.com>
> Signed-off-by: Christian Lamparter <chunk...@gmail.com>
Hello? 

Are there any news on this? Or is there anything else that I'm missing
which blocks this patch? If it's because the device won't be working
either with this patch, then please let me know. If Renesas is willing
to add the uPD720201/2 firmware (Like they did for their R-Car controllers)
to linux-firmware, I can write the  necessary firmware loader from
the PDF for this device and put it into xhci-quirks.

Regards,
Christian

> ---
>  drivers/usb/host/xhci-pci.c | 59 
> +
>  1 file changed, 59 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
> index 48672fa..3271a81 100644
> --- a/drivers/usb/host/xhci-pci.c
> +++ b/drivers/usb/host/xhci-pci.c
> @@ -207,6 +207,55 @@ static void xhci_pme_acpi_rtd3_enable(struct pci_dev 
> *dev)
>  static void xhci_pme_acpi_rtd3_enable(struct pci_dev *dev) { }
>  #endif /* CONFIG_ACPI */
>  
> +static int renesas_check_if_fw_dl_is_needed(struct pci_dev *pdev)
> +{
> + int err;
> + u8 fw_state;
> +
> + /*
> +  * Only the uPD720201K8-711-BAC-A or uPD720202K8-711-BAA-A
> +  * are listed in R19UH0078EJ0500 Rev.5.00 as devices which
> +  * need a firmware in order to work.
> +  *
> +  *  - uPD720202 ES 2.0 sample & CS sample & Mass product, ID is 2.
> +  *  - uPD720201 ES 2.0 sample whose revision ID is 2.
> +  *  - uPD720201 ES 2.1 sample & CS sample & Mass product, ID is 3.
> +  */
> + if (!((pdev->vendor == PCI_VENDOR_ID_RENESAS) &&
> + ((pdev->device == 0x0015 && pdev->revision == 0x02) ||
> +  (pdev->device == 0x0014 &&
> +   (pdev->revision == 0x02 || pdev->revision == 0x03)
> + return 0;
> +
> + /*
> +  * Test if the firmware was uploaded and is running.
> +  * As most BIOSes will initialize the device for us.
> +  */
> + err = pci_read_config_byte(pdev, 0xf4, _state);
> + if (err)
> + return pcibios_err_to_errno(err);
> +
> + /* Check the "Result Code" Bits (6:4) and act accordingly */
> + switch (fw_state & 0x70) {
> + case 0: /* No result yet */
> + dev_err(>dev, "FW is not ready/loaded yet.");
> + return -ENODEV;
> +
> + case BIT(4): /* Success, device should be working. */
> + dev_dbg(>dev, "FW is ready.");
> + return 0;
> +
> + case BIT(5): /* Error State */
> + dev_err(>dev, "HW is in an error state.");
> + return -ENODEV;
> +
> + default: /* All other states are marked as "Reserved states" */
> + dev_err(>dev, "HW is in an invalid state (%x).",
> + (fw_state & 0x70) >> 4);
> + return -EINVAL;
> + }
> +}
> +
>  /* called during probe() after chip reset completes */
>  static int xhci_pci_setup(struct usb_hcd *hcd)
>  {
> @@ -246,6 +295,11 @@ static int xhci_pci_probe(struct pci_dev *dev, const 
> struct pci_device_id *id)
>   struct hc_driver *driver;
>   struct usb_hcd *hcd;
>  
> + /* Check if this device is a RENESAS uPD720201/2 device. */
> + retval = renesas_check_if_fw_dl_is_needed(dev);
> + if (retval)
> + return retval;
> +
>   driver = (struct hc_driver *)id->driver_data;
>  
>   /* Prevent runtime suspending between USB-2 and USB-3 initialization */
> @@ -424,6 +478,11 @@ static int xhci_pci_resume(struct usb_hcd *hcd, bool 
> hibernated)
>   if (pdev->vendor == PCI_VENDOR_ID_INTEL)
>

[PATCH] usb: xhci: handle uPD720201 and uPD720202 w/o ROM

[Christian Lamparter]

This patch adds a firmware check for the uPD720201K8-711-BAC-A
and uPD720202K8-711-BAA-A variant. Both of these chips are listed
in Renesas' R19UH0078EJ0500 Rev.5.00 "User's Manual: Hardware" as
devices which need a firmware in order to work as they do not have
support to load the firmware from an external ROM.

Currently, the xhci-pci driver is unable to initialize the hcd in
this case. Instead it will wait for 30 seconds and cause a timeout
in xhci_handshake() and fails.

[5.116990] xhci_hcd :45:00.0: new USB bus registered ...
[   32.335215] xhci_hcd :45:00.0: can't setup: -110
[   32.340179] xhci_hcd :45:00.0: USB bus 2 deregistered
[   32.345587] xhci_hcd :45:00.0: init :45:00.0 fail, -110
[   32.351496] xhci_hcd: probe of :45:00.0 failed with error -110

Cc: Yoshihiro Shimoda <yoshihiro.shimoda...@renesas.com>
Signed-off-by: Christian Lamparter <chunk...@gmail.com>
---
 drivers/usb/host/xhci-pci.c | 59 +
 1 file changed, 59 insertions(+)

diff --git a/drivers/usb/host/xhci-pci.c b/drivers/usb/host/xhci-pci.c
index 48672fa..3271a81 100644
--- a/drivers/usb/host/xhci-pci.c
+++ b/drivers/usb/host/xhci-pci.c
@@ -207,6 +207,55 @@ static void xhci_pme_acpi_rtd3_enable(struct pci_dev *dev)
 static void xhci_pme_acpi_rtd3_enable(struct pci_dev *dev) { }
 #endif /* CONFIG_ACPI */
 
+static int renesas_check_if_fw_dl_is_needed(struct pci_dev *pdev)
+{
+   int err;
+   u8 fw_state;
+
+   /*
+* Only the uPD720201K8-711-BAC-A or uPD720202K8-711-BAA-A
+* are listed in R19UH0078EJ0500 Rev.5.00 as devices which
+* need a firmware in order to work.
+*
+*  - uPD720202 ES 2.0 sample & CS sample & Mass product, ID is 2.
+*  - uPD720201 ES 2.0 sample whose revision ID is 2.
+*  - uPD720201 ES 2.1 sample & CS sample & Mass product, ID is 3.
+*/
+   if (!((pdev->vendor == PCI_VENDOR_ID_RENESAS) &&
+   ((pdev->device == 0x0015 && pdev->revision == 0x02) ||
+(pdev->device == 0x0014 &&
+ (pdev->revision == 0x02 || pdev->revision == 0x03)
+   return 0;
+
+   /*
+* Test if the firmware was uploaded and is running.
+* As most BIOSes will initialize the device for us.
+*/
+   err = pci_read_config_byte(pdev, 0xf4, _state);
+   if (err)
+   return pcibios_err_to_errno(err);
+
+   /* Check the "Result Code" Bits (6:4) and act accordingly */
+   switch (fw_state & 0x70) {
+   case 0: /* No result yet */
+   dev_err(>dev, "FW is not ready/loaded yet.");
+   return -ENODEV;
+
+   case BIT(4): /* Success, device should be working. */
+   dev_dbg(>dev, "FW is ready.");
+   return 0;
+
+   case BIT(5): /* Error State */
+   dev_err(>dev, "HW is in an error state.");
+   return -ENODEV;
+
+   default: /* All other states are marked as "Reserved states" */
+   dev_err(>dev, "HW is in an invalid state (%x).",
+   (fw_state & 0x70) >> 4);
+   return -EINVAL;
+   }
+}
+
 /* called during probe() after chip reset completes */
 static int xhci_pci_setup(struct usb_hcd *hcd)
 {
@@ -246,6 +295,11 @@ static int xhci_pci_probe(struct pci_dev *dev, const 
struct pci_device_id *id)
struct hc_driver *driver;
struct usb_hcd *hcd;
 
+   /* Check if this device is a RENESAS uPD720201/2 device. */
+   retval = renesas_check_if_fw_dl_is_needed(dev);
+   if (retval)
+   return retval;
+
driver = (struct hc_driver *)id->driver_data;
 
/* Prevent runtime suspending between USB-2 and USB-3 initialization */
@@ -424,6 +478,11 @@ static int xhci_pci_resume(struct usb_hcd *hcd, bool 
hibernated)
if (pdev->vendor == PCI_VENDOR_ID_INTEL)
usb_enable_intel_xhci_ports(pdev);
 
+   /* Check if this device is a RENESAS uPD720201/2 device. */
+   retval = renesas_check_if_fw_dl_is_needed(pdev);
+   if (retval)
+   return retval;
+
if (xhci->quirks & XHCI_SSIC_PORT_UNUSED)
xhci_ssic_port_unused_quirk(hcd, false);
 
-- 
2.8.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: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4

[Christian Lamparter]

On Tuesday, May 17, 2016 04:50:48 PM John Youn wrote:
> On 5/14/2016 6:11 AM, Christian Lamparter wrote:
> > On Thursday, May 12, 2016 11:40:28 AM John Youn wrote:
> >> On 5/12/2016 6:30 AM, Christian Lamparter wrote:
> >>> On Thursday, May 12, 2016 01:55:44 PM Arnd Bergmann wrote:
> >>>> On Thursday 12 May 2016 11:58:18 Christian Lamparter wrote:
> >>>>>>>> Detecting the endianess of the
> >>>>>>>> device is probably the best future-proof solution, but it's also
> >>>>>>>> considerably more work to do in the driver, and comes with a
> >>>>>>>> tiny runtime overhead.
> >>>>>>>
> >>>>>>> The runtime overhead is probably non-measurable compared with the cost
> >>>>>>> of the actual MMIOs.
> >>>>>>
> >>>>>> Right. The code size increase is probably measurable (but still small),
> >>>>>> the runtime overhead is not.
> >>>>>
> >>>>> Ok, so no rebuts or complains have been posted.
> >>>>>
> >>>>> I've tested the patch you made in: https://lkml.org/lkml/2016/5/9/354
> >>>>> and it works: 
> >>>>>
> >>>>> Tested-by: Christian Lamparter <chunk...@googlemail.com>
> >>>>>
> >>>>> So, how do we go from here? There is are two small issues with the
> >>>>> original patch (#ifdef DWC2_LOG_WRITES got converted to lower case:
> >>>>> #ifdef dwc2_log_writes) and I guess a proper subject would be nice.  
> >>>>>
> >>>>> Arnd, can you please respin and post it (cc'd stable as well)?
> >>>>> So this is can be picked up? Or what's your plan?
> >>>>
> >>>> (I just realized my reply was stuck in my outbox, so the patch
> >>>> went out first)
> >>>>
> >>>> If I recall correctly, the rough consensus was to go with your longer
> >>>> patch in the future (fixed up for the comments that BenH and
> >>>> I sent), and I'd suggest basing it on top of a fixed version of
> >>>> my patch.
> >>> Well, but it comes with the "overhead"! So this was just as I said:
> >>> "Let's look at it and see if it's any good"... And I think it isn't
> >>> since the usb/host/ehci people also opted for #ifdef CONFIG_BIG_ENDIAN
> >>> archs etc...
> >>
> >> I slightly prefer the more general patch for future kernel versions.
> >> The overhead will probably be negligible, but we can perform some
> >> testing to make sure.
> >>
> >> Can you resubmit with all gathered feedback?
> > 
> > Yes, here are the changes.
> > 
> > I've tested it on my MyBook Live Duo. The usbotg comes right up:
> > [12610.540004] dwc2 4bff8.usbotg: USB bus 1 deregistered
> > [12612.513934] dwc2 4bff8.usbotg: Specified GNPTXFDEP=1024 > 256
> > [12612.518756] dwc2 4bff8.usbotg: EPs: 3, shared fifos, 2042 entries in 
> > SPRAM
> > [12612.530112] dwc2 4bff8.usbotg: DWC OTG Controller
> > [12612.533948] dwc2 4bff8.usbotg: new USB bus registered, assigned bus 
> > number 1
> > [12612.540083] dwc2 4bff8.usbotg: irq 33, io mem 0x
> > 
> > John: Can you run some perf test with it?
> > 
> > I've based this on:
> > 
> > commit 6ea2fffc9057a67df1994d85a7c085d899eaa25a
> > Author: Arnd Bergmann <a...@arndb.de>
> > Date:   Fri May 13 15:52:27 2016 +0200
> > 
> > usb: dwc2: fix regression on big-endian PowerPC/ARM systems
> > 
> > so naturally, it needs to be applied first.
> > Most of the conversion work was done by the attached
> > coccinelle semantic patches. 
> > 
> > I had to edit the __bic32 and __orr32 helpers by hand.
> > As well as some debugfs code and stuff in gadget.c.
> > 
> 
> Thanks Christian.
> 
> I'll keep this in our internal tree and send it to Felipe later. This
> causes a bunch of conflicts that I have to fix up and I should do a
> bit of testing as well.
> 
> And since there is a patch that fixes the regression this is can wait.
> 
> Regards,
> John
---
Hey, that's really nice of you to do that :-D. Please keep me in the
loop (Cc) for those then. 

Yes, this needs definitely testing on all the affected ARCHs. 
I've attached a diff to a updated version of the patch. It
drops the special MIPS case (as requested by Arnd).

BTW, I looked into the ioread32_

Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4

[Christian Lamparter]

On Thursday, May 12, 2016 11:40:28 AM John Youn wrote:
> On 5/12/2016 6:30 AM, Christian Lamparter wrote:
> > On Thursday, May 12, 2016 01:55:44 PM Arnd Bergmann wrote:
> >> On Thursday 12 May 2016 11:58:18 Christian Lamparter wrote:
> >>>>>> Detecting the endianess of the
> >>>>>> device is probably the best future-proof solution, but it's also
> >>>>>> considerably more work to do in the driver, and comes with a
> >>>>>> tiny runtime overhead.
> >>>>>
> >>>>> The runtime overhead is probably non-measurable compared with the cost
> >>>>> of the actual MMIOs.
> >>>>
> >>>> Right. The code size increase is probably measurable (but still small),
> >>>> the runtime overhead is not.
> >>>
> >>> Ok, so no rebuts or complains have been posted.
> >>>
> >>> I've tested the patch you made in: https://lkml.org/lkml/2016/5/9/354
> >>> and it works: 
> >>>
> >>> Tested-by: Christian Lamparter <chunk...@googlemail.com>
> >>>
> >>> So, how do we go from here? There is are two small issues with the
> >>> original patch (#ifdef DWC2_LOG_WRITES got converted to lower case:
> >>> #ifdef dwc2_log_writes) and I guess a proper subject would be nice.  
> >>>
> >>> Arnd, can you please respin and post it (cc'd stable as well)?
> >>> So this is can be picked up? Or what's your plan?
> >>
> >> (I just realized my reply was stuck in my outbox, so the patch
> >> went out first)
> >>
> >> If I recall correctly, the rough consensus was to go with your longer
> >> patch in the future (fixed up for the comments that BenH and
> >> I sent), and I'd suggest basing it on top of a fixed version of
> >> my patch.
> > Well, but it comes with the "overhead"! So this was just as I said:
> > "Let's look at it and see if it's any good"... And I think it isn't
> > since the usb/host/ehci people also opted for #ifdef CONFIG_BIG_ENDIAN
> > archs etc...
> 
> I slightly prefer the more general patch for future kernel versions.
> The overhead will probably be negligible, but we can perform some
> testing to make sure.
> 
> Can you resubmit with all gathered feedback?

Yes, here are the changes.

I've tested it on my MyBook Live Duo. The usbotg comes right up:
[12610.540004] dwc2 4bff8.usbotg: USB bus 1 deregistered
[12612.513934] dwc2 4bff8.usbotg: Specified GNPTXFDEP=1024 > 256
[12612.518756] dwc2 4bff8.usbotg: EPs: 3, shared fifos, 2042 entries in 
SPRAM
[12612.530112] dwc2 4bff8.usbotg: DWC OTG Controller
[12612.533948] dwc2 4bff8.usbotg: new USB bus registered, assigned bus 
number 1
[12612.540083] dwc2 4bff8.usbotg: irq 33, io mem 0x

John: Can you run some perf test with it?

I've based this on:

commit 6ea2fffc9057a67df1994d85a7c085d899eaa25a
Author: Arnd Bergmann <a...@arndb.de>
Date:   Fri May 13 15:52:27 2016 +0200

usb: dwc2: fix regression on big-endian PowerPC/ARM systems

so naturally, it needs to be applied first.
Most of the conversion work was done by the attached
coccinelle semantic patches. 

I had to edit the __bic32 and __orr32 helpers by hand.
As well as some debugfs code and stuff in gadget.c.

here's the diffstat for everything. It's quite a lot:
 core.c  |  221 ++---
 core.h  |  134 +++-
 core_intr.c |   70 +-
 debugfs.c   |   55 +++-
 gadget.c|  396 +---
 hcd.c   |  382 -
 hcd.h   |   10 -
 hcd_ddma.c  |   10 -
 hcd_intr.c  |   93 ++
 hcd_queue.c |   10 -
 platform.c  |4 
 11 files changed, 709 insertions(+), 676 deletions(-)

So, Felipe: what's your plan?

---
@@ 
struct dwc2_hsotg *hsotg;
expression addr; 
@@

-dwc2_readl(hsotg->regs + addr)
+dwc2_readl(hsotg, addr)

@@
struct dwc2_hsotg *hsotg;
expression addr, value; 
@@

-dwc2_writel(value, hsotg->regs + addr)
+dwc2_writel(hsotg, value, addr)

@@
struct dwc2_hsotg *hsotg;
expression addr, port;
@@
addr = hsotg->regs + port;
...
-dwc2_readl(addr)
+dwc2_readl(hsotg, port)

@@
struct dwc2_hsotg *hsotg;
expression addr, val; 
@@

-__orr32(hsotg->regs + addr, val)
+__orr32(hsotg, addr, val)

@@
struct dwc2_hsotg *hsotg;
expression addr, val; 
@@

-__bic32(hsotg->regs + addr, val)
+__bic32(hsotg, addr, val)

@@
expression port;
identifier regs;
@@
-dwc2_readl(regs + port)
+dwc2_readl(hsotg, port)
---
diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
index 4135a5f..f773146 100644
--- a/drivers/usb

Re: [PATCH v4] usb: dwc2: fix regression on big-endian PowerPC/ARM systems

[Christian Lamparter]

On Friday, May 13, 2016 03:52:27 PM Arnd Bergmann wrote:
> A patch that went into Linux-4.4 to fix big-endian mode on a Lantiq
> MIPS system unfortunately broke big-endian operation on PowerPC
> APM82181 as reported by Christian Lamparter, and likely other
> systems.
> 
> It actually introduced multiple issues:
> 
> - it broke big-endian ARM kernels: any machine that was working
>   correctly with a little-endian kernel is no longer using byteswaps
>   on big-endian kernels, which clearly breaks them.
> - On PowerPC the same thing must be true: if it was working before,
>   using big-endian kernels is now broken. Unlike ARM, 32-bit PowerPC
>   usually uses big-endian kernels, so they are likely all broken.
> - The barrier for dwc2_writel is on the wrong side of the __raw_writel(),
>   so the MMIO no longer synchronizes with DMA operations.
> - On architectures that require specific CPU instructions for MMIO
>   access, using the __raw_ variant may turn this into a pointer
>   dereference that does not have the same effect as the readl/writel.
> 
> This patch is a simple revert for all architectures other than MIPS,
> in the hope that we can more easily backport it to fix the regression
> on PowerPC and ARM systems without breaking the Lantiq system again.
> 
> We should follow this up with a more elaborate change to add runtime
> detection of endianness, to make sure it also works on all other
> combinations of architectures and implementations of the usb-dwc2
> device. That patch however will be fairly large and not appropriate
> for backports to stable kernels.
> 
> Felipe suggested a different approach, using an endianness switching
> register to always put the device into LE mode, but unfortunately
> the dwc2 hardware does not provide a generic way to do that. Also,
> I see no practical way of addressing the problem more generally by
> patching architecture specific code on MIPS.
> 
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> Fixes: 95c8bc360944 ("usb: dwc2: Use platform endianness when accessing 
> registers")
Thanks Arnd,

Tested-by: Christian Lamparter <chunk...@googlemail.com>

dwc2 4bff8.usbotg: Specified GNPTXFDEP=1024 > 256
dwc2 4bff8.usbotg: EPs: 3, shared fifos, 2042 entries in SPRAM
dwc2 4bff8.usbotg: DWC OTG Controller
dwc2 4bff8.usbotg: new USB bus registered, assigned bus number 1
dwc2 4bff8.usbotg: irq 33, io mem 0x
hub 1-0:1.0: USB hub found
hub 1-0:1.0: 1 port detected

Regards,
Christian
--
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: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4

[Christian Lamparter]

On Thursday, May 12, 2016 11:40:28 AM John Youn wrote:
> On 5/12/2016 6:30 AM, Christian Lamparter wrote:
> > On Thursday, May 12, 2016 01:55:44 PM Arnd Bergmann wrote:
> >> On Thursday 12 May 2016 11:58:18 Christian Lamparter wrote:
> >>>>>> Detecting the endianess of the
> >>>>>> device is probably the best future-proof solution, but it's also
> >>>>>> considerably more work to do in the driver, and comes with a
> >>>>>> tiny runtime overhead.
> >>>>>
> >>>>> The runtime overhead is probably non-measurable compared with the cost
> >>>>> of the actual MMIOs.
> >>>>
> >>>> Right. The code size increase is probably measurable (but still small),
> >>>> the runtime overhead is not.
> >>>
> >>> Ok, so no rebuts or complains have been posted.
> >>>
> >>> I've tested the patch you made in: https://lkml.org/lkml/2016/5/9/354
> >>> and it works: 
> >>>
> >>> Tested-by: Christian Lamparter <chunk...@googlemail.com>
> >>>
> >>> So, how do we go from here? There is are two small issues with the
> >>> original patch (#ifdef DWC2_LOG_WRITES got converted to lower case:
> >>> #ifdef dwc2_log_writes) and I guess a proper subject would be nice.  
> >>>
> >>> Arnd, can you please respin and post it (cc'd stable as well)?
> >>> So this is can be picked up? Or what's your plan?
> >>
> >> (I just realized my reply was stuck in my outbox, so the patch
> >> went out first)
> >>
> >> If I recall correctly, the rough consensus was to go with your longer
> >> patch in the future (fixed up for the comments that BenH and
> >> I sent), and I'd suggest basing it on top of a fixed version of
> >> my patch.
> > Well, but it comes with the "overhead"! So this was just as I said:
> > "Let's look at it and see if it's any good"... And I think it isn't
> > since the usb/host/ehci people also opted for #ifdef CONFIG_BIG_ENDIAN
> > archs etc...
> 
> I slightly prefer the more general patch for future kernel versions.
> The overhead will probably be negligible, but we can perform some
> testing to make sure.
> 
> Can you resubmit with all gathered feedback?
Yes I think I can do that. But I would really like to get the
regression out of the way. So for that: I back Arnd's patch.
It explains the problem much better and doesn't kill MIPS 
like the revert I was doing in my initial post to the MLs.
Also, another bonus: his patch is suited to port to stable.

The auto-detection approach is not that easy to get right,
given all the stuff that's going on with BE8, LE4, ... So
can we have your "blessing" for Arnd's patch for now? since
that way, I can base my patch on top of his work about the
issues of endiannes? (Just say: ACK :) )

Arnd: do you have a version with the #ifdef lower/uppercase
fix? Or should I give it a try (and fail in a different way ;) )
 
> >> Felipe just had another idea, to change the endianess of the dwc2
> >> block by setting a registers (if that exists). That would indeed
> >> be preferable, then we can just revert the broken change that
> >> went into 4.4 and backport that fix instead.
> > Just a quick reply. I have the docs for the thing. There's something
> > like that in GAHBCFG at Bit 24... BUT it only switches the endiannes
> > for the DMA descriptors (which is not always used, there are devices
> > with PIO only)! It doesn't deal with the MMIO access at all. 
> 
> That's correct. It only affects descriptor endianness for DMA
> descriptor mode of operation.

Ok. The funny thing is that for the MyBook Live Duo this setting might
be important since the PLB_DMA engine is not part of the DWC library...
Instead it's from IBM and operates in: Big Endian :-D.

Regards,
Christian
--
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: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4

[Christian Lamparter]

On Thursday, May 12, 2016 01:55:44 PM Arnd Bergmann wrote:
> On Thursday 12 May 2016 11:58:18 Christian Lamparter wrote:
> > > > > Detecting the endianess of the
> > > > > device is probably the best future-proof solution, but it's also
> > > > > considerably more work to do in the driver, and comes with a
> > > > > tiny runtime overhead.
> > > > 
> > > > The runtime overhead is probably non-measurable compared with the cost
> > > > of the actual MMIOs.
> > > 
> > > Right. The code size increase is probably measurable (but still small),
> > > the runtime overhead is not.
> > 
> > Ok, so no rebuts or complains have been posted.
> > 
> > I've tested the patch you made in: https://lkml.org/lkml/2016/5/9/354
> > and it works: 
> > 
> > Tested-by: Christian Lamparter <chunk...@googlemail.com>
> > 
> > So, how do we go from here? There is are two small issues with the
> > original patch (#ifdef DWC2_LOG_WRITES got converted to lower case:
> > #ifdef dwc2_log_writes) and I guess a proper subject would be nice.  
> > 
> > Arnd, can you please respin and post it (cc'd stable as well)?
> > So this is can be picked up? Or what's your plan?
> 
> (I just realized my reply was stuck in my outbox, so the patch
> went out first)
> 
> If I recall correctly, the rough consensus was to go with your longer
> patch in the future (fixed up for the comments that BenH and
> I sent), and I'd suggest basing it on top of a fixed version of
> my patch.
Well, but it comes with the "overhead"! So this was just as I said:
"Let's look at it and see if it's any good"... And I think it isn't
since the usb/host/ehci people also opted for #ifdef CONFIG_BIG_ENDIAN
archs etc...
 
> Felipe just had another idea, to change the endianess of the dwc2
> block by setting a registers (if that exists). That would indeed
> be preferable, then we can just revert the broken change that
> went into 4.4 and backport that fix instead.
Just a quick reply. I have the docs for the thing. There's something
like that in GAHBCFG at Bit 24... BUT it only switches the endiannes
for the DMA descriptors (which is not always used, there are devices
with PIO only)! It doesn't deal with the MMIO access at all. 

The pin that would select which endian the device uses is probably
connected to a DCR or GPIO but I don't know which or where so this
is more or less useless. (Or the selectable endianness was dropped
during synth).

Regards,
Christian
--
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: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4

[Christian Lamparter]

 > > considerably more work to do in the driver, and comes with a
> > > tiny runtime overhead.
> > 
> > The runtime overhead is probably non-measurable compared with the cost
> > of the actual MMIOs.
> 
> Right. The code size increase is probably measurable (but still small),
> the runtime overhead is not.

Ok, so no rebuts or complains have been posted.

I've tested the patch you made in: https://lkml.org/lkml/2016/5/9/354
and it works: 

Tested-by: Christian Lamparter <chunk...@googlemail.com>

So, how do we go from here? There is are two small issues with the
original patch (#ifdef DWC2_LOG_WRITES got converted to lower case:
#ifdef dwc2_log_writes) and I guess a proper subject would be nice.  

Arnd, can you please respin and post it (cc'd stable as well)?
So this is can be picked up? Or what's your plan?

Regards,
Christian
--
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: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4

[Christian Lamparter]

Uh, Thanks for the participation!

On Monday, May 09, 2016 05:08:48 PM Arnd Bergmann wrote:
> On Monday 09 May 2016 13:39:50 Felipe Balbi wrote:
> > Arnd Bergmann <a...@arndb.de> writes:
> > > On Monday 09 May 2016 10:23:22 Benjamin Herrenschmidt wrote:
> > >> On Sun, 2016-05-08 at 13:44 +0200, Christian Lamparter wrote:
> > >
> > > The patch that caused the problem had multiple issues:
> > >
> > > - it broke big-endian ARM kernels: any machine that was working
> > >   correctly with a little-endian kernel is no longer using byteswaps
> > >   on big-endian kernels, which clearly breaks them.
> > > - On PowerPC the same thing must be true: if it was working before,
> > >   using big-endian kernels is now broken. Unlike ARM, 32-bit PowerPC
> > >   usually uses big-endian kernels, so they are likely all broken.
> > > - The barrier for dwc2_writel is on the wrong side of the __raw_writel(),
> > >   so the MMIO no longer synchronizes with DMA operations.
> > > - On architectures that require specific CPU instructions for MMIO
> > >   access, using the __raw_ variant may turn this into a pointer
> > >   dereference that does not have the same effect as the readl/writel.
> > >
> > > I think we can simply make this set of accessors architecture-dependent
> > > (MIPS vs. the rest of the world) to revert ARM and PowerPC back to
> > > the working version.
> > 
> > and patch all drivers similarly? Shouldn't arch/mips itself deal with it
> > and hide it from drivers ?
> > 
> 
> Unfortunately, I don't see any way this could be done in MIPS specific
> code: There is typically a byteswap between the internal bus and the PCI
> bus on big-endian MIPS systems, so the PCI MMIO ends up being little-endian,
> which matches the expected behavior of readl/writel. However, drivers
> for non-PCI devices often use the same readl/writel accessors because
> that is how it's done on ARMv6/ARMv7.
> 
> Doing it hardcoded by architecture is just the simplest way to deal
> with it, working on the assumption that nothing actually needs the
> runtime detection that Ben suggested. Detecting the endianess of the
> device is probably the best future-proof solution, but it's also
> considerably more work to do in the driver, and comes with a
> tiny runtime overhead.
> 
Ok, just to have it on the table. I went ahead and implemented the
"Detect Endian". 

I looked in the DWC USB OTG's Databook documents v3.30a (If someone wants
them too, PM me). If I read the Application Interface Feature list on
page 30 correctly. The endianess is selectable by a "pin" That said
I don't know which one is it in the APM82181 or any other arch. I looked
around for configuration registers and stuff but unlike DesignWare's AHB
DMA  Controller, there's no Bit in the "User HW Config Registers" that
would tell us if it was configured as big-endian or little-endian at
the moment.

One way out would be to detect the endianess automatically by looking at
the values in the GSNPSID register. This is a read-only register containing
the release number of the core being used. The "upper" 16-bits of it are
hardcoded to 0x4f45 (The comment in dwc2_get_hwparams [1] has it backwards
but not the code below).

I ran into the following issues:
- gadget.c uses ioread32_rep [0] & iowrite32_rep [1].
  This is interesting because both of these functions actually use
  the __raw_io* on powerpc. This is because powerpc uses the default
  defines of include/asm-generic/io.h [2].

  Ideally, this should be done by sth like a writesl_be or writesl(e)
  function. But I found none so for now: Let's make a ugly hack:
  to_correct_endian that will work for testing, but will be replaced.

   - dwc2_readl, dwc2_writel. I have no insight in the craziness that's
  going on with MIPS and the memory barrier. But it got me thinking
  rather than CONFIG_MIPS, isn't there a umbrella
  "ARCH_HAS_NEED_MEMORY_BARRIER_FOR_MMIO" config symbol?

- is_little_endian (do we want a separate is_big_endian?)
  Also, do we want to be able to overwrite the detection code
  if the endian setting was set in the device tree?. For now
  it always does auto detection (see dwc2_detect_endiannes() ).

( - 80 character per line issues, is it possible to drop the
 hsotg->reg + REGISTER from the dwc2_readl/writel since we
 pass the hsotg now anyway and do the reg + REGISTER
 calculation in the accessor? I played around with macros
 as most functions calling the accessors have the hsotg
 variable anyway )
 

Re: usb: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4

[Christian Lamparter]

On Sunday, May 08, 2016 08:40:55 PM Benjamin Herrenschmidt wrote:
> On Sun, 2016-05-08 at 00:54 +0200, Christian Lamparter via Linuxppc-dev 
> wrote:
> > I've been looking in getting the MyBook Live Duo's USB OTG port
> > to function. The SoC is a APM82181. Which has a PowerPC 464 core
> > and related to the supported canyonlands architecture in
> > arch/powerpc/.
> > 
> > Currently in -next the dwc2 module doesn't load: 
> 
> Smells like the APM implementation is little endian. You might need to
> use a flag to indicate what endian to use instead and set it
> appropriately based on some DT properties.
I tried. As per common-properties[0], I added little-endian; but it has no
effect. I looked in dwc2_driver_probe and found no way of specifying the
endian of the device. It all comes down to the dwc2_readl & dwc2_writel
accessors. These - sadly - have been hardwired to use __raw_readl and
__raw_writel. So, it's always "native-endian". While common-properties
says little-endian should be preferred.

> > dwc2 4bff8.usbotg: dwc2_core_reset() HANG! AHB Idle GRSTCTL=80
> > dwc2 4bff8.usbotg: Bad value for GSNPSID: 0x0a29544f
> > 
> > Looking at the Bad GSNPSID value: 0x0a29544f. It is obvious that
> > this is an endian problem. git finds this patch:
> > 
> > commit 95c8bc3609440af5e4a4f760b8680caea7424396
> > Author: Antti Seppälä <a.sepp...@gmail.com>
> > Date:   Thu Aug 20 21:41:07 2015 +0300
> > 
> > usb: dwc2: Use platform endianness when accessing registers
> > 
> > This patch is necessary to access dwc2 registers correctly on
> > big-endian
> > systems such as the mips based SoCs made by Lantiq. Then dwc2 can
> > be
> > used to replace ifx-hcd driver for Lantiq platforms found e.g. in
> > OpenWrt.
> > 
> > The patch was autogenerated with the following commands:
> > $EDITOR core.h
> > sed -i "s/\<readl\>/dwc2_readl/g" *.c hcd.h hw.h
> > sed -i "s/\<writel\>/dwc2_writel/g" *.c hcd.h hw.h
> > 
> > Some files were then hand-edited to fix checkpatch.pl warnings
> > about
> > too long lines.
> > 
> > which unfortunately, broke the USB-OTG port on the MyBook Live Duo.
> > Reverting to the readl / writel:
> > 
> > --- 
> > diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> > index 3c58d63..c021c1f 100644
> > --- a/drivers/usb/dwc2/core.h
> > +++ b/drivers/usb/dwc2/core.h
> > @@ -66,7 +66,7 @@
> >  
> >  static inline u32 dwc2_readl(const void __iomem *addr)
> >  {
> > -   u32 value = __raw_readl(addr);
> > +   u32 value = readl(addr);
> >  
> > /* In order to preserve endianness __raw_* operation is
> > used. Therefore
> >  * a barrier is needed to ensure IO access is not re-ordered 
> > across
> > @@ -78,7 +78,7 @@ static inline u32 dwc2_readl(const void __iomem
> > *addr)
> >  
> >  static inline void dwc2_writel(u32 value, void __iomem *addr)
> >  {
> > -   __raw_writel(value, addr);
> > +   writel(value, addr);
> >  
> > /*
> >  * In order to preserve endianness __raw_* operation is
> > used. Therefore
> > 
> > ---
> > 
> > restores the dwc-otg port to full working order:
> > dwc2 4bff8.usbotg: Specified GNPTXFDEP=1024 > 256
> > dwc2 4bff8.usbotg: EPs: 3, shared fifos, 2042 entries in SPRAM
> > dwc2 4bff8.usbotg: DWC OTG Controller
> > dwc2 4bff8.usbotg: new USB bus registered, assigned bus number 1
> > dwc2 4bff8.usbotg: irq 33, io mem 0x
> > hub 1-0:1.0: USB hub found
> > hub 1-0:1.0: 1 port detected
> > root@mbl:~# usb 1-1: new high-speed USB device number 2 using dwc2
> > 
> > So, what to do?
 ^^^

Regards,
Christian

[0] 
<http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/common-properties.txt>

--
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: dwc2: regression on MyBook Live Duo / Canyonlands since 4.3.0-rc4

[Christian Lamparter]

Hello,

I've been looking in getting the MyBook Live Duo's USB OTG port
to function. The SoC is a APM82181. Which has a PowerPC 464 core
and related to the supported canyonlands architecture in arch/powerpc/.

Currently in -next the dwc2 module doesn't load: 

dwc2 4bff8.usbotg: dwc2_core_reset() HANG! AHB Idle GRSTCTL=80
dwc2 4bff8.usbotg: Bad value for GSNPSID: 0x0a29544f

Looking at the Bad GSNPSID value: 0x0a29544f. It is obvious that
this is an endian problem. git finds this patch:

commit 95c8bc3609440af5e4a4f760b8680caea7424396
Author: Antti Seppälä 
Date:   Thu Aug 20 21:41:07 2015 +0300

usb: dwc2: Use platform endianness when accessing registers

This patch is necessary to access dwc2 registers correctly on big-endian
systems such as the mips based SoCs made by Lantiq. Then dwc2 can be
used to replace ifx-hcd driver for Lantiq platforms found e.g. in
OpenWrt.

The patch was autogenerated with the following commands:
$EDITOR core.h
sed -i "s/\/dwc2_readl/g" *.c hcd.h hw.h
sed -i "s/\/dwc2_writel/g" *.c hcd.h hw.h

Some files were then hand-edited to fix checkpatch.pl warnings about
too long lines.

which unfortunately, broke the USB-OTG port on the MyBook Live Duo.
Reverting to the readl / writel:

--- 
diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 3c58d63..c021c1f 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -66,7 +66,7 @@
 
 static inline u32 dwc2_readl(const void __iomem *addr)
 {
-   u32 value = __raw_readl(addr);
+   u32 value = readl(addr);
 
/* In order to preserve endianness __raw_* operation is used. Therefore
 * a barrier is needed to ensure IO access is not re-ordered across
@@ -78,7 +78,7 @@ static inline u32 dwc2_readl(const void __iomem *addr)
 
 static inline void dwc2_writel(u32 value, void __iomem *addr)
 {
-   __raw_writel(value, addr);
+   writel(value, addr);
 
/*
 * In order to preserve endianness __raw_* operation is used. Therefore

---

restores the dwc-otg port to full working order:
dwc2 4bff8.usbotg: Specified GNPTXFDEP=1024 > 256
dwc2 4bff8.usbotg: EPs: 3, shared fifos, 2042 entries in SPRAM
dwc2 4bff8.usbotg: DWC OTG Controller
dwc2 4bff8.usbotg: new USB bus registered, assigned bus number 1
dwc2 4bff8.usbotg: irq 33, io mem 0x
hub 1-0:1.0: USB hub found
hub 1-0:1.0: 1 port detected
root@mbl:~# usb 1-1: new high-speed USB device number 2 using dwc2

So, what to do?

Regards,
Christian
--
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 v1.1] usbip: move usbip_protocol.txt to Documentation

[Christian Lamparter]

The usbip_protocol.txt, a document which describes usbip's
inner workings is currently located in the projects source
directory (drivers/usb/usbip/...). This patch moves it to
Documentation/usb.

This discussion was brought up by Guy Harris [0] during the
review of the USBIP dissector I wrote. For anyone interested:
support is available with the latest wireshark master/dev tree.
Simply select a packet from the usbip's tcp-stream you are
intrested on and select the USBIP as the protocol in the
"Decode As" dialog box [1].

Signed-off-by: Christian Lamparter <chunk...@googlemail.com>

[0] <https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12127#c2>
[1] 
<https://www.wireshark.org/docs/wsug_html_chunked/ChCustProtocolDissectionSection.html#ChAdvDecodeAs>
---
 {drivers/usb/usbip => Documentation/usb}/usbip_protocol.txt | 0
 MAINTAINERS | 1 +
 2 files changed, 1 insertion(+)
 rename {drivers/usb/usbip => Documentation/usb}/usbip_protocol.txt (100%)

diff --git a/drivers/usb/usbip/usbip_protocol.txt 
b/Documentation/usb/usbip_protocol.txt
similarity index 100%
rename from drivers/usb/usbip/usbip_protocol.txt
rename to Documentation/usb/usbip_protocol.txt
diff --git a/MAINTAINERS b/MAINTAINERS
index c592d54..327d57b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11371,6 +11371,7 @@ M:  Valentina Manea <valentina.mane...@gmail.com>
 M: Shuah Khan <shuah...@samsung.com>
 L: linux-usb@vger.kernel.org
 S: Maintained
+F: Documentation/usb/usbip_protocol.txt
 F: drivers/usb/usbip/
 F: tools/usb/usbip/
 
-- 
2.7.0

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

[PATCH] usbip: move usbip_protocol.txt to Documentation

[Christian Lamparter]

The usbip_protocol.txt, a document which describes usbip's
inner workings is currently located in the projects source
directory (drivers/usb/usbip/...). This patch moves it to
Documentation/usb.

This discussion was brought up by Guy Harris [0] during the
review of the USBIP dissector I wrote. For anyone interested:
support is available with the latest wireshark master/dev tree.
Simply select a packet from the usbip's tcp-stream you are
intrested on and select the USBIP as the protocol in the
"Decode As" dialog box [1].

Signed-off-by: Christian Lamparter <chunk...@googlemail.com>

[0] <https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=12127#c2>
[1] 
<https://www.wireshark.org/docs/wsug_html_chunked/ChCustProtocolDissectionSection.html#ChAdvDecodeAs>
---
 Documentation/usb/usbip_protocol.txt | 358 +++
 MAINTAINERS  |   1 +
 drivers/usb/usbip/usbip_protocol.txt | 358 ---
 3 files changed, 359 insertions(+), 358 deletions(-)
 create mode 100644 Documentation/usb/usbip_protocol.txt
 delete mode 100644 drivers/usb/usbip/usbip_protocol.txt

diff --git a/Documentation/usb/usbip_protocol.txt 
b/Documentation/usb/usbip_protocol.txt
new file mode 100644
index 000..16b6fe2
--- /dev/null
+++ b/Documentation/usb/usbip_protocol.txt
@@ -0,0 +1,358 @@
+PRELIMINARY DRAFT, MAY CONTAIN MISTAKES!
+28 Jun 2011
+
+The USB/IP protocol follows a server/client architecture. The server exports 
the
+USB devices and the clients imports them. The device driver for the exported
+USB device runs on the client machine.
+
+The client may ask for the list of the exported USB devices. To get the list 
the
+client opens a TCP/IP connection towards the server, and sends an 
OP_REQ_DEVLIST
+packet on top of the TCP/IP connection (so the actual OP_REQ_DEVLIST may be 
sent
+in one or more pieces at the low level transport layer). The server sends back
+the OP_REP_DEVLIST packet which lists the exported USB devices. Finally the
+TCP/IP connection is closed.
+
+ virtual host controller usb host
+  "client"   "server"
+  (imports USB devices) (exports USB devices)
+  | |
+  |  OP_REQ_DEVLIST |
+  | --> |
+  | |
+  |  OP_REP_DEVLIST |
+  | <-- |
+  | |
+
+Once the client knows the list of exported USB devices it may decide to use one
+of them. First the client opens a TCP/IP connection towards the server and
+sends an OP_REQ_IMPORT packet. The server replies with OP_REP_IMPORT. If the
+import was successful the TCP/IP connection remains open and will be used
+to transfer the URB traffic between the client and the server. The client may
+send two types of packets: the USBIP_CMD_SUBMIT to submit an URB, and
+USBIP_CMD_UNLINK to unlink a previously submitted URB. The answers of the
+server may be USBIP_RET_SUBMIT and USBIP_RET_UNLINK respectively.
+
+ virtual host controller usb host
+  "client"   "server"
+  (imports USB devices) (exports USB devices)
+  | |
+  |  OP_REQ_IMPORT  |
+  | --> |
+  | |
+  |  OP_REP_IMPORT  |
+  | <-- |
+  | |
+  | |
+  |USBIP_CMD_SUBMIT(seqnum = n) |
+  | --> |
+  | |
+  |USBIP_RET_SUBMIT(seqnum = n) |
+  | <-- |
+  |.|
+  |:|
+  | |
+  |USBIP_CMD_SUBMIT(seqnum = m) |
+  | --> |
+  | |
+  |USBIP_CMD_SUBMIT(seqnum = m+1)   |
+  | --> |
+  |

Re: FUSB200 xhci issue

[Christian Lamparter]

On Thursday 08 August 2013 17:35:34 Oleksij Rempel wrote:
 Am 31.07.2013 08:52, schrieb Oleksij Rempel:
  Am 28.07.2013 22:41, schrieb Christian Lamparter:
  On Sunday, July 28, 2013 04:28:25 PM Oleksij Rempel wrote:
  Am 28.07.2013 14:12, schrieb Oleksij Rempel:
  Am 28.07.2013 13:38, schrieb Christian Lamparter:
 
  before  rmmod.
  Oh, I it was on the latest wireless-testing. (And the ath9k_htc
  module
  had the patch ath9k_htc: reboot firmwware if it was loaded).
 
  Furthermore, I did the same test with one of the ehci-only ports
  and it worked. Both, devices (one had a AR7015, the other a AR9271)
  came back after autosuspend there.
 
  Grrr... so it brings us back to xhci issue. Even EP4 workaround wont
  work here :( Suddenly i have no more ideas.
 
  Sarah, it's your turn now.
 
  Christian,
  can you please provide some more info about your xhci controller. I'll
  try to get me same.
 
  Well, it's a laptop (HP DV6-6003EG). I recon that getting 100% the
  same setup will be difficult. However, since the uPD720200 was/is
  very popular, it should be very easy to find one. [It's probably
  on all of these 10 euro usb-3.0 pcie-adapters. So as long as you
  got a free 1x-pcie port you should be good.]
 
  Here's the lspci summary:
 
  19:00.0 USB controller [0c03]: NEC Corporation uPD720200 USB 3.0 Host
  Controller [1033:0194] (rev 04) (prog-if 30 [XHCI])
   Subsystem: Hewlett-Packard Company Device [103c:1657]
   Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop-
  ParErr- Stepping- SERR- FastB2B- DisINTx+
   Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast
  TAbort- TAbort- MAbort- SERR- PERR- INTx-
   Latency: 0, Cache Line Size: 64 bytes
   Interrupt: pin A routed to IRQ 19
   Region 0: Memory at d340 (64-bit, non-prefetchable)
  [size=8K]
   Capabilities: access denied
   Kernel driver in use: xhci_hcd
 
 
  Thx... i purchased on random on ebay, will see what i get.
 
  I know now why carl9170 don't triggering this bug. Carl uses EP4 as
  Interrupt with packet size 64. ath9k-htc initially have EP4=Intr,
  Interval=1, but will reconfigure it to Bulk, Interval=0.
  It mean, before usb suspend EP4=Bulk, Interval=0 and after resume
  EP4=Intr, Inter=?. May be xhci can't handle some thing like this? Or may
  be interval stay 0, and xhci will overfill usb buffer on adapter - at
  least it looks so.
 
 Christian,
 can you please test one more patch. It is working for me, but who knows. 
 More testing is never bad idea ;)

It sort of works, but not without a hiccup: 

I get the following messages when I try to load the driver
again after an autosuspend cycle (ar9271, NEC xhci):

ath: phy6: Reading Magic # failed
ath9k_htc: Failed to Initialize the device
usb 2-1: ath9k_htc: USB layer deinitialized


However, the device is resetted automatically and it
comes back on the second probe attempt.



Anyway, I do have a question about something else too.

in ath9k_htc's hif_usb:

 struct usb_host_interface *alt = hif_dev-interface-altsetting[0];
 struct usb_endpoint_descriptor *endp;
 ...
 /* On downloading the firmware to the target, the USB descriptor of EP4
  * is 'patched' to change the type of the endpoint to Bulk. This will
  * bring down CPU usage during the scan period. */

 for (idx = 0; idx  alt-desc.bNumEndpoints; idx++) {
   endp = alt-endpoint[idx].desc;
   if ((endp-bmAttributes  USB_ENDPOINT_XFERTYPE_MASK) == 
 USB_ENDPOINT_XFER_INT) {
 endp-bmAttributes = ~USB_ENDPOINT_XFERTYPE_MASK;
 endp-bmAttributes |= USB_ENDPOINT_XFER_BULK;
// endp-bInterval = 0;
   }
 }

Alan, can you please tell us, if it is really safe to
override the bmAttributes this way? After all (according to
the comment) the device has morphed (EP4 has changed).

Or, is it necessary for the driver call usb_reset_device
or (usb_reset_configuration) in this case? 

Regards,
   Chr
--
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: FUSB200 xhci issue

[Christian Lamparter]

On Thursday 08 August 2013 22:19:32 Alan Stern wrote:
 On Thu, 8 Aug 2013, Christian Lamparter wrote:
 
  Anyway, I do have a question about something else too.
  
  in ath9k_htc's hif_usb:
  
   struct usb_host_interface *alt = hif_dev-interface-altsetting[0];
   struct usb_endpoint_descriptor *endp;
   ...
   /* On downloading the firmware to the target, the USB descriptor of EP4
* is 'patched' to change the type of the endpoint to Bulk. This will
* bring down CPU usage during the scan period. */
  
   for (idx = 0; idx  alt-desc.bNumEndpoints; idx++) {
 endp = alt-endpoint[idx].desc;
 if ((endp-bmAttributes  USB_ENDPOINT_XFERTYPE_MASK) == 
   USB_ENDPOINT_XFER_INT) {
   endp-bmAttributes = ~USB_ENDPOINT_XFERTYPE_MASK;
   endp-bmAttributes |= USB_ENDPOINT_XFER_BULK;
  // endp-bInterval = 0;
 }
   }
  
  Alan, can you please tell us, if it is really safe to
  override the bmAttributes this way? After all (according to
  the comment) the device has morphed (EP4 has changed).
 
 This does not look like a good idea.  Why does the driver do it?

Probably because people use ath9k_htc devices with everything that
has some sort of usb port (devkits and embedded systems: Dockstar,
Rasberry Pi, ...) to get wifi connectivity. ...


Yeah. I think we better ask Rajkumar Manoharan 
(before I write more text out of thin air :-D )

commit 4a0e8ecca4eeed38d4b3b7a317a3aaab4dd3cacd
Author: Rajkumar Manoharan rmanoha...@atheros.com
Date:   Tue Sep 14 14:35:55 2010 +0530

ath9k_htc: Fix CPU usage issue during scan period

Does anyone know his Qualcomm Atheros address?
 
  Or, is it necessary for the driver call usb_reset_device
  or (usb_reset_configuration) in this case? 
 
 After loading firmware, a reset generally is necessary.  Some devices 
 will do it themselves; others require you to call usb_reset_device().

This makes things complicated. Because, as far as I remember,
usb_reset_device() will cause the current driver to be unbound
unless its called during .probe, right?

You see, ath9k_htc loads its firmware asynchronously in .probe
(ath9k_htc's .probe routine finishes before the firmware is
retrieved via the firmware loader helper... so part of the
firmware download is done in a firmware_complete callback
on a workqueue). 

So, if we call usb_reset_device there and the driver is unbound
and later rebound. the next ath9k_htc .probe will start again and
again and again not knowing that it is already initialized 
(and we have a loop).


This could be solved, if the devices changes the usb-id again 
when a proper wifi ath9k_htc firmware was downloaded. So, the
driver would know that it doesn't have to download and reset
the device... But we need a free USB-ID for that.

Alan, Oleksij: What do you think?

Regards,
Chr
--
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: FUSB200 xhci issue

[Christian Lamparter]

On Sunday, July 28, 2013 04:28:25 PM Oleksij Rempel wrote:
 Am 28.07.2013 14:12, schrieb Oleksij Rempel:
  Am 28.07.2013 13:38, schrieb Christian Lamparter:
 
 
  Anyway, I tried the -next branch.
 
  commit dbbb809d592dde0b3c9ecb97b3b387ff8e40e799
  Author: Oleksij Rempel li...@rempel-privat.de
  Date:   Wed Jul 24 10:26:18 2013 +0200
 
k2_fw_usb_api: workaround for EP4 bug.
 
  but still, the device won't show up after autosuspend.
 
  Hm... firmware probably didn't rebooted before suspend. Did interface
  was up, before autosuspend? If no, you need latest wireles-testing -
  there are patches to handle this issue. Or just make ifconfig wlan1 up
 before  rmmod.
  Oh, I it was on the latest wireless-testing. (And the ath9k_htc module
  had the patch ath9k_htc: reboot firmwware if it was loaded).
 
  Furthermore, I did the same test with one of the ehci-only ports
  and it worked. Both, devices (one had a AR7015, the other a AR9271)
  came back after autosuspend there.
 
  Grrr... so it brings us back to xhci issue. Even EP4 workaround wont
  work here :( Suddenly i have no more ideas.
 
  Sarah, it's your turn now.
 
 Christian,
 can you please provide some more info about your xhci controller. I'll 
 try to get me same.

Well, it's a laptop (HP DV6-6003EG). I recon that getting 100% the
same setup will be difficult. However, since the uPD720200 was/is
very popular, it should be very easy to find one. [It's probably
on all of these 10 euro usb-3.0 pcie-adapters. So as long as you
got a free 1x-pcie port you should be good.]

Here's the lspci summary:

19:00.0 USB controller [0c03]: NEC Corporation uPD720200 USB 3.0 Host 
Controller [1033:0194] (rev 04) (prog-if 30 [XHCI])
Subsystem: Hewlett-Packard Company Device [103c:1657]
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx+
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast TAbort- TAbort- 
MAbort- SERR- PERR- INTx-
Latency: 0, Cache Line Size: 64 bytes
Interrupt: pin A routed to IRQ 19
Region 0: Memory at d340 (64-bit, non-prefetchable) [size=8K]
Capabilities: access denied
Kernel driver in use: xhci_hcd

Regards,
Chr
--
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: FUSB200 xhci issue

[Christian Lamparter]

On Wednesday, July 24, 2013 12:37:55 PM Oleksij Rempel wrote:
 Am 23.07.2013 20:26, schrieb Christian Lamparter:
  On Tuesday, July 23, 2013 06:59:46 AM Oleksij Rempel wrote:
  Am 22.07.2013 23:23, schrieb Christian Lamparter:
  On Monday, July 22, 2013 10:47:41 PM Oleksij Rempel wrote:
  Am 22.07.2013 21:54, schrieb Christian Lamparter:
  Hello!
 
  On Monday, July 22, 2013 05:21:54 PM Oleksij Rempel wrote:
  i'm one of ath9k_htc devs. Currently i'm working on usb_suspend issue 
  of
  this adapters. Looks like ar9271 and ar7010 have FUSB200, and i
  accidentally discovered that 9170 have it too. Are there any issue with
  usb-suspend + xhci controllers by you? Did you some how specially
  handled it?
 
  No, I haven't heard any complains about xhci + suspend. In fact,
  it's working fine with the NEC xhci I have. I also have a AR9271
  and AR7010, so if you want I could try if they survive a suspend
  +resume cycle when attached.
 
  But, I do have a bug-report from someone else who has/had? problems
  with carl9170 and xhci. If you want, you can get the details from:
  carl9170 A-MPDU transmit problem:
  http://comments.gmane.org/gmane.linux.kernel.wireless.general/104597
 
  The likely cause is related to Intel's xhci silicon (Ivy Bridge is
  affected, but I don't know about Haswell):
  http://permalink.gmane.org/gmane.linux.kernel.wireless.general/104602
 
  Same situation is here - i have problem on Ivy Bridge.
  (Note: I don't have any Ivy Bridge system. Just Sandy Bridge
  and AMD's new A6-1450 Temash and both xhci work. So I can't
  do any proper comparisons like you.)
 
  Steps to reproduce:
  - plug adapter. Module and firmware will be loaded
  - make sure usb autosupend is enabled. By default it is not! Use
  powertop or directly sysfs to enable autosuspend for this device
  - rmmod  and wait some seconds until adapter is suspended and then
  modprobe ath9k_htc
 
  first packet which is bigger as 64Byte will kill EP4 FIFO. Size register
  will report wrong size.. bigger as FIFO can handle. And only first ~40
  readet bytes will be actually OK.. all the rest of packet will be 
  trashed.
 
  This is what happens with a WN721 (ar9271):
 
  [17619.597905] usbcore: deregistering interface driver ath9k_htc
  [17619.679549] usb 2-2: ath9k_htc: USB layer deinitialized
  [17619.679602] ath9k_htc: Driver unloaded
  suspend
 
  resume
  [17667.543024] usb 2-2: reset high-speed USB device number 3 using 
  xhci_hcd 
  [17667.572168] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88031aa77600
  [17667.572174] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88031aa77640
  [17667.572177] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88031aa77680
  [17667.572180] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88031aa776c0
  [17667.572183] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88031aa77700
  [17667.572185] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88031aa77740
  [17667.573826] usb 2-2: ath9k_htc: Firmware htc_9271.fw requested
  [17667.573873] usbcore: registered new interface driver ath9k_htc
  [17668.038200] usb 2-2: ath9k_htc: Transferred FW: htc_9271.fw, size: 
  51272
  [17668.273249] ath9k_htc 2-2:1.0: ath9k_htc: HTC initialized with 33 
  credits
 
  The driver loads, but there's no wlanX interface, no phyX interface
  and the driver can't be unloaded due to Error: Module ath9k_htc is in 
  use.
 
  So, it is exactly this bug.
  After firmware was loaded ath9k trying to set some registers. Since
  command buffer is trashed it will write some nonsense to registers and
  some times in wrong to wrong addresses. I have some patches to do crc
  check of commands, to easy detect corrupted ones.
 
  You reproduced it on NEC xhci? Is it possible to reproduce it in
  carl9170?
 
  Ok: That's dmesg of your scenario:
 
  [  809.995966] usbcore: deregistering interface driver carl9170
  suspend
 
  resume
  [  826.365596] usb 2-2: reset high-speed USB device number 2 using xhci_hcd
  [  826.431154] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88045af2b900
  [  826.431159] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88045af2b940
  [  826.431162] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88045af2b980
  [  826.431164] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88045af2b9c0
  [  826.432257] usbcore: registered new interface driver carl9170
  [  826.433717] usb 2-2: driver   API: 1.9.8 2013-03-23 [1-1]
  ...
  [  826.816110] usb 2-2: Atheros AR9170 is registered as 'phy3'
 
  carl9170 is able to recover and the stick is working after autosuspend 
  cycle.
 
 Thank you for your tests. USB configuration and handlers look similar on 
 this two firmwares. May be problem

Re: FUSB200 xhci issue

[Christian Lamparter]

On Tuesday, July 23, 2013 06:59:46 AM Oleksij Rempel wrote:
 Am 22.07.2013 23:23, schrieb Christian Lamparter:
  On Monday, July 22, 2013 10:47:41 PM Oleksij Rempel wrote:
  Am 22.07.2013 21:54, schrieb Christian Lamparter:
  Hello!
 
  On Monday, July 22, 2013 05:21:54 PM Oleksij Rempel wrote:
  i'm one of ath9k_htc devs. Currently i'm working on usb_suspend issue of
  this adapters. Looks like ar9271 and ar7010 have FUSB200, and i
  accidentally discovered that 9170 have it too. Are there any issue with
  usb-suspend + xhci controllers by you? Did you some how specially
  handled it?
 
  No, I haven't heard any complains about xhci + suspend. In fact,
  it's working fine with the NEC xhci I have. I also have a AR9271
  and AR7010, so if you want I could try if they survive a suspend
  +resume cycle when attached.
 
  But, I do have a bug-report from someone else who has/had? problems
  with carl9170 and xhci. If you want, you can get the details from:
  carl9170 A-MPDU transmit problem:
  http://comments.gmane.org/gmane.linux.kernel.wireless.general/104597
 
  The likely cause is related to Intel's xhci silicon (Ivy Bridge is
  affected, but I don't know about Haswell):
  http://permalink.gmane.org/gmane.linux.kernel.wireless.general/104602
 
  Same situation is here - i have problem on Ivy Bridge.
  (Note: I don't have any Ivy Bridge system. Just Sandy Bridge
  and AMD's new A6-1450 Temash and both xhci work. So I can't
  do any proper comparisons like you.)
 
  Steps to reproduce:
  - plug adapter. Module and firmware will be loaded
  - make sure usb autosupend is enabled. By default it is not! Use
  powertop or directly sysfs to enable autosuspend for this device
  - rmmod  and wait some seconds until adapter is suspended and then
  modprobe ath9k_htc
 
  first packet which is bigger as 64Byte will kill EP4 FIFO. Size register
  will report wrong size.. bigger as FIFO can handle. And only first ~40
  readet bytes will be actually OK.. all the rest of packet will be trashed.
 
  This is what happens with a WN721 (ar9271):
 
  [17619.597905] usbcore: deregistering interface driver ath9k_htc
  [17619.679549] usb 2-2: ath9k_htc: USB layer deinitialized
  [17619.679602] ath9k_htc: Driver unloaded
  suspend
 
  resume
  [17667.543024] usb 2-2: reset high-speed USB device number 3 using xhci_hcd 
  
  [17667.572168] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88031aa77600
  [17667.572174] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88031aa77640
  [17667.572177] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88031aa77680
  [17667.572180] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88031aa776c0
  [17667.572183] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88031aa77700
  [17667.572185] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
  disabled ep 88031aa77740
  [17667.573826] usb 2-2: ath9k_htc: Firmware htc_9271.fw requested
  [17667.573873] usbcore: registered new interface driver ath9k_htc
  [17668.038200] usb 2-2: ath9k_htc: Transferred FW: htc_9271.fw, size: 51272
  [17668.273249] ath9k_htc 2-2:1.0: ath9k_htc: HTC initialized with 33 credits
 
  The driver loads, but there's no wlanX interface, no phyX interface
  and the driver can't be unloaded due to Error: Module ath9k_htc is in use.
 
 So, it is exactly this bug.
 After firmware was loaded ath9k trying to set some registers. Since 
 command buffer is trashed it will write some nonsense to registers and 
 some times in wrong to wrong addresses. I have some patches to do crc 
 check of commands, to easy detect corrupted ones.
 
 You reproduced it on NEC xhci? Is it possible to reproduce it in 
 carl9170?

Ok: That's dmesg of your scenario:

[  809.995966] usbcore: deregistering interface driver carl9170
suspend

resume
[  826.365596] usb 2-2: reset high-speed USB device number 2 using xhci_hcd
[  826.431154] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
disabled ep 88045af2b900
[  826.431159] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
disabled ep 88045af2b940
[  826.431162] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
disabled ep 88045af2b980
[  826.431164] xhci_hcd :19:00.0: xHCI xhci_drop_endpoint called with 
disabled ep 88045af2b9c0
[  826.432257] usbcore: registered new interface driver carl9170
[  826.433717] usb 2-2: driver   API: 1.9.8 2013-03-23 [1-1]
...
[  826.816110] usb 2-2: Atheros AR9170 is registered as 'phy3'

carl9170 is able to recover and the stick is working after autosuspend cycle.

 How about carl9170 A-MPDU transmit problem?
This was already isolated. The AMPDU transmit problem only happens with 
Ivy Bridge's xhci.

Regards

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

Re: carl9170 A-MPDU transmit problem

[Christian Lamparter]

On Monday, February 25, 2013 12:41:06 AM Alan Stern wrote:
 On Sun, 24 Feb 2013, Christian Lamparter wrote:
 
   The usbmon trace indicates that the data gets delivered to the device
   as it should, but the device doesn't accept it for several seconds.
  
  Looking at the logs, I find myself wondering how the 88012fe19500
  urb-complete ninja'd right in between the 880146c8af00 xmit and
  complete.
  
  88012fe19500 1519981417 S Bo:3:003:1 -115 126 = 7e00 190f0100 
  23232303 42b53600 82b11a00 01c02e00 6a00e846 c2ad3e00
  ...
  880146c8af00 1522200650 S Bo:3:003:1 -115 62 = 3e00 01000500 
  0300    22000$
  88012fe19500 1522200720 C Bo:3:003:1 0 126  
  880146c8af00 1522200756 C Bo:3:003:1 0 62 
 
 In fact this is both normal and required.  Packets to any particular 
 endpoint must always be delivered in order.  Therefore the URBs have
 to complete in the same order as they were submitted.
Yes, I know that ;). I guess I should have said: It's strange that
after such a long silence the urb tx trigger at 1519981417 seemd to
unfreeze the pending urb. It's almost as if a urb completion event
was lost and the urb_complete just had to wait until another tx urb
on the same ep went by to free it.   
 
  From the device side, taking the data shouldn't be a problem. The
  rx is handled by hardware dma. The data from the host is put into
  packages of 320 bytes (The carl9170 firmware has about 180 to 190
  of these 320 byte packages reserved for this purpose. So at no 
  point there should be any long delay because of lack of resources
  or whatever). Also, it doesn't look the was any unusually high 
  load on the link. And the hardware can handle sustained wifi traffic
  up to 160mbit/s (udp peaks at about 180mbit/s) without choking.
  
   Or can you think of any other interesting
  bits that could help to explain why the Arrandale box [...] worked
  perfectly whereas (all his) Ivy Bridge ones have problems.
  (Of course, I assume that it is always the same device, the
  same firmware and the same kernel drivers in all tests, right)?
 
 No, not really.  Unless one is using USB-2 and the other USB-3 -- the 
 device might have a bug in its USB-3 firmware.
Don't worry, the device was designed about 5 years ago. Hence, we don't
need to care about any USB 3.0 features.

 What happens if xhci-hcd is unloaded before the test?
Isn't xhci-hcd needed to drive the usb 3.0 ports? I know about the hub
concept with uhci/ohci and ehci, but I thought they did away with that.

Regards,
Chr
--
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: carl9170 A-MPDU transmit problem

[Christian Lamparter]

On Monday, February 25, 2013 04:29:55 PM Alan Stern wrote:
 On Mon, 25 Feb 2013, Christian Lamparter wrote:
   In fact this is both normal and required.  Packets to any particular 
   endpoint must always be delivered in order.  Therefore the URBs have
   to complete in the same order as they were submitted.
  Yes, I know that ;). I guess I should have said: It's strange that
  after such a long silence the urb tx trigger at 1519981417 seemd to
  unfreeze the pending urb. It's almost as if a urb completion event
  was lost and the urb_complete just had to wait until another tx urb
  on the same ep went by to free it.   
 
 That's quite possible.  The new URB may trigger something in the
 xhci-hcd driver or in the xHCI hardware.  Sarah (CC'ed), the maintainer
 for xhci-hcd, is the person who would know best.

Thanks (Hello Sarah).

One detail that might be important (to keep in mind):

Original report http://www.spinics.net/lists/linux-wireless/msg103880.html:
 On the air everything seems to go smoothly for a while, but 
 then the D-Link adapter stops transmitting *DATA* frames for a while. [...] 
 Eventually it sends the action frame with the *DELBA* request, but
 immediately before sending the action frame it sends a single *DATA*
 frame (1). This pattern repeats each time this happens.

Now if we take this and apply it to the usbmon recording in:
http://www.spinics.net/lists/linux-wireless/msg103915.html

 Normally the time between submission and callback for a given urb
 is short. However, some are much longer, e.g.:

88012fe19500 1519981417 S Bo:3:003:1 -115 126 = 7e00 ... -- DATA

 [... long period where the device receives commands on EP4 and sends
 wifi data to the host via EP2 - so it is working!]

880146c8af00 1522200650 S Bo:3:003:1 -115 62 = 3e00 ... -- DELBA
88012fe19500 1522200720 C Bo:3:003:1 0 126  -- DATA urb completion
880146c8af00 1522200756 C Bo:3:003:1 0 62  -- DELBA urb completion

It would mean that the (delayed) urb with the *DATA* frame urb was not
sent (?or received?) by the usb dongle until the *DELBA* came along (1)
and triggered the TX for both (in quick succession). So, I think we 
should be looking for lost/unhandled interrupts/events.

One more thing: So far this issue only occurs with:
00:14.0 USB controller [0c03]: Intel Corporation 7 Series/C210 Series Chipset 
Family USB xHCI Host Controller [8086:1e31] (rev 04) (prog-if 30 [XHCI])

However, it not all xhci-hcd are affected. I have not seen this with
the NEC Corporation uPD720200 I have in my sandy bridge laptop:
19:00.0 USB controller [0c03]: NEC Corporation uPD720200 USB 3.0 Host 
Controller [1033:0194] (rev 04)

Regards,
Chr
--
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: carl9170 A-MPDU transmit problem

[Christian Lamparter]

On Monday, February 25, 2013 08:46:23 PM Seth Forshee wrote:
 On Mon, Feb 25, 2013 at 11:13:57AM -0800, Sarah Sharp wrote:
  Hmm, yeah, that kind of points to an Intel xHCI hardware issue.  It's
  too bad you don't have a USB analyzer (the high speed ones are about
  $480).  Can you send me a link so I can purchase the device and test it
  with my analyzer?
 
 Hmm. This looks the same, but it sounds like there are multiple hardware
 versions sold under the same name so I'm not sure. The one I have is
 hardware version A2.
 
 http://www.amazon.com/D-Link-DWA-160-Extreme-N-Dual-Band-802-11n/dp/B00127OVHI
 
Or this one:
http://www.ebay.com/itm/D-Link-DWA-160-300Mbps-Xtreme-N-Duo-Band-Wireless-USB-Adapter-Win7-Vista-XP-/180869622792

In the description they say it's A2 (so it should be the same Seth is using)
(Note: The current DWA-160 have RaLink/MediaTek chips)

Regards,
Chr
--
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: carl9170 A-MPDU transmit problem

[Christian Lamparter]

CC'd linux-usb and Stephen.

If some folks there know how to debug these pesky usb transport
stalls/errors?... then please join!
(Start of the discussion was this mail:
http://www.spinics.net/lists/linux-wireless/msg103880.html)

Stephen, I'm sorry to bother you again ;). But can you
tell me how the download DMA engine and FUSB200 handle
and react to usb transfer errors? Is the error detection
and recovery done by the hardware or is there something
the driver/firmware should be aware of?

On Saturday 23 February 2013 07:46:00 Seth Forshee wrote:
 On Sat, Feb 23, 2013 at 12:48:51AM +0100, Christian Lamparter wrote:
  So it looks like we need to ask whenever the USB transport
  is reliable or not. Did you (by any change) also monitor
  usb_tx_anch_urbs [And does it get stuck at some  1 value
  as well?]. I'm asking this because if the driver has more than
  8 (=AR9170_NUM_TX_URBS) concurrently outgoing URBs, the 
  overflow is queued in tx_wait. Of course, the urb completion
  handler (carl9170_usb_tx_data_complete) takes care of
  delivering the next frame in the tx_wait line and so on...
  But according to your report, this doesn't seem to work!
  What's a bit odd is that the device is able to recover. Because 
  normally if there is an USB error, the endpoint will halt and
  no traffic will get through it anymore [So the DELBA should be
  stuck as well!].
 
 The carl9170 usb code seems to be working properly. If tx_anch_urbs
 reaches 8 the overflow is queued in tx_wait as you said, and the next
 queued frame gets delivered from carl9170_usb_tx_data_complete(). The
 stuck frame does get passed to a successful usb_submit_urb() call before
 tx stops, but it still isn't transmitted until the DELBA comes along
 (and tx_anch_urbs decrements to 1 and then gets stuck there while tx is
 stalled, as would be expected).
usb_submit_urb() is async, so unless the URB data structure is
bogus, the device is in the middle of a reset/is removed or OOM
it won't return with an -ENUMBER.

Since neither of us has probably access to an USB analyzer, the
next best thing would be to enable ehci_hcd's debug facilities
and check if the stuck frame produced any DataBufferErr, XactErr
or something else.

Also, we should check what the device is doing. The hardware has
an (Faraday Tech) FUSB200 PHY. It's initialized and partially 
controlled by the carl9170 firmware.
(fw source is available at https://github.com/chunkeey/carl9170fw).

The standard usb code (ep0 control, get/set_configuration/
interface, get_status, ...) is under carlfw/usb/usb.c and
carlfw/usb/main.c.

The code which deals with the I/O of WiFi-frames is located in
carlfw/src/hostif.c (TX is handled by handle_download and
handle_download_exception).

Note: If you want to add printf in the firmware:
INFO(Text %d %x %p, int, hex, pointer);
(And then watch dmesg)

You can also use debugfs's hw_ioread32 and hw_iowrite32 to
monitor and manipulate hardware registers (0x1c-0x1e2000)
and the firmware memory space (0x2-0x203ffc). 
[addresses have to be aligned on a 4-byte boundary]

For example:

To read the FUSB200 register base from 0x1e1000 - 0x1e1034:
# echo 0x1e1000 14  /sys/kernel/debug/.../carl9170/hw_ioread32
# cat /sys/.../hw_ioread32
001e1000 = 8464
001e1004 = 044c097b
...
001e1034 = 

To write some 0xdeadbeef into 0x1e1000:
# echo 0x1e1000 0xdeadbeef  /sys/kernel/debug/.../carl9170/hw_iowrite32

Regards,
Christian
--
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