diff mbox

[LEDE-DEV,3/3] lantiq: add device tree binding for dwc2 on danube

Message ID 3371505.961QRNrVA8@debian64
State RFC
Headers show

Commit Message

Christian Lamparter Oct. 29, 2016, 6:44 p.m. UTC 
On Friday, October 28, 2016 11:22:12 PM CEST Ben Mulvihill wrote:
> On Fri, 2016-10-28 at 18:28 +0200, Christian Lamparter wrote:
> > Hello,
> > 
> > On Friday, October 28, 2016 4:30:56 PM CEST Ben Mulvihill wrote:
> > > Add device tree binding for dwc2 usb driver on lantiq danube
> > > 
> > > Signed-off-by: Ben Mulvihill <ben.mulvihill@gmail.com>
> > > ---
> > > diff -uprN a/target/linux/lantiq/dts/danube.dtsi b/target/linux/lantiq/dts/danube.dtsi
> > > --- a/target/linux/lantiq/dts/danube.dtsi	2016-10-27 19:56:07.090392399 +0200
> > > +++ b/target/linux/lantiq/dts/danube.dtsi	2016-10-27 20:47:34.387511522 +0200
> > > @@ -140,7 +140,7 @@
> > >  		};
> > >  
> > >  		ifxhcd@E101000 {
> > > -			compatible = "lantiq,ifxhcd-danube";
> > > +			compatible = "lantiq,ifxhcd-danube", "lantiq,ifxhcd-danube-dwc2";
> > Usually for device tree, the first compatible string is reserved for the
> > "exact device" that the node represents [0]. So wouldn't switching around
> > the strings (i.e.: "lantiq,ifxhcd-danube-dwc2", "lantiq,ifxhcd-danube")
> > make more sense? After all, the dwc2 is the "more exact device" in this case?
> > 
> 
> Thanks for reviewing.
> 
> Are they not equally "exact"? They are two alternative, and completely
> separate, drivers. (The names chosen for the bindings are perhaps a
> little misleading in that regard.) I Left "lantiq,ifxhcd-danube" in 
> first position simply because it has been the default driver until 
> now, and I didn't want to change the default at this stage. Not that
> it will make any difference to which driver is actually used unless
> for some strange reason someone decides to include both drivers in
> the same build.
> 
> Once we're sure that dwc2 works properly on danube, the old 
> ifxhcd-danube driver can be ditched from the source tree completely.
> But I thought it was better to get an ack from John on
> these first.

Ok, Thanks, I guess this should work fine if you follow through with 
phasing out the old driver too (Yay for that!). The 
"lantiq,ifxhcd-danube" will take preference as it is the first entry,
but that's fine.

I tested the patch on my EasyBox 802 (Arcadyan ARV752DPW).

[    6.291426] dwc2 1e101000.ifxhcd: requested GPIO 464
[    6.294998] dwc2 1e101000.ifxhcd: Configuration mismatch. Forcing host mode
^^

This can be fixed by setting dr_mode to host in the DTS.

---
---

The generic dwc2 defaults to OTG mode (as per generic.txt) [0].
(Well, technically, it's UNKNOWN at first but here's the logic [1]).

[    7.159262] dwc2 1e101000.ifxhcd: DWC OTG Controller
[    7.162832] dwc2 1e101000.ifxhcd: new USB bus registered, assigned bus number 1
[    7.169959] dwc2 1e101000.ifxhcd: irq 62, io mem 0x00000000
[    7.177488] hub 1-0:1.0: USB hub found
[    7.180650] hub 1-0:1.0: 1 port detected
[    7.196214] init: - preinit -

[    7.849577] dwc2 1e101000.ifxhcd: Mode Mismatch Interrupt: currently in Host mode
[    7.855582] dwc2 1e101000.ifxhcd: Mode Mismatch Interrupt: currently in Host mode
[    7.862820] dwc2 1e101000.ifxhcd: Mode Mismatch Interrupt: currently in Host mode
[    8.051083] usb 1-1: new high-speed USB device number 2 using dwc2
[    8.055952] dwc2 1e101000.ifxhcd: Mode Mismatch Interrupt: currently in Host mode
[    8.063064] dwc2 1e101000.ifxhcd: Mode Mismatch Interrupt: currently in Host mode

I don't think these are much of an issue. At least for this device the ifxhcd driver
also produces the same messages. For example, this is an extract bootlog from the
opernwrt wiki [2].

| [    6.980000] IFXUSB: ifxusb_hcd: version 3.2 B110801
| [    7.484000] IFXUSB: USB core #0 soft-reset
| [    7.688000] IFXUSB: USB core #0 soft-reset
| [    7.692000] ifxusb_hcd ifxusb_hcd: IFX USB Controller
| [    7.696000] ifxusb_hcd ifxusb_hcd: new USB bus registered, assigned bus number 1
| [    7.704000] ifxusb_hcd ifxusb_hcd: irq 62, io mem 0xbe101000
| [    7.712000] IFXUSB: Mode Mismatch Interrupt: currently in Host mode
| [    7.716000] IFXUSB: Mode Mismatch Interrupt: currently in Host mode
 
Now, there has been recent changes to dwc2 which might fix these pesky
errors. The discussion can be found on the LKML [3] and three patches
were committed to 4.9-rc.

usb: dwc2: Properly account for the force mode delays [4]
usb: dwc2: Add delay to core soft reset [5]
usb: dwc2: gadget: Only initialize device if in device mode [6]

I'll give then a try and see test if they help. Otherwise:

Tested-by: Christian Lamparter <chunkeey@gmail.com>

Regards,
Christian

[0] <https://www.kernel.org/doc/Documentation/devicetree/bindings/usb/generic.txt>
[1] <http://lxr.free-electrons.com/source/drivers/usb/dwc2/platform.c#L213>
[2] <https://wiki.openwrt.org/toh/arcadyan/arv752dpw>
[3] <https://lkml.org/lkml/2016/4/7/690>

[4] <https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/dwc2?id=2938fc63e0c26bf694436ac81bc776c8b7eced0c>
[5] <https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/dwc2?id=fef6bc37dbafe0d6d71c808c8867a8c5ab4b9816>
[6] <https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/dwc2?id=d0f0ac56b34b28e80223d7086b4decdf027c27ed>

Comments

Christian Lamparter Oct. 29, 2016, 8:18 p.m. UTC | #1 
On Saturday, October 29, 2016 8:44:27 PM CEST Christian Lamparter wrote:
> [...] 
> [    7.849577] dwc2 1e101000.ifxhcd: Mode Mismatch Interrupt: currently in Host mode
> [    7.855582] dwc2 1e101000.ifxhcd: Mode Mismatch Interrupt: currently in Host mode
> [    7.862820] dwc2 1e101000.ifxhcd: Mode Mismatch Interrupt: currently in Host mode
> [    8.051083] usb 1-1: new high-speed USB device number 2 using dwc2
> [    8.055952] dwc2 1e101000.ifxhcd: Mode Mismatch Interrupt: currently in Host mode
> [    8.063064] dwc2 1e101000.ifxhcd: Mode Mismatch Interrupt: currently in Host mode
>
> Now, there has been recent changes to dwc2 which might fix these pesky
> errors. The discussion can be found on the LKML [3] and three patches
> were committed to 4.9-rc.
> 
> usb: dwc2: Properly account for the force mode delays [4]
> usb: dwc2: Add delay to core soft reset [5]
> usb: dwc2: gadget: Only initialize device if in device mode [6]
> 
> I'll give then a try and see test if they help. Otherwise:

Not much. There are now just four "Mode Mismatch Interrupt" instead of five.

I did a short 400MiB read speed test from a fast usb stick.
The dwc2 driver (with the param_ltq values) is faster than 
ifxhcd.

ifxhcd:
real 0m 23.26s
user 0m 0.01s
sys 0m 15.18s

dwc2:
real 0m 16.31s
user 0m 0.00s
sys 0m 8.38s

Regards,
Christian
 
> [4] <https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/dwc2?id=2938fc63e0c26bf694436ac81bc776c8b7eced0c>
> [5] <https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/dwc2?id=fef6bc37dbafe0d6d71c808c8867a8c5ab4b9816>
> [6] <https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/dwc2?id=d0f0ac56b34b28e80223d7086b4decdf027c27ed>
Ben Mulvihill Oct. 30, 2016, 12:10 p.m. UTC | #2 
On Sat, 2016-10-29 at 22:18 +0200, Christian Lamparter wrote:
> On Saturday, October 29, 2016 8:44:27 PM CEST Christian Lamparter wrote:
> > [...] 
> > [    7.849577] dwc2 1e101000.ifxhcd: Mode Mismatch Interrupt: currently in Host mode
> > [    7.855582] dwc2 1e101000.ifxhcd: Mode Mismatch Interrupt: currently in Host mode
> > [    7.862820] dwc2 1e101000.ifxhcd: Mode Mismatch Interrupt: currently in Host mode
> > [    8.051083] usb 1-1: new high-speed USB device number 2 using dwc2
> > [    8.055952] dwc2 1e101000.ifxhcd: Mode Mismatch Interrupt: currently in Host mode
> > [    8.063064] dwc2 1e101000.ifxhcd: Mode Mismatch Interrupt: currently in Host mode
> >
> > Now, there has been recent changes to dwc2 which might fix these pesky
> > errors. The discussion can be found on the LKML [3] and three patches
> > were committed to 4.9-rc.
> > 
> > usb: dwc2: Properly account for the force mode delays [4]
> > usb: dwc2: Add delay to core soft reset [5]
> > usb: dwc2: gadget: Only initialize device if in device mode [6]
> > 
> > I'll give then a try and see test if they help. Otherwise:
> 
> Not much. There are now just four "Mode Mismatch Interrupt" instead of five.
> 
> I did a short 400MiB read speed test from a fast usb stick.
> The dwc2 driver (with the param_ltq values) is faster than 
> ifxhcd.
> 
> ifxhcd:
> real 0m 23.26s
> user 0m 0.01s
> sys 0m 15.18s
> 
> dwc2:
> real 0m 16.31s
> user 0m 0.00s
> sys 0m 8.38s
> 
> Regards,
> Christian
>  
> > [4] <https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/dwc2?id=2938fc63e0c26bf694436ac81bc776c8b7eced0c>
> > [5] <https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/dwc2?id=fef6bc37dbafe0d6d71c808c8867a8c5ab4b9816>
> > [6] <https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/drivers/usb/dwc2?id=d0f0ac56b34b28e80223d7086b4decdf027c27ed>

Thanks for testing this. It looks like we'll have to live with the
"mode mismatch" errors for the moment. I can certainly do a patch to 
add  dr_mode to the dts to get rid of the initial "Configuration 
mismatch" error at least. The same should also be done on ar9 -
I know the error appears there too. What about vrx?
diff mbox

Patch

diff --git a/target/linux/lantiq/dts/danube.dtsi b/target/linux/lantiq/dts/danube.dtsi
index 009696a..538ec19 100644
--- a/target/linux/lantiq/dts/danube.dtsi
+++ b/target/linux/lantiq/dts/danube.dtsi
@@ -146,6 +146,7 @@ 
 			interrupt-parent = <&icu0>;
 			interrupts = <62>;
 			status = "disabled";
+			dr_mode = "host";
 		};
 
 		deu@E103100 {