Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
Hi,
On Sat, Oct 11, 2014 at 01:22:58PM +0800, Huang Rui wrote:
> On Sat, Oct 11, 2014 at 01:14:44PM +0800, Huang Rui wrote:
> > On Fri, Oct 10, 2014 at 09:04:15AM -0500, Felipe Balbi wrote:
> > > Hi,
> > >
> > > On Fri, Oct 10, 2014 at 05:25:34PM +0800, Huang Rui wrote:
> > > > > > I enabled dwc3 and gadget debug/verbose configuration, the whole
> > > > > > testing dmesg
> > > > >
> > > > > oh, that's why it's so slow :-) I'm getting over 30MB/sec with a
> > > > > Cortex
> > > > > A9 :-)
> > > > >
> > > >
> > > > Yes, maybe have two reasons:
> > > > 1) The input clock is much slower than SOC's.
> > > > 2) I used high speed mode.
> > >
> > > right, i'm running at highspeed too.
> > >
> > > > Because of the timing issue on FPGA, bulk write transfer would get
> > > > stuck when use more than 1MB (can pass on small file write) on super
> > > > speed mode. (Gadget Zero failed on 1/3/5/7 with 10s timeout)
> > >
> > > These shouldn't fail. I'll leave testusb running tonight.
> > >
> > > > > > Do you want to see the whole testing dmesg, with which debug level
> > > > > > enablement?
> > > > >
> > > > > This is good for me, thank you.
> > > >
> > > > The test log with booting is attached. Please review.
> > >
> > > will do.
> > >
> > > > > ps: FYI, I left my board running overnight the same test. It has been
> > > > > pretty stable so far.
> > > > >
> > > >
> > > > High speed mode is stable in my FPGA board, but super speed is not
> > > > at current.
> > >
> > > weird. Got any logs ? If you want to share logs I can probably help you
> > > debugging that.
> > >
> >
> > Sure. Below is my controller as super speed mode on gadget zero test 1 (bulk
> > write). Test 9/10 can be passed and device is able to enumerated, so control
> > transfer should be OK.
> >
> > Bus 007 Device 004: ID 0525:a4a0 Netchip Technology, Inc. Linux-USB "Gadget
> > Zero"
> >
> > root@hr-bak:/home/ray/usb# ./testusb.sh 1
> > unknown speed /dev/bus/usb/007/0040
> > /dev/bus/usb/007/004 test 1 --> 110 (Connection timed out)
> >
> > Host:
> > [ 8793.096303] usb 7-1: new SuperSpeed USB device number 4 using xhci_hcd
> > [ 8793.119876] usb 7-1: New USB device found, idVendor=0525, idProduct=a4a0
> > [ 8793.120109] usb 7-1: New USB device strings: Mfr=1, Product=2,
> > SerialNumber=3
> > [ 8793.120352] usb 7-1: Product: Gadget Zero
> > [ 8793.120493] usb 7-1: Manufacturer: Linux 3.17.0-rc5-dwc3-upstream+ with
> > dwc3-gadget
> > [ 8793.120751] usb 7-1: SerialNumber: 0123456789.0123456789.0123456789
> > [ 8793.489749] usbtest 7-1:3.0: Linux gadget zero
> > [ 8793.489933] usbtest 7-1:3.0: super-speed {control in/out bulk-in
> > bulk-out} tests (+alt)
> > [ 8793.490246] usbcore: registered new interface driver usbtest
> > [ 8815.325781] usbcore: deregistering interface driver usbtest
> > [ 8819.760443] usbtest 7-1:3.0: Linux gadget zero
> > [ 8819.760621] usbtest 7-1:3.0: super-speed {control in/out bulk-in
> > bulk-out} tests (+alt)
> > [ 8819.760921] usbcore: registered new interface driver usbtest
> > [ 8891.317350] usbtest 7-1:3.0: TEST 1: write 512 bytes 20 times
> > [ 8901.316770] usb 7-1: test1 failed, iterations left 19, status -110 (not
> > 0)
oh, ok. See this thread:
http://marc.info/?l=linux-usb&m=141296688426206
--
balbi
signature.asc
Description: Digital signature
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
On Sat, Oct 11, 2014 at 01:14:44PM +0800, Huang Rui wrote:
> On Fri, Oct 10, 2014 at 09:04:15AM -0500, Felipe Balbi wrote:
> > Hi,
> >
> > On Fri, Oct 10, 2014 at 05:25:34PM +0800, Huang Rui wrote:
> > > > > I enabled dwc3 and gadget debug/verbose configuration, the whole
> > > > > testing dmesg
> > > >
> > > > oh, that's why it's so slow :-) I'm getting over 30MB/sec with a Cortex
> > > > A9 :-)
> > > >
> > >
> > > Yes, maybe have two reasons:
> > > 1) The input clock is much slower than SOC's.
> > > 2) I used high speed mode.
> >
> > right, i'm running at highspeed too.
> >
> > > Because of the timing issue on FPGA, bulk write transfer would get
> > > stuck when use more than 1MB (can pass on small file write) on super
> > > speed mode. (Gadget Zero failed on 1/3/5/7 with 10s timeout)
> >
> > These shouldn't fail. I'll leave testusb running tonight.
> >
> > > > > Do you want to see the whole testing dmesg, with which debug level
> > > > > enablement?
> > > >
> > > > This is good for me, thank you.
> > >
> > > The test log with booting is attached. Please review.
> >
> > will do.
> >
> > > > ps: FYI, I left my board running overnight the same test. It has been
> > > > pretty stable so far.
> > > >
> > >
> > > High speed mode is stable in my FPGA board, but super speed is not
> > > at current.
> >
> > weird. Got any logs ? If you want to share logs I can probably help you
> > debugging that.
> >
>
> Sure. Below is my controller as super speed mode on gadget zero test 1 (bulk
> write). Test 9/10 can be passed and device is able to enumerated, so control
> transfer should be OK.
>
> Bus 007 Device 004: ID 0525:a4a0 Netchip Technology, Inc. Linux-USB "Gadget
> Zero"
>
> root@hr-bak:/home/ray/usb# ./testusb.sh 1
> unknown speed /dev/bus/usb/007/0040
> /dev/bus/usb/007/004 test 1 --> 110 (Connection timed out)
>
> Host:
> [ 8793.096303] usb 7-1: new SuperSpeed USB device number 4 using xhci_hcd
> [ 8793.119876] usb 7-1: New USB device found, idVendor=0525, idProduct=a4a0
> [ 8793.120109] usb 7-1: New USB device strings: Mfr=1, Product=2,
> SerialNumber=3
> [ 8793.120352] usb 7-1: Product: Gadget Zero
> [ 8793.120493] usb 7-1: Manufacturer: Linux 3.17.0-rc5-dwc3-upstream+ with
> dwc3-gadget
> [ 8793.120751] usb 7-1: SerialNumber: 0123456789.0123456789.0123456789
> [ 8793.489749] usbtest 7-1:3.0: Linux gadget zero
> [ 8793.489933] usbtest 7-1:3.0: super-speed {control in/out bulk-in bulk-out}
> tests (+alt)
> [ 8793.490246] usbcore: registered new interface driver usbtest
> [ 8815.325781] usbcore: deregistering interface driver usbtest
> [ 8819.760443] usbtest 7-1:3.0: Linux gadget zero
> [ 8819.760621] usbtest 7-1:3.0: super-speed {control in/out bulk-in bulk-out}
> tests (+alt)
> [ 8819.760921] usbcore: registered new interface driver usbtest
> [ 8891.317350] usbtest 7-1:3.0: TEST 1: write 512 bytes 20 times
> [ 8901.316770] usb 7-1: test1 failed, iterations left 19, status -110 (not 0)
>
> Device:
> [ 7872.401865] udc dwc3.0.auto: registering UDC driver [zero]
> [ 7872.420057] zero gadget: adding 'source/sink'/88002e593e00 to config
> 'source/sink'/a01ad000
> [ 7872.420072] zero gadget: super speed source/sink: IN/ep1in, OUT/ep1out,
> ISO-IN/ep2in, ISO-OUT/ep2out, INT-IN/ep3in, INT-OUT/ep3out
> [ 7872.420076] zero gadget: adding 'loopback'/88002e593000 to config
> 'loopback'/a01ad0e0
> [ 7872.420081] zero gadget: super speed loopback: IN/ep1in, OUT/ep1out
> [ 7872.420086] zero gadget: Gadget Zero, version: Cinco de Mayo 2008
> [ 7872.420089] zero gadget: zero ready
> [ 7872.661926] dwc3 dwc3.0.auto: request 880186d91180 from ep0out
> completed 8/8 ===> 0
> [ 7872.662505] dwc3 dwc3.0.auto: request 880186d91180 from ep0out
> completed 18/18 ===> 0
> [ 7872.663039] dwc3 dwc3.0.auto: request 880186d91180 from ep0out
> completed 5/5 ===> 0
> [ 7872.663655] dwc3 dwc3.0.auto: request 880186d91180 from ep0out
> completed 22/22 ===> 0
> [ 7872.664261] dwc3 dwc3.0.auto: request 880186d91180 from ep0out
> completed 9/9 ===> 0
> [ 7872.664890] dwc3 dwc3.0.auto: request 880186d91180 from ep0out
> completed 140/140 ===> 0
> [ 7872.665924] dwc3 dwc3.0.auto: request 880186d91180 from ep0out
> completed 9/9 ===> 0
> [ 7872.666493] dwc3 dwc3.0.auto: request 880186d91180 from ep0out
> completed 44/44 ===> 0
> [ 7872.667596] dwc3 dwc3.0.auto: request 880186d91180 from ep0out
> completed 4/4 ===> 0
> [ 7872.668135] dwc3 dwc3.0.auto: request 880186d91180 from ep0out
> completed 24/24 ===> 0
> [ 7872.668933] dwc3 dwc3.0.auto: request 880186d91180 from ep0out
> completed 98/98 ===> 0
> [ 7872.669501] dwc3 dwc3.0.auto: request 880186d91180 from ep0out
> completed 66/66 ===> 0
> [ 7872.671680] zero gadget: super-speed config #3: source/sink
> [ 7872.671766] zero gadget: source/sink enabled, alt intf 0
> [ 7872.671898] dwc3 dwc3.0.auto: request 880186d91180 from ep0out
> completed 0/0 ===> 0
> [ 7872.672400] dwc3
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
On Fri, Oct 10, 2014 at 09:04:15AM -0500, Felipe Balbi wrote:
> Hi,
>
> On Fri, Oct 10, 2014 at 05:25:34PM +0800, Huang Rui wrote:
> > > > I enabled dwc3 and gadget debug/verbose configuration, the whole
> > > > testing dmesg
> > >
> > > oh, that's why it's so slow :-) I'm getting over 30MB/sec with a Cortex
> > > A9 :-)
> > >
> >
> > Yes, maybe have two reasons:
> > 1) The input clock is much slower than SOC's.
> > 2) I used high speed mode.
>
> right, i'm running at highspeed too.
>
> > Because of the timing issue on FPGA, bulk write transfer would get
> > stuck when use more than 1MB (can pass on small file write) on super
> > speed mode. (Gadget Zero failed on 1/3/5/7 with 10s timeout)
>
> These shouldn't fail. I'll leave testusb running tonight.
>
> > > > Do you want to see the whole testing dmesg, with which debug level
> > > > enablement?
> > >
> > > This is good for me, thank you.
> >
> > The test log with booting is attached. Please review.
>
> will do.
>
> > > ps: FYI, I left my board running overnight the same test. It has been
> > > pretty stable so far.
> > >
> >
> > High speed mode is stable in my FPGA board, but super speed is not
> > at current.
>
> weird. Got any logs ? If you want to share logs I can probably help you
> debugging that.
>
Sure. Below is my controller as super speed mode on gadget zero test 1 (bulk
write). Test 9/10 can be passed and device is able to enumerated, so control
transfer should be OK.
Bus 007 Device 004: ID 0525:a4a0 Netchip Technology, Inc. Linux-USB "Gadget
Zero"
root@hr-bak:/home/ray/usb# ./testusb.sh 1
unknown speed /dev/bus/usb/007/0040
/dev/bus/usb/007/004 test 1 --> 110 (Connection timed out)
Host:
[ 8793.096303] usb 7-1: new SuperSpeed USB device number 4 using xhci_hcd
[ 8793.119876] usb 7-1: New USB device found, idVendor=0525, idProduct=a4a0
[ 8793.120109] usb 7-1: New USB device strings: Mfr=1, Product=2, SerialNumber=3
[ 8793.120352] usb 7-1: Product: Gadget Zero
[ 8793.120493] usb 7-1: Manufacturer: Linux 3.17.0-rc5-dwc3-upstream+ with
dwc3-gadget
[ 8793.120751] usb 7-1: SerialNumber: 0123456789.0123456789.0123456789
[ 8793.489749] usbtest 7-1:3.0: Linux gadget zero
[ 8793.489933] usbtest 7-1:3.0: super-speed {control in/out bulk-in bulk-out}
tests (+alt)
[ 8793.490246] usbcore: registered new interface driver usbtest
[ 8815.325781] usbcore: deregistering interface driver usbtest
[ 8819.760443] usbtest 7-1:3.0: Linux gadget zero
[ 8819.760621] usbtest 7-1:3.0: super-speed {control in/out bulk-in bulk-out}
tests (+alt)
[ 8819.760921] usbcore: registered new interface driver usbtest
[ 8891.317350] usbtest 7-1:3.0: TEST 1: write 512 bytes 20 times
[ 8901.316770] usb 7-1: test1 failed, iterations left 19, status -110 (not 0)
Device:
[ 7872.401865] udc dwc3.0.auto: registering UDC driver [zero]
[ 7872.420057] zero gadget: adding 'source/sink'/88002e593e00 to config
'source/sink'/a01ad000
[ 7872.420072] zero gadget: super speed source/sink: IN/ep1in, OUT/ep1out,
ISO-IN/ep2in, ISO-OUT/ep2out, INT-IN/ep3in, INT-OUT/ep3out
[ 7872.420076] zero gadget: adding 'loopback'/88002e593000 to config
'loopback'/a01ad0e0
[ 7872.420081] zero gadget: super speed loopback: IN/ep1in, OUT/ep1out
[ 7872.420086] zero gadget: Gadget Zero, version: Cinco de Mayo 2008
[ 7872.420089] zero gadget: zero ready
[ 7872.661926] dwc3 dwc3.0.auto: request 880186d91180 from ep0out completed
8/8 ===> 0
[ 7872.662505] dwc3 dwc3.0.auto: request 880186d91180 from ep0out completed
18/18 ===> 0
[ 7872.663039] dwc3 dwc3.0.auto: request 880186d91180 from ep0out completed
5/5 ===> 0
[ 7872.663655] dwc3 dwc3.0.auto: request 880186d91180 from ep0out completed
22/22 ===> 0
[ 7872.664261] dwc3 dwc3.0.auto: request 880186d91180 from ep0out completed
9/9 ===> 0
[ 7872.664890] dwc3 dwc3.0.auto: request 880186d91180 from ep0out completed
140/140 ===> 0
[ 7872.665924] dwc3 dwc3.0.auto: request 880186d91180 from ep0out completed
9/9 ===> 0
[ 7872.666493] dwc3 dwc3.0.auto: request 880186d91180 from ep0out completed
44/44 ===> 0
[ 7872.667596] dwc3 dwc3.0.auto: request 880186d91180 from ep0out completed
4/4 ===> 0
[ 7872.668135] dwc3 dwc3.0.auto: request 880186d91180 from ep0out completed
24/24 ===> 0
[ 7872.668933] dwc3 dwc3.0.auto: request 880186d91180 from ep0out completed
98/98 ===> 0
[ 7872.669501] dwc3 dwc3.0.auto: request 880186d91180 from ep0out completed
66/66 ===> 0
[ 7872.671680] zero gadget: super-speed config #3: source/sink
[ 7872.671766] zero gadget: source/sink enabled, alt intf 0
[ 7872.671898] dwc3 dwc3.0.auto: request 880186d91180 from ep0out completed
0/0 ===> 0
[ 7872.672400] dwc3 dwc3.0.auto: request 880186d91180 from ep0out completed
42/42 ===> 0
[ 7970.768261] dwc3 dwc3.0.auto: request 88018778eb40 from ep1in completed
0/512 ===> -108
[ 7970.768277] dwc3 dwc3.0.auto: request 88018778e600 from ep1out completed
0/512 ===> -108
[ 7970.768349] zero ga
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
Hi, On Fri, Oct 10, 2014 at 05:25:34PM +0800, Huang Rui wrote: > > > I enabled dwc3 and gadget debug/verbose configuration, the whole testing > > > dmesg > > > > oh, that's why it's so slow :-) I'm getting over 30MB/sec with a Cortex > > A9 :-) > > > > Yes, maybe have two reasons: > 1) The input clock is much slower than SOC's. > 2) I used high speed mode. right, i'm running at highspeed too. > Because of the timing issue on FPGA, bulk write transfer would get > stuck when use more than 1MB (can pass on small file write) on super > speed mode. (Gadget Zero failed on 1/3/5/7 with 10s timeout) These shouldn't fail. I'll leave testusb running tonight. > > > Do you want to see the whole testing dmesg, with which debug level > > > enablement? > > > > This is good for me, thank you. > > The test log with booting is attached. Please review. will do. > > ps: FYI, I left my board running overnight the same test. It has been > > pretty stable so far. > > > > High speed mode is stable in my FPGA board, but super speed is not > at current. weird. Got any logs ? If you want to share logs I can probably help you debugging that. -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
Hi, On Fri, Oct 10, 2014 at 08:43:20AM +0800, Huang Rui wrote: > > > But it does't block my building. Because I can comment CROSS_COMPILE at > > > makefile. > > > > > > I am testing my driver and dwc3 controller with MSC tool currently, and > > > will > > > let you know message log later. > > > > Cool, thanks. > > > > I have completed a round of MSC testing. All the test cases are passed. Please > see below result: > > root@hr-ub:/home/ray/felipe/usb-tools# ./msc.sh -o /dev/sdb1 > Starting test suite: 2014年 10月 09日 星期四 18:02:41 CST > test 0a: simple 4k read/write > test 0: sent 3.91 MB read 0.03 MB/s write 0.03 MB/s ... > success > test 0b: simple 8k read/write > test 0: sent 7.81 MB read 6.15 MB/s write 5.96 MB/s ... > success > test 0c: simple 16k read/write > test 0: sent 15.62 MB read 14.30 MB/s write 12.97 MB/s ... > success > test 0d: simple 32k read/write > test 0: sent 31.25 MB read 18.26 MB/s write 17.10 MB/s ... > success > test 0e: simple 64k read/write > test 0: sent 62.50 MB read 22.00 MB/s write 16.33 MB/s ... > success > test 1: simple 1-sector read/write > test 1: sent 500.00 kB read 1.05 MB/s write 0.93 MB/s ... > success > test 2: simple 8-sectors read/write > test 2: sent 3.91 MB read 6.52 MB/s write 6.10 MB/s ... > success > test 3: simple 32-sectors read/write > test 3: sent 15.62 MB read 14.33 MB/s write 13.14 MB/s ... > success > test 4: simple 64-sectors read/write > test 4: sent 31.25 MB read 17.79 MB/s write 16.38 MB/s ... > success > test 5a: scatter/gather for 2-sectors buflen 4k > test 5: sent1000.00 kB read 1.85 MB/s write 1.56 MB/s ... > success > test 5b: scatter/gather for 2-sectors buflen 8k > test 5: sent1000.00 kB read 1.91 MB/s write 1.63 MB/s ... > success > test 5c: scatter/gather for 2-sectors buflen 16k > test 5: sent1000.00 kB read 1.94 MB/s write 1.62 MB/s ... > success > test 5d: scatter/gather for 2-sectors buflen 32k > test 5: sent1000.00 kB read 1.93 MB/s write 1.64 MB/s ... > success > test 5e: scatter/gather for 2-sectors buflen 64k > test 5: sent1000.00 kB read 1.96 MB/s write 1.68 MB/s ... > success > test 6a: scatter/gather for 8-sectors buflen 4k > test 6: sent 3.91 MB read 6.53 MB/s write 6.05 MB/s ... > success > test 6b: scatter/gather for 8-sectors buflen 8k > test 6: sent 3.91 MB read 6.56 MB/s write 5.98 MB/s ... > success > test 6c: scatter/gather for 8-sectors buflen 16k > test 6: sent 3.91 MB read 6.55 MB/s write 6.15 MB/s ... > success > test 6d: scatter/gather for 8-sectors buflen 32k > test 6: sent 3.91 MB read 6.51 MB/s write 6.06 MB/s ... > success > test 6e: scatter/gather for 8-sectors buflen 64k > test 6: sent 3.91 MB read 6.50 MB/s write 6.06 MB/s ... > success > test 7a: scatter/gather for 32-sectors buflen 16k > test 7: sent 15.62 MB read 14.23 MB/s write 12.99 MB/s ... > success > test 7b: scatter/gather for 32-sectors buflen 32k > test 7: sent 15.62 MB read 14.27 MB/s write 12.91 MB/s ... > success > test 7c: scatter/gather for 32-sectors buflen 64k > test 7: sent 15.62 MB read 14.73 MB/s write 13.00 MB/s ... > success > test 8a: scatter/gather for 64-sectors buflen 32k > test 8: sent 31.25 MB read 18.42 MB/s write 16.65 MB/s ... > success > test 8b: scatter/gather for 64-sectors buflen 64k > test 8: sent 31.25 MB read 17.70 MB/s write 16.39 MB/s ... > success > test 9: scatter/gather for 128-sectors buflen 64k > test 9: sent 62.50 MB read 21.14 MB/s write 16.07 MB/s ... > success > test 10: read over the end of the block device > test 10: sent 62.01 MB read 0.00 MB/s write 0.00 MB/s ... > success > test 11: lseek past the end of the block device > test 11: sent 0.00 B read 0.00 MB/s write 0.00 MB/s ... > success > test 12: write over the end of the block device > test 12: sent 0.00 B read 0.00 MB/s write 0.00 MB/s ... > success > test 13: write 1 sg, read 8 random size sgs > test 13: sent 62.50 MB read 21.61 MB/s write 16.21 MB/s ... > success > test 14: write 8 random size sgs, read 1 sg > test 14: sent 62.50 MB read 22.52 MB/s write 18.61 MB/s ... > success > test 15: write and read 8 random size sgs > test 15: sent 62.50 MB read 22.31 MB/s write 19.28 MB/s ... > success > test 16a: read with heap allocated buffer > test 16: sent 62.50 MB read 21.69 MB/s write 0.00 MB/s ... > success > test 16b: read with stack allocated buffer > test 16: sent 62.50 MB read 21.42 MB/s write 0.00 MB/s ... > success > test 17a: write with heap allocated buffer
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
On Thu, Oct 09, 2014 at 10:09:37AM -0500, Felipe Balbi wrote: > Hi, > > On Thu, Oct 09, 2014 at 01:10:36PM +0800, Huang Rui wrote: > > > don't worry about that. Some of the tools need to run on the target but > you're not doing that :-) You can make a specific tool by e.g. "make msc". > > > But it does't block my building. Because I can comment CROSS_COMPILE at > > makefile. > > > > I am testing my driver and dwc3 controller with MSC tool currently, and will > > let you know message log later. > > Cool, thanks. > I have completed a round of MSC testing. All the test cases are passed. Please see below result: root@hr-ub:/home/ray/felipe/usb-tools# ./msc.sh -o /dev/sdb1 Starting test suite: 2014年 10月 09日 星期四 18:02:41 CST test 0a: simple 4k read/write test 0: sent 3.91 MB read 0.03 MB/s write 0.03 MB/s ... success test 0b: simple 8k read/write test 0: sent 7.81 MB read 6.15 MB/s write 5.96 MB/s ... success test 0c: simple 16k read/write test 0: sent 15.62 MB read 14.30 MB/s write 12.97 MB/s ... success test 0d: simple 32k read/write test 0: sent 31.25 MB read 18.26 MB/s write 17.10 MB/s ... success test 0e: simple 64k read/write test 0: sent 62.50 MB read 22.00 MB/s write 16.33 MB/s ... success test 1: simple 1-sector read/write test 1: sent 500.00 kB read 1.05 MB/s write 0.93 MB/s ... success test 2: simple 8-sectors read/write test 2: sent 3.91 MB read 6.52 MB/s write 6.10 MB/s ... success test 3: simple 32-sectors read/write test 3: sent 15.62 MB read 14.33 MB/s write 13.14 MB/s ... success test 4: simple 64-sectors read/write test 4: sent 31.25 MB read 17.79 MB/s write 16.38 MB/s ... success test 5a: scatter/gather for 2-sectors buflen 4k test 5: sent1000.00 kB read 1.85 MB/s write 1.56 MB/s ... success test 5b: scatter/gather for 2-sectors buflen 8k test 5: sent1000.00 kB read 1.91 MB/s write 1.63 MB/s ... success test 5c: scatter/gather for 2-sectors buflen 16k test 5: sent1000.00 kB read 1.94 MB/s write 1.62 MB/s ... success test 5d: scatter/gather for 2-sectors buflen 32k test 5: sent1000.00 kB read 1.93 MB/s write 1.64 MB/s ... success test 5e: scatter/gather for 2-sectors buflen 64k test 5: sent1000.00 kB read 1.96 MB/s write 1.68 MB/s ... success test 6a: scatter/gather for 8-sectors buflen 4k test 6: sent 3.91 MB read 6.53 MB/s write 6.05 MB/s ... success test 6b: scatter/gather for 8-sectors buflen 8k test 6: sent 3.91 MB read 6.56 MB/s write 5.98 MB/s ... success test 6c: scatter/gather for 8-sectors buflen 16k test 6: sent 3.91 MB read 6.55 MB/s write 6.15 MB/s ... success test 6d: scatter/gather for 8-sectors buflen 32k test 6: sent 3.91 MB read 6.51 MB/s write 6.06 MB/s ... success test 6e: scatter/gather for 8-sectors buflen 64k test 6: sent 3.91 MB read 6.50 MB/s write 6.06 MB/s ... success test 7a: scatter/gather for 32-sectors buflen 16k test 7: sent 15.62 MB read 14.23 MB/s write 12.99 MB/s ... success test 7b: scatter/gather for 32-sectors buflen 32k test 7: sent 15.62 MB read 14.27 MB/s write 12.91 MB/s ... success test 7c: scatter/gather for 32-sectors buflen 64k test 7: sent 15.62 MB read 14.73 MB/s write 13.00 MB/s ... success test 8a: scatter/gather for 64-sectors buflen 32k test 8: sent 31.25 MB read 18.42 MB/s write 16.65 MB/s ... success test 8b: scatter/gather for 64-sectors buflen 64k test 8: sent 31.25 MB read 17.70 MB/s write 16.39 MB/s ... success test 9: scatter/gather for 128-sectors buflen 64k test 9: sent 62.50 MB read 21.14 MB/s write 16.07 MB/s ... success test 10: read over the end of the block device test 10: sent 62.01 MB read 0.00 MB/s write 0.00 MB/s ... success test 11: lseek past the end of the block device test 11: sent 0.00 B read 0.00 MB/s write 0.00 MB/s ... success test 12: write over the end of the block device test 12: sent 0.00 B read 0.00 MB/s write 0.00 MB/s ... success test 13: write 1 sg, read 8 random size sgs test 13: sent 62.50 MB read 21.61 MB/s write 16.21 MB/s ... success test 14: write 8 random size sgs, read 1 sg test 14: sent 62.50 MB read 22.52 MB/s write 18.61 MB/s ... success test 15: write and read 8 random size sgs test 15: sent 62.50 MB read 22.31 MB/s write 19.28 MB/s ... success test 16a: read with heap allocated buffer test 16: sent 62.50 MB read 21.69 MB/s write 0.00 MB/s ... success test 16b: read with stack allocated buffer test 16: sent 62.50 MB read 21.42 MB/s write 0.00 MB/s ... success test 17a: write with heap allocated buffer tes
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
On Fri, Oct 10, 2014 at 08:43:19AM +0800, Huang Rui wrote: > On Thu, Oct 09, 2014 at 10:09:37AM -0500, Felipe Balbi wrote: > > Hi, > > > > On Thu, Oct 09, 2014 at 01:10:36PM +0800, Huang Rui wrote: > > > > > > > > > don't worry about that. Some of the tools need to run on the target but > > you're not doing that :-) You can make a specific tool by e.g. "make msc". > > > > > But it does't block my building. Because I can comment CROSS_COMPILE at > > > makefile. > > > > > > I am testing my driver and dwc3 controller with MSC tool currently, and > > > will > > > let you know message log later. > > > > Cool, thanks. > > > > I have completed a round of MSC testing. All the test cases are passed. Please > see below result: > > root@hr-ub:/home/ray/felipe/usb-tools# ./msc.sh -o /dev/sdb1 > Starting test suite: 2014年 10月 09日 星期四 18:02:41 CST > test 0a: simple 4k read/write > test 0: sent 3.91 MB read 0.03 MB/s write 0.03 MB/s ... > success > test 0b: simple 8k read/write > test 0: sent 7.81 MB read 6.15 MB/s write 5.96 MB/s ... > success > test 0c: simple 16k read/write > test 0: sent 15.62 MB read 14.30 MB/s write 12.97 MB/s ... > success > test 0d: simple 32k read/write > test 0: sent 31.25 MB read 18.26 MB/s write 17.10 MB/s ... > success > test 0e: simple 64k read/write > test 0: sent 62.50 MB read 22.00 MB/s write 16.33 MB/s ... > success > test 1: simple 1-sector read/write > test 1: sent 500.00 kB read 1.05 MB/s write 0.93 MB/s ... > success > test 2: simple 8-sectors read/write > test 2: sent 3.91 MB read 6.52 MB/s write 6.10 MB/s ... > success > test 3: simple 32-sectors read/write > test 3: sent 15.62 MB read 14.33 MB/s write 13.14 MB/s ... > success > test 4: simple 64-sectors read/write > test 4: sent 31.25 MB read 17.79 MB/s write 16.38 MB/s ... > success > test 5a: scatter/gather for 2-sectors buflen 4k > test 5: sent1000.00 kB read 1.85 MB/s write 1.56 MB/s ... > success > test 5b: scatter/gather for 2-sectors buflen 8k > test 5: sent1000.00 kB read 1.91 MB/s write 1.63 MB/s ... > success > test 5c: scatter/gather for 2-sectors buflen 16k > test 5: sent1000.00 kB read 1.94 MB/s write 1.62 MB/s ... > success > test 5d: scatter/gather for 2-sectors buflen 32k > test 5: sent1000.00 kB read 1.93 MB/s write 1.64 MB/s ... > success > test 5e: scatter/gather for 2-sectors buflen 64k > test 5: sent1000.00 kB read 1.96 MB/s write 1.68 MB/s ... > success > test 6a: scatter/gather for 8-sectors buflen 4k > test 6: sent 3.91 MB read 6.53 MB/s write 6.05 MB/s ... > success > test 6b: scatter/gather for 8-sectors buflen 8k > test 6: sent 3.91 MB read 6.56 MB/s write 5.98 MB/s ... > success > test 6c: scatter/gather for 8-sectors buflen 16k > test 6: sent 3.91 MB read 6.55 MB/s write 6.15 MB/s ... > success > test 6d: scatter/gather for 8-sectors buflen 32k > test 6: sent 3.91 MB read 6.51 MB/s write 6.06 MB/s ... > success > test 6e: scatter/gather for 8-sectors buflen 64k > test 6: sent 3.91 MB read 6.50 MB/s write 6.06 MB/s ... > success > test 7a: scatter/gather for 32-sectors buflen 16k > test 7: sent 15.62 MB read 14.23 MB/s write 12.99 MB/s ... > success > test 7b: scatter/gather for 32-sectors buflen 32k > test 7: sent 15.62 MB read 14.27 MB/s write 12.91 MB/s ... > success > test 7c: scatter/gather for 32-sectors buflen 64k > test 7: sent 15.62 MB read 14.73 MB/s write 13.00 MB/s ... > success > test 8a: scatter/gather for 64-sectors buflen 32k > test 8: sent 31.25 MB read 18.42 MB/s write 16.65 MB/s ... > success > test 8b: scatter/gather for 64-sectors buflen 64k > test 8: sent 31.25 MB read 17.70 MB/s write 16.39 MB/s ... > success > test 9: scatter/gather for 128-sectors buflen 64k > test 9: sent 62.50 MB read 21.14 MB/s write 16.07 MB/s ... > success > test 10: read over the end of the block device > test 10: sent 62.01 MB read 0.00 MB/s write 0.00 MB/s ... > success > test 11: lseek past the end of the block device > test 11: sent 0.00 B read 0.00 MB/s write 0.00 MB/s ... > success > test 12: write over the end of the block device > test 12: sent 0.00 B read 0.00 MB/s write 0.00 MB/s ... > success > test 13: write 1 sg, read 8 random size sgs > test 13: sent 62.50 MB read 21.61 MB/s write 16.21 MB/s ... > success > test 14: write 8 random size sgs, read 1 sg > test 14: sent 62.50 MB read 22.52 MB/s write 18.61 MB/s ... > success > test 15: write and read 8 random size sgs > test 15: sent 62.50 MB read 22.31 MB/s write 19.28 MB/s ... > suc
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
Hi, On Thu, Oct 09, 2014 at 01:10:36PM +0800, Huang Rui wrote: > > > > ray@hr-ub:~/felipe/usb-tools$ make > > > > LINK companion-desc > > > > companion-desc.o: In function `main': > > > > /home/ray/felipe/usb-tools/companion-desc.c:219: undefined reference to > > > > `libusb_init' > > > > /home/ray/felipe/usb-tools/companion-desc.c:221: undefined reference to > > > > `libusb_open_device_with_vid_pid' > > > > companion-desc.o: In function `do_test': > > > > /home/ray/felipe/usb-tools/companion-desc.c:176: undefined reference to > > > > `libusb_get_device' > > > > /home/ray/felipe/usb-tools/companion-desc.c:178: undefined reference to > > > > `libusb_get_device_descriptor' > > > > companion-desc.o: In function `check_configurations': > > > > /home/ray/felipe/usb-tools/companion-desc.c:153: undefined reference to > > > > `libusb_get_config_descriptor' > > > > companion-desc.o: In function `main': > > > > /home/ray/felipe/usb-tools/companion-desc.c:239: undefined reference to > > > > `libusb_close' > > > > /home/ray/felipe/usb-tools/companion-desc.c:242: undefined reference to > > > > `libusb_exit' > > > > companion-desc.o: In function `do_test': > > > > /home/ray/felipe/usb-tools/companion-desc.c:163: undefined reference to > > > > `libusb_free_config_descriptor' > > > > collect2: error: ld returned 1 exit status > > > > make: *** [companion-desc] Error 1 > > > > alright, libraries should be listed last. But only a few GCC versions > > are more anal about it. Anyway, pushed a patch on Makefile, see if it > > works for you. > > > > Felipe, I am on Chinese holiday in the past week. And back to work on > patch refine. > > After fetch your latest codes of usbtool, anything is good except a > arm gcc error, since I used x86 platform. > > ray@hr-ub:~/felipe/usb-tools$ make > CC companion-desc.o > LINK companion-desc > CC testmode.o > LINK testmode > CC cleware.o > LINK cleware > CC control.o > LINK control > CC device-reset.o > LINK device-reset > CC msc.o > LINK msc > CC testusb.o > LINK testusb > CC serialc.o > LINK serialc > CC acmc.o > LINK acmc > CC switchbox.o > LINK switchbox > CC seriald.o > /bin/sh: 1: arm-linux-gcc: not found > make: *** [seriald.o] Error 127 don't worry about that. Some of the tools need to run on the target but you're not doing that :-) You can make a specific tool by e.g. "make msc". > But it does't block my building. Because I can comment CROSS_COMPILE at > makefile. > > I am testing my driver and dwc3 controller with MSC tool currently, and will > let you know message log later. Cool, thanks. -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
On Tue, Sep 30, 2014 at 09:33:49AM -0500, Felipe Balbi wrote:
> On Mon, Sep 29, 2014 at 11:48:41PM -0500, Felipe Balbi wrote:
> > Hi,
> >
> > On Tue, Sep 30, 2014 at 11:12:55AM +0800, Huang Rui wrote:
> > > > > > > > > > On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> > > > > > > > > > > diff --git a/drivers/usb/dwc3/core.c
> > > > > > > > > > > b/drivers/usb/dwc3/core.c
> > > > > > > > > > > index b0f4d52..6138c5d 100644
> > > > > > > > > > > --- a/drivers/usb/dwc3/core.c
> > > > > > > > > > > +++ b/drivers/usb/dwc3/core.c
> > > > > > > > > > > @@ -115,6 +115,25 @@ static int
> > > > > > > > > > > dwc3_core_soft_reset(struct dwc3 *dwc)
> > > > > > > > > > > }
> > > > > > > > > > >
> > > > > > > > > > > /**
> > > > > > > > > > > + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface
> > > > > > > > > > > for NL
> > > > > > > > > > > + * @dwc: Pointer to our controller context structure
> > > > > > > > > > > + */
> > > > > > > > > > > +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
> > > > > > > > > > > +{
> > > > > > > > > > > + u32 reg = 0;
> > > > > > > > > > > +
> > > > > > > > > > > + reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK |
> > > > > > > > > > > DWC3_GUSB3PIPECTL_UX_EXITINPX
> > > > > > > > > > > + | DWC3_GUSB3PIPECTL_UX_EXITINPX |
> > > > > > > > > > > DWC3_GUSB3PIPECTL_U1U2EXITFAIL
> > > > > > > > > > > + | DWC3_GUSB3PIPECTL_DEPOCHANGE |
> > > > > > > > > > > DWC3_GUSB3PIPECTL_RX_DETOPOLL;
> > > > > > > > > >
> > > > > > > > > > UX_EXITINPX is supposed to be used with a faulty PHY, I
> > > > > > > > > > need to see an
> > > > > > > > > > erratum before I can accept it. You have also duplicated
> > > > > > > > > > the bit in this
> > > > > > > > > > initialization.
> > > > > > > > > >
> > > > > > > > > > U1U2EXITFAIL -> also a workaround bit and I need to see an
> > > > > > > > > > erratum.
> > > > > > > > > >
> > > > > > > > > > RX_DETOPOLL -> it seems like it's safe to leave this one
> > > > > > > > > > out as it can
> > > > > > > > > > only be proven to work with this bit after going through
> > > > > > > > > > full USB
> > > > > > > > > > certification.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > What do you mean of the faulty PHY?
> > > > > > > > > We would use AMD PHY to talk with DWC3 controller.
> > > > > > > >
> > > > > > > > Look at the description of each of those bits and you'll see it
> > > > > > > > mentions
> > > > > > > > they're supposed to be used for workarounds. Here's
> > > > > > > > UX_EXIT_IN_PX, as an
> > > > > > > > example:
> > > > > > > >
> > > > > > > > "
> > > > > > > > This bit is added for SS PHY workaround where SS PHY ...
> > > > > > > > "
> > > > > > > >
> > > > > > >
> > > > > > > Got it, so faulty PHY you meant that some special PHYs didn't not
> > > > > > > meet
> > > > > > > the standards someplace.
> > > > > > >
> > > > > > > For simulation board, we used vendor provided PHY. But on SOC, we
> > > > > > > will use AMD PHY. I will re-check again to confirm which
> > > > > > > workarounds
> > > > > > > need on simulation board and which workarounds need on SOC.
> > > > > >
> > > > > > Thanks, that's great. I wonder if there's a way to detect that we're
> > > > > > running on FPGA. Can you check with your HW designers ? I'll go dig
> > > > > > on
> > > > > > Synopsys databook myself too on Monday.
> > > > > >
> > > > >
> > > > > I checked with HW designers, they can update the origin value of GUID
> > > > > register of FPGA to add ASCII codes as prefix of "fpga". I can checked
> > > > > it before driver re-writes it as kernel version (I see you latest
> > > > > testing branch have this behavior).
> > > >
> > > > while that's a nice idea, it wouldn't work for everybody; only AMD.
> > > >
> > > > If there's no "generic" way implemented by Synopsys into the RTL, then
> > > > we better not add anything.
> > >
> > > Because the RTL is frozen.
> > >
> > > I checked the spec again of GUID:
> > >
> > > 1) To store the version or revision of your system
> > > 2) To store hardware configurations that are outside the core
> > > 3) As a scratch register.
> > >
> > > As 2) described, it also makes sence the store the HW configuration (FPGA
> > > version) on this register.
> > >
> > > Can we split the 32bit space to two 16bit area, one of them stores the HW
> > > configuration, and the other stores the kernel version? I think other SOC
> > > (especially x86-based) would use this method. :)
> > >
> > > It looks like there isn't another register can program into the version
> > > info.
> >
> > the problem is that we won't have something that will work for
> > everybody. Note that the register can be used as scratch register as
> > well and there's really no way we will be able to get every RTL designer
> > in every company out there who's licensing this core to agree on using
> > this register the exact same way.
> >
> > Unless it ships already built into the RTL Syno
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
On Mon, Sep 29, 2014 at 11:28:57AM +0300, Heikki Krogerus wrote:
> > > > A question, the dwc3 controller is the PCI-E device in my platform,
> > > > but the class code of PCI header is 0x0c0330, the same with xHC.
> > > > That's because it need to meet the windows enviroment. The dwc3
> > > > controller acted as only host mode to bind with windows xhci driver.
> > > > But on linux, sometimes, we auto-bind with xhci driver as well (class
> > > > code 0x0c0330) despite we use Pid/Vid on dwc3 driver. Then I need
> > > > rmmod xhci_hcd module and modprobe dwc3_pci module manually.
> > > >
> > > > Any idea to resolve this issue?
>
> Declare a fixup quirk. I'm not a pci expert, but I think something
> like this should work..
>
> static void dwc3_fix_amd_nl_class(struct pci_dev *pdev)
> {
> pdev->class = 0x0c03fe;
> }
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_NL,
> dwc3_fix_amd_nl_class);
>
Heikki, your info inspires me. :)
I looked at pci driver, this update should put in pci/quirks.c.
Because the behavior of the global pci_fixups_header array is
maintained at that file.
When system inits, pci driver would load fixup devices before driver
attached. So it should define the fixup header under pci/quirks.c.
After do some tests in my side. This quirk works well.
Thanks,
Rui
--
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: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
On Mon, Sep 29, 2014 at 11:48:41PM -0500, Felipe Balbi wrote:
> Hi,
>
> On Tue, Sep 30, 2014 at 11:12:55AM +0800, Huang Rui wrote:
> > > > > > > > > On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> > > > > > > > > > diff --git a/drivers/usb/dwc3/core.c
> > > > > > > > > > b/drivers/usb/dwc3/core.c
> > > > > > > > > > index b0f4d52..6138c5d 100644
> > > > > > > > > > --- a/drivers/usb/dwc3/core.c
> > > > > > > > > > +++ b/drivers/usb/dwc3/core.c
> > > > > > > > > > @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct
> > > > > > > > > > dwc3 *dwc)
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > /**
> > > > > > > > > > + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface
> > > > > > > > > > for NL
> > > > > > > > > > + * @dwc: Pointer to our controller context structure
> > > > > > > > > > + */
> > > > > > > > > > +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
> > > > > > > > > > +{
> > > > > > > > > > + u32 reg = 0;
> > > > > > > > > > +
> > > > > > > > > > + reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK |
> > > > > > > > > > DWC3_GUSB3PIPECTL_UX_EXITINPX
> > > > > > > > > > + | DWC3_GUSB3PIPECTL_UX_EXITINPX |
> > > > > > > > > > DWC3_GUSB3PIPECTL_U1U2EXITFAIL
> > > > > > > > > > + | DWC3_GUSB3PIPECTL_DEPOCHANGE |
> > > > > > > > > > DWC3_GUSB3PIPECTL_RX_DETOPOLL;
> > > > > > > > >
> > > > > > > > > UX_EXITINPX is supposed to be used with a faulty PHY, I need
> > > > > > > > > to see an
> > > > > > > > > erratum before I can accept it. You have also duplicated the
> > > > > > > > > bit in this
> > > > > > > > > initialization.
> > > > > > > > >
> > > > > > > > > U1U2EXITFAIL -> also a workaround bit and I need to see an
> > > > > > > > > erratum.
> > > > > > > > >
> > > > > > > > > RX_DETOPOLL -> it seems like it's safe to leave this one out
> > > > > > > > > as it can
> > > > > > > > > only be proven to work with this bit after going through full
> > > > > > > > > USB
> > > > > > > > > certification.
> > > > > > > > >
> > > > > > > >
> > > > > > > > What do you mean of the faulty PHY?
> > > > > > > > We would use AMD PHY to talk with DWC3 controller.
> > > > > > >
> > > > > > > Look at the description of each of those bits and you'll see it
> > > > > > > mentions
> > > > > > > they're supposed to be used for workarounds. Here's
> > > > > > > UX_EXIT_IN_PX, as an
> > > > > > > example:
> > > > > > >
> > > > > > > "
> > > > > > > This bit is added for SS PHY workaround where SS PHY ...
> > > > > > > "
> > > > > > >
> > > > > >
> > > > > > Got it, so faulty PHY you meant that some special PHYs didn't not
> > > > > > meet
> > > > > > the standards someplace.
> > > > > >
> > > > > > For simulation board, we used vendor provided PHY. But on SOC, we
> > > > > > will use AMD PHY. I will re-check again to confirm which workarounds
> > > > > > need on simulation board and which workarounds need on SOC.
> > > > >
> > > > > Thanks, that's great. I wonder if there's a way to detect that we're
> > > > > running on FPGA. Can you check with your HW designers ? I'll go dig on
> > > > > Synopsys databook myself too on Monday.
> > > > >
> > > >
> > > > I checked with HW designers, they can update the origin value of GUID
> > > > register of FPGA to add ASCII codes as prefix of "fpga". I can checked
> > > > it before driver re-writes it as kernel version (I see you latest
> > > > testing branch have this behavior).
> > >
> > > while that's a nice idea, it wouldn't work for everybody; only AMD.
> > >
> > > If there's no "generic" way implemented by Synopsys into the RTL, then
> > > we better not add anything.
> >
> > Because the RTL is frozen.
> >
> > I checked the spec again of GUID:
> >
> > 1) To store the version or revision of your system
> > 2) To store hardware configurations that are outside the core
> > 3) As a scratch register.
> >
> > As 2) described, it also makes sence the store the HW configuration (FPGA
> > version) on this register.
> >
> > Can we split the 32bit space to two 16bit area, one of them stores the HW
> > configuration, and the other stores the kernel version? I think other SOC
> > (especially x86-based) would use this method. :)
> >
> > It looks like there isn't another register can program into the version
> > info.
>
> the problem is that we won't have something that will work for
> everybody. Note that the register can be used as scratch register as
> well and there's really no way we will be able to get every RTL designer
> in every company out there who's licensing this core to agree on using
> this register the exact same way.
>
> Unless it ships already built into the RTL Synopsys delivers, there's
> no way we can use it in the core dwc3 driver ;-)
>
> > > > > > > Also, I'd have to ask you to provide full boot logs of your
> > > > > > > platform
> > > > > > > booting with my testing/next (where all the latest patches are)
> > > > > > > plus
> > > > > > > your patches.
> > > > > >
>
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
Hi,
On Tue, Sep 30, 2014 at 11:12:55AM +0800, Huang Rui wrote:
> > > > > > > > On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> > > > > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > > > > > > index b0f4d52..6138c5d 100644
> > > > > > > > > --- a/drivers/usb/dwc3/core.c
> > > > > > > > > +++ b/drivers/usb/dwc3/core.c
> > > > > > > > > @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct
> > > > > > > > > dwc3 *dwc)
> > > > > > > > > }
> > > > > > > > >
> > > > > > > > > /**
> > > > > > > > > + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for
> > > > > > > > > NL
> > > > > > > > > + * @dwc: Pointer to our controller context structure
> > > > > > > > > + */
> > > > > > > > > +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
> > > > > > > > > +{
> > > > > > > > > + u32 reg = 0;
> > > > > > > > > +
> > > > > > > > > + reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK |
> > > > > > > > > DWC3_GUSB3PIPECTL_UX_EXITINPX
> > > > > > > > > + | DWC3_GUSB3PIPECTL_UX_EXITINPX |
> > > > > > > > > DWC3_GUSB3PIPECTL_U1U2EXITFAIL
> > > > > > > > > + | DWC3_GUSB3PIPECTL_DEPOCHANGE |
> > > > > > > > > DWC3_GUSB3PIPECTL_RX_DETOPOLL;
> > > > > > > >
> > > > > > > > UX_EXITINPX is supposed to be used with a faulty PHY, I need to
> > > > > > > > see an
> > > > > > > > erratum before I can accept it. You have also duplicated the
> > > > > > > > bit in this
> > > > > > > > initialization.
> > > > > > > >
> > > > > > > > U1U2EXITFAIL -> also a workaround bit and I need to see an
> > > > > > > > erratum.
> > > > > > > >
> > > > > > > > RX_DETOPOLL -> it seems like it's safe to leave this one out as
> > > > > > > > it can
> > > > > > > > only be proven to work with this bit after going through full
> > > > > > > > USB
> > > > > > > > certification.
> > > > > > > >
> > > > > > >
> > > > > > > What do you mean of the faulty PHY?
> > > > > > > We would use AMD PHY to talk with DWC3 controller.
> > > > > >
> > > > > > Look at the description of each of those bits and you'll see it
> > > > > > mentions
> > > > > > they're supposed to be used for workarounds. Here's UX_EXIT_IN_PX,
> > > > > > as an
> > > > > > example:
> > > > > >
> > > > > > "
> > > > > > This bit is added for SS PHY workaround where SS PHY ...
> > > > > > "
> > > > > >
> > > > >
> > > > > Got it, so faulty PHY you meant that some special PHYs didn't not meet
> > > > > the standards someplace.
> > > > >
> > > > > For simulation board, we used vendor provided PHY. But on SOC, we
> > > > > will use AMD PHY. I will re-check again to confirm which workarounds
> > > > > need on simulation board and which workarounds need on SOC.
> > > >
> > > > Thanks, that's great. I wonder if there's a way to detect that we're
> > > > running on FPGA. Can you check with your HW designers ? I'll go dig on
> > > > Synopsys databook myself too on Monday.
> > > >
> > >
> > > I checked with HW designers, they can update the origin value of GUID
> > > register of FPGA to add ASCII codes as prefix of "fpga". I can checked
> > > it before driver re-writes it as kernel version (I see you latest
> > > testing branch have this behavior).
> >
> > while that's a nice idea, it wouldn't work for everybody; only AMD.
> >
> > If there's no "generic" way implemented by Synopsys into the RTL, then
> > we better not add anything.
>
> Because the RTL is frozen.
>
> I checked the spec again of GUID:
>
> 1) To store the version or revision of your system
> 2) To store hardware configurations that are outside the core
> 3) As a scratch register.
>
> As 2) described, it also makes sence the store the HW configuration (FPGA
> version) on this register.
>
> Can we split the 32bit space to two 16bit area, one of them stores the HW
> configuration, and the other stores the kernel version? I think other SOC
> (especially x86-based) would use this method. :)
>
> It looks like there isn't another register can program into the version info.
the problem is that we won't have something that will work for
everybody. Note that the register can be used as scratch register as
well and there's really no way we will be able to get every RTL designer
in every company out there who's licensing this core to agree on using
this register the exact same way.
Unless it ships already built into the RTL Synopsys delivers, there's
no way we can use it in the core dwc3 driver ;-)
> > > > > > Also, I'd have to ask you to provide full boot logs of your platform
> > > > > > booting with my testing/next (where all the latest patches are) plus
> > > > > > your patches.
> > > > >
> > > > > I will provide the boot logs to you. Actually, I already did the
> > > > > gadget mass storage and gadget zero testing with my patches under 3.14
> > > > > before.
> > > >
> > > > v3.14 is rather old, if you're sending patches upstream. I'd rather see
> > > > patches tested with either next or latest Linus' tag. My testing/ne
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
On Mon, Sep 29, 2014 at 09:15:13AM -0500, Felipe Balbi wrote:
> Hi,
>
> On Mon, Sep 29, 2014 at 05:38:32PM +0800, Huang Rui wrote:
> > > > > > > On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> > > > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > > > > > index b0f4d52..6138c5d 100644
> > > > > > > > --- a/drivers/usb/dwc3/core.c
> > > > > > > > +++ b/drivers/usb/dwc3/core.c
> > > > > > > > @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct
> > > > > > > > dwc3 *dwc)
> > > > > > > > }
> > > > > > > >
> > > > > > > > /**
> > > > > > > > + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for NL
> > > > > > > > + * @dwc: Pointer to our controller context structure
> > > > > > > > + */
> > > > > > > > +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
> > > > > > > > +{
> > > > > > > > + u32 reg = 0;
> > > > > > > > +
> > > > > > > > + reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK |
> > > > > > > > DWC3_GUSB3PIPECTL_UX_EXITINPX
> > > > > > > > + | DWC3_GUSB3PIPECTL_UX_EXITINPX |
> > > > > > > > DWC3_GUSB3PIPECTL_U1U2EXITFAIL
> > > > > > > > + | DWC3_GUSB3PIPECTL_DEPOCHANGE |
> > > > > > > > DWC3_GUSB3PIPECTL_RX_DETOPOLL;
> > > > > > >
> > > > > > > UX_EXITINPX is supposed to be used with a faulty PHY, I need to
> > > > > > > see an
> > > > > > > erratum before I can accept it. You have also duplicated the bit
> > > > > > > in this
> > > > > > > initialization.
> > > > > > >
> > > > > > > U1U2EXITFAIL -> also a workaround bit and I need to see an
> > > > > > > erratum.
> > > > > > >
> > > > > > > RX_DETOPOLL -> it seems like it's safe to leave this one out as
> > > > > > > it can
> > > > > > > only be proven to work with this bit after going through full USB
> > > > > > > certification.
> > > > > > >
> > > > > >
> > > > > > What do you mean of the faulty PHY?
> > > > > > We would use AMD PHY to talk with DWC3 controller.
> > > > >
> > > > > Look at the description of each of those bits and you'll see it
> > > > > mentions
> > > > > they're supposed to be used for workarounds. Here's UX_EXIT_IN_PX, as
> > > > > an
> > > > > example:
> > > > >
> > > > > "
> > > > > This bit is added for SS PHY workaround where SS PHY ...
> > > > > "
> > > > >
> > > >
> > > > Got it, so faulty PHY you meant that some special PHYs didn't not meet
> > > > the standards someplace.
> > > >
> > > > For simulation board, we used vendor provided PHY. But on SOC, we
> > > > will use AMD PHY. I will re-check again to confirm which workarounds
> > > > need on simulation board and which workarounds need on SOC.
> > >
> > > Thanks, that's great. I wonder if there's a way to detect that we're
> > > running on FPGA. Can you check with your HW designers ? I'll go dig on
> > > Synopsys databook myself too on Monday.
> > >
> >
> > I checked with HW designers, they can update the origin value of GUID
> > register of FPGA to add ASCII codes as prefix of "fpga". I can checked
> > it before driver re-writes it as kernel version (I see you latest
> > testing branch have this behavior).
>
> while that's a nice idea, it wouldn't work for everybody; only AMD.
>
> If there's no "generic" way implemented by Synopsys into the RTL, then
> we better not add anything.
>
Because the RTL is frozen.
I checked the spec again of GUID:
1) To store the version or revision of your system
2) To store hardware configurations that are outside the core
3) As a scratch register.
As 2) described, it also makes sence the store the HW configuration (FPGA
version) on this register.
Can we split the 32bit space to two 16bit area, one of them stores the HW
configuration, and the other stores the kernel version? I think other SOC
(especially x86-based) would use this method. :)
It looks like there isn't another register can program into the version info.
> > > > > It's alright that AMD PHY needs this bit, but then, let's get
> > > > > confirmation from IP/SoC/SilVal team and add a proper comment stating
> > > > > why we need them. This is so we don't forget that $version of AMD's
> > > > > PHY
> > > > > needs workarounds for A, B, and C silicon errata.
> > > > >
> > > >
> > > > Yes, but currently, I needn't write AMD own phy driver. There isn't
> > > > any requirement from HW side to program the phy register. So I used
> > > > NOP USB transceiver driver till now.
> > >
> > > NOP is a perfectly valid use-case :-) We still want to know that version
> > > x of AMD's PHY is quirky on these or that case :-)
> > >
> >
> > I can use the SMBus revsion ID for different chips version of amd. You
> > can refer usb/host/pci-quirks.c, I already added the different chip
> > version macros there for xHC. If PHY version updates, the chip version
> > must update too.
>
> Please provide a patch and let's discuss :-)
>
> > > > > Also, I'd have to ask you to provide full boot logs of your platform
> > > > > booting with my testing/next (where all
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
Hi,
On Mon, Sep 29, 2014 at 05:59:42PM +, Paul Zimmerman wrote:
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 68497b3..112352e 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -1577,6 +1577,13 @@ static int dwc3_gadget_start(struct usb_gadget *g,
> > }
> > dwc3_writel(dwc->regs, DWC3_DCFG, reg);
> >
> > + if (dwc->has_lpm_erratum) {
> > + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> > + /* REVISIT should this be configurable ? */
> > + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
> > + dwc3_writel(dwc->regs, DWC3_DCTL, reg);
> > + }
>
> Yes, I think this really wants to be configurable. The value used is
> supposed to depend on the latencies in the system etc.
alright, in that case we need to pass that through DT/pdata as well.
--
balbi
signature.asc
Description: Digital signature
RE: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Friday, September 26, 2014 9:31 PM
>
> On Sat, Sep 27, 2014 at 01:05:46AM +, Paul Zimmerman wrote:
> >
> > Well, it's called LPM Errata because the feature was added to the USB
> > spec as an erratum. It's not an erratum to our controller. But I don't
> > have any strong feelings about how the driver support is implemented.
>
> how about adding a "has_lpm_erratum" to struct dwc3 which gets
> initialized either through pdata or DT ? Then above WARN() would become:
>
> WARN(dwc->revision < DWC3_REVISION_240A && dwc->has_lpm_erratum",
> "LPM Erratum not available on dwc3 revisisions < 2.40a\n");
>
> Then we're not really calling it a quirk. In fact Huang, when respining
> your series, let's convert your quirk bits into single-bit fields inside
> struct dwc3 and struct platform_data. Like so:
>
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 4d4e854..e233ce1 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -695,11 +695,13 @@ static int dwc3_probe(struct platform_device *pdev)
>
> if (node) {
> dwc->maximum_speed = of_usb_get_maximum_speed(node);
> + dwc->has_lpm_erratum = of_property_read_bool(node,
> "snps,has-lpm-erratum");
>
> dwc->needs_fifo_resize = of_property_read_bool(node,
> "tx-fifo-resize");
> dwc->dr_mode = of_usb_get_dr_mode(node);
> } else if (pdata) {
> dwc->maximum_speed = pdata->maximum_speed;
> + dwc->has_lpm_erratum = pdata->has_lpm_erratum;
>
> dwc->needs_fifo_resize = pdata->tx_fifo_resize;
> dwc->dr_mode = pdata->dr_mode;
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
> index 66f6256..23e1504 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -661,6 +661,8 @@ struct dwc3_scratchpad_array {
> * @ep0_bounced: true when we used bounce buffer
> * @ep0_expect_in: true when we expect a DATA IN transfer
> * @has_hibernation: true when dwc3 was configured with Hibernation
> + * @has_lpm_erratum: true when core was configured with LPM Erratum. Note
> that
> + * there's now way for software to detect this in runtime.
> * @is_selfpowered: true when we are selfpowered
> * @needs_fifo_resize: not all users might want fifo resizing, flag it
> * @pullups_connected: true when Run/Stop bit is set
> @@ -764,6 +766,7 @@ struct dwc3 {
> unsignedep0_bounced:1;
> unsignedep0_expect_in:1;
> unsignedhas_hibernation:1;
> + unsignedhas_lpm_erratum:1;
> unsignedis_selfpowered:1;
> unsignedneeds_fifo_resize:1;
> unsignedpullups_connected:1;
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 68497b3..112352e 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -1577,6 +1577,13 @@ static int dwc3_gadget_start(struct usb_gadget *g,
> }
> dwc3_writel(dwc->regs, DWC3_DCFG, reg);
>
> + if (dwc->has_lpm_erratum) {
> + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> + /* REVISIT should this be configurable ? */
> + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
> + dwc3_writel(dwc->regs, DWC3_DCTL, reg);
> + }
Yes, I think this really wants to be configurable. The value used is
supposed to depend on the latencies in the system etc.
--
Paul
--
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: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
Hi,
On Mon, Sep 29, 2014 at 05:38:32PM +0800, Huang Rui wrote:
> > > > > > On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> > > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > > > > index b0f4d52..6138c5d 100644
> > > > > > > --- a/drivers/usb/dwc3/core.c
> > > > > > > +++ b/drivers/usb/dwc3/core.c
> > > > > > > @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3
> > > > > > > *dwc)
> > > > > > > }
> > > > > > >
> > > > > > > /**
> > > > > > > + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for NL
> > > > > > > + * @dwc: Pointer to our controller context structure
> > > > > > > + */
> > > > > > > +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
> > > > > > > +{
> > > > > > > + u32 reg = 0;
> > > > > > > +
> > > > > > > + reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK |
> > > > > > > DWC3_GUSB3PIPECTL_UX_EXITINPX
> > > > > > > + | DWC3_GUSB3PIPECTL_UX_EXITINPX |
> > > > > > > DWC3_GUSB3PIPECTL_U1U2EXITFAIL
> > > > > > > + | DWC3_GUSB3PIPECTL_DEPOCHANGE |
> > > > > > > DWC3_GUSB3PIPECTL_RX_DETOPOLL;
> > > > > >
> > > > > > UX_EXITINPX is supposed to be used with a faulty PHY, I need to see
> > > > > > an
> > > > > > erratum before I can accept it. You have also duplicated the bit in
> > > > > > this
> > > > > > initialization.
> > > > > >
> > > > > > U1U2EXITFAIL -> also a workaround bit and I need to see an erratum.
> > > > > >
> > > > > > RX_DETOPOLL -> it seems like it's safe to leave this one out as it
> > > > > > can
> > > > > > only be proven to work with this bit after going through full USB
> > > > > > certification.
> > > > > >
> > > > >
> > > > > What do you mean of the faulty PHY?
> > > > > We would use AMD PHY to talk with DWC3 controller.
> > > >
> > > > Look at the description of each of those bits and you'll see it mentions
> > > > they're supposed to be used for workarounds. Here's UX_EXIT_IN_PX, as an
> > > > example:
> > > >
> > > > "
> > > > This bit is added for SS PHY workaround where SS PHY ...
> > > > "
> > > >
> > >
> > > Got it, so faulty PHY you meant that some special PHYs didn't not meet
> > > the standards someplace.
> > >
> > > For simulation board, we used vendor provided PHY. But on SOC, we
> > > will use AMD PHY. I will re-check again to confirm which workarounds
> > > need on simulation board and which workarounds need on SOC.
> >
> > Thanks, that's great. I wonder if there's a way to detect that we're
> > running on FPGA. Can you check with your HW designers ? I'll go dig on
> > Synopsys databook myself too on Monday.
> >
>
> I checked with HW designers, they can update the origin value of GUID
> register of FPGA to add ASCII codes as prefix of "fpga". I can checked
> it before driver re-writes it as kernel version (I see you latest
> testing branch have this behavior).
while that's a nice idea, it wouldn't work for everybody; only AMD.
If there's no "generic" way implemented by Synopsys into the RTL, then
we better not add anything.
> > > > It's alright that AMD PHY needs this bit, but then, let's get
> > > > confirmation from IP/SoC/SilVal team and add a proper comment stating
> > > > why we need them. This is so we don't forget that $version of AMD's PHY
> > > > needs workarounds for A, B, and C silicon errata.
> > > >
> > >
> > > Yes, but currently, I needn't write AMD own phy driver. There isn't
> > > any requirement from HW side to program the phy register. So I used
> > > NOP USB transceiver driver till now.
> >
> > NOP is a perfectly valid use-case :-) We still want to know that version
> > x of AMD's PHY is quirky on these or that case :-)
> >
>
> I can use the SMBus revsion ID for different chips version of amd. You
> can refer usb/host/pci-quirks.c, I already added the different chip
> version macros there for xHC. If PHY version updates, the chip version
> must update too.
Please provide a patch and let's discuss :-)
> > > > Also, I'd have to ask you to provide full boot logs of your platform
> > > > booting with my testing/next (where all the latest patches are) plus
> > > > your patches.
> > >
> > > I will provide the boot logs to you. Actually, I already did the
> > > gadget mass storage and gadget zero testing with my patches under 3.14
> > > before.
> >
> > v3.14 is rather old, if you're sending patches upstream. I'd rather see
> > patches tested with either next or latest Linus' tag. My testing/next
> > branch is also usually fine too.
> >
>
> Yes, I am doing these testing.
>
> An issue meet your msc.c.
>
> ray@hr-slim:~/felipe/usb-tools$ gcc -Wall -O2 -D_GNU_SOURCE
> -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -g -o msc msc.c
> /tmp/ccUBcDC4.o: In function `do_write':
> /home/ray/felipe/usb-tools/msc.c:275: undefined reference to `clock_gettime'
> /home/ray/felipe/usb-tools/msc.c:308: undefined reference to `clock_gettime'
> /tmp/ccUBcDC4.o: In function `do_read':
> /home/ray/felipe/usb-tools/msc.c:332: undefined
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
On Sun, Sep 28, 2014 at 06:41:39PM -0500, Felipe Balbi wrote:
> Hi,
>
> On Sun, Sep 28, 2014 at 11:11:23AM +0800, Huang Rui wrote:
> > On Fri, Sep 26, 2014 at 09:35:21AM -0500, Felipe Balbi wrote:
> > > On Fri, Sep 26, 2014 at 04:50:26PM +0800, Huang Rui wrote:
> > > > On Thu, Sep 25, 2014 at 09:50:32AM -0500, Felipe Balbi wrote:
> > > > > Hi,
> > > > >
> > > > > On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> > > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > > > index b0f4d52..6138c5d 100644
> > > > > > --- a/drivers/usb/dwc3/core.c
> > > > > > +++ b/drivers/usb/dwc3/core.c
> > > > > > @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3
> > > > > > *dwc)
> > > > > > }
> > > > > >
> > > > > > /**
> > > > > > + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for NL
> > > > > > + * @dwc: Pointer to our controller context structure
> > > > > > + */
> > > > > > +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
> > > > > > +{
> > > > > > + u32 reg = 0;
> > > > > > +
> > > > > > + reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK |
> > > > > > DWC3_GUSB3PIPECTL_UX_EXITINPX
> > > > > > + | DWC3_GUSB3PIPECTL_UX_EXITINPX |
> > > > > > DWC3_GUSB3PIPECTL_U1U2EXITFAIL
> > > > > > + | DWC3_GUSB3PIPECTL_DEPOCHANGE |
> > > > > > DWC3_GUSB3PIPECTL_RX_DETOPOLL;
> > > > >
> > > > > UX_EXITINPX is supposed to be used with a faulty PHY, I need to see an
> > > > > erratum before I can accept it. You have also duplicated the bit in
> > > > > this
> > > > > initialization.
> > > > >
> > > > > U1U2EXITFAIL -> also a workaround bit and I need to see an erratum.
> > > > >
> > > > > RX_DETOPOLL -> it seems like it's safe to leave this one out as it can
> > > > > only be proven to work with this bit after going through full USB
> > > > > certification.
> > > > >
> > > >
> > > > What do you mean of the faulty PHY?
> > > > We would use AMD PHY to talk with DWC3 controller.
> > >
> > > Look at the description of each of those bits and you'll see it mentions
> > > they're supposed to be used for workarounds. Here's UX_EXIT_IN_PX, as an
> > > example:
> > >
> > > "
> > > This bit is added for SS PHY workaround where SS PHY ...
> > > "
> > >
> >
> > Got it, so faulty PHY you meant that some special PHYs didn't not meet
> > the standards someplace.
> >
> > For simulation board, we used vendor provided PHY. But on SOC, we
> > will use AMD PHY. I will re-check again to confirm which workarounds
> > need on simulation board and which workarounds need on SOC.
>
> Thanks, that's great. I wonder if there's a way to detect that we're
> running on FPGA. Can you check with your HW designers ? I'll go dig on
> Synopsys databook myself too on Monday.
>
I checked with HW designers, they can update the origin value of GUID
register of FPGA to add ASCII codes as prefix of "fpga". I can checked
it before driver re-writes it as kernel version (I see you latest
testing branch have this behavior).
> > > It's alright that AMD PHY needs this bit, but then, let's get
> > > confirmation from IP/SoC/SilVal team and add a proper comment stating
> > > why we need them. This is so we don't forget that $version of AMD's PHY
> > > needs workarounds for A, B, and C silicon errata.
> > >
> >
> > Yes, but currently, I needn't write AMD own phy driver. There isn't
> > any requirement from HW side to program the phy register. So I used
> > NOP USB transceiver driver till now.
>
> NOP is a perfectly valid use-case :-) We still want to know that version
> x of AMD's PHY is quirky on these or that case :-)
>
I can use the SMBus revsion ID for different chips version of amd. You
can refer usb/host/pci-quirks.c, I already added the different chip
version macros there for xHC. If PHY version updates, the chip version
must update too.
> > > Also, I'd have to ask you to provide full boot logs of your platform
> > > booting with my testing/next (where all the latest patches are) plus
> > > your patches.
> >
> > I will provide the boot logs to you. Actually, I already did the
> > gadget mass storage and gadget zero testing with my patches under 3.14
> > before.
>
> v3.14 is rather old, if you're sending patches upstream. I'd rather see
> patches tested with either next or latest Linus' tag. My testing/next
> branch is also usually fine too.
>
Yes, I am doing these testing.
An issue meet your msc.c.
ray@hr-slim:~/felipe/usb-tools$ gcc -Wall -O2 -D_GNU_SOURCE
-D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -g -o msc msc.c
/tmp/ccUBcDC4.o: In function `do_write':
/home/ray/felipe/usb-tools/msc.c:275: undefined reference to `clock_gettime'
/home/ray/felipe/usb-tools/msc.c:308: undefined reference to `clock_gettime'
/tmp/ccUBcDC4.o: In function `do_read':
/home/ray/felipe/usb-tools/msc.c:332: undefined reference to `clock_gettime'
/home/ray/felipe/usb-tools/msc.c:349: undefined reference to `clock_gettime'
/tmp/ccUBcDC4.o: In function `do_writev':
/home/ray/felipe/usb
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
> > > A question, the dwc3 controller is the PCI-E device in my platform,
> > > but the class code of PCI header is 0x0c0330, the same with xHC.
> > > That's because it need to meet the windows enviroment. The dwc3
> > > controller acted as only host mode to bind with windows xhci driver.
> > > But on linux, sometimes, we auto-bind with xhci driver as well (class
> > > code 0x0c0330) despite we use Pid/Vid on dwc3 driver. Then I need
> > > rmmod xhci_hcd module and modprobe dwc3_pci module manually.
> > >
> > > Any idea to resolve this issue?
Declare a fixup quirk. I'm not a pci expert, but I think something
like this should work..
static void dwc3_fix_amd_nl_class(struct pci_dev *pdev)
{
pdev->class = 0x0c03fe;
}
DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_AMD, PCI_DEVICE_ID_AMD_NL,
dwc3_fix_amd_nl_class);
> > Heikki, can you confirm if your DWC3 incarnations present this same
> > "feature" ? :-)
The DWC3 is not given xHCI class code on our boards.
Cheers,
--
heikki
--
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: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
On Sun, Sep 28, 2014 at 06:46:14PM -0500, Felipe Balbi wrote:
> Hi,
>
> On Sun, Sep 28, 2014 at 05:18:58PM +0800, Huang Rui wrote:
> > > > > From: Felipe Balbi [mailto:ba...@ti.com]
> > > > > Sent: Friday, September 26, 2014 5:54 PM
> > > > >
> > > > > On Fri, Sep 26, 2014 at 11:18:48PM +, Paul Zimmerman wrote:
> > > > > > > From: Felipe Balbi [mailto:ba...@ti.com]
> > > > > > > Sent: Friday, September 26, 2014 2:40 PM
> > > > > > >
> > > > > > > On Fri, Sep 26, 2014 at 08:57:19PM +, Paul Zimmerman wrote:
> > > > > > > > > > diff --git a/drivers/usb/dwc3/gadget.c
> > > > > > > > > > b/drivers/usb/dwc3/gadget.c
> > > > > > > > > > index 0fcc0a3..8277065 100644
> > > > > > > > > > --- a/drivers/usb/dwc3/gadget.c
> > > > > > > > > > +++ b/drivers/usb/dwc3/gadget.c
> > > > > > > > > > @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int
> > > > > > > > > > irq, void *_dwc)
> > > > > > > > > > */
> > > > > > > > > > int dwc3_gadget_init(struct dwc3 *dwc)
> > > > > > > > > > {
> > > > > > > > > > + u32 reg;
> > > > > > > > > > int ret;
> > > > > > > > > >
> > > > > > > > > > dwc->ctrl_req = dma_alloc_coherent(dwc->dev,
> > > > > > > > > > sizeof(*dwc->ctrl_req),
> > > > > > > > > > @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3
> > > > > > > > > > *dwc)
> > > > > > > > > > if (ret)
> > > > > > > > > > goto err4;
> > > > > > > > > >
> > > > > > > > > > + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> > > > > > > > > > + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> > > > > > > > > > + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
> > > > > > > > >
> > > > > > > > > weird, why would Synopsys put this here ? It seems like this
> > > > > > > > > is only
> > > > > > > > > useful when LPM Errata is enabled and that's, apparently, a
> > > > > > > > > host-side
> > > > > > > > > thing.
> > > > > > > > >
> > > > > > > > > Paul, can you comment ?
> > > > > > > >
> > > > > > > > These bits contribute to how the device responds to an LPM
> > > > > > > > transaction
> > > > > > > > from the host. If DCFG.LPMCap=1, they set the BESL value above
> > > > > > > > which
> > > > > > > > the core will send a NYET. If DCFG.LPMCap=0, they have no
> > > > > > > > effect. So
> > > > > > > > it's definitely a device-side thing, but only if the core is
> > > > > > > > configured
> > > > > > > > with LPM Errata support enabled.
> > > > > > >
> > > > > > > right, and how can SW detect if LPM Errata was enabled ? From the
> > > > > > > host
> > > > > > > point of view, we can check bit 20 of xHCI capability register.
> > > > > > > What
> > > > > > > about device ? I can't seem to find anything :-s
> > > > > >
> > > > > > I just talked to one of our RTL designers. You're right, there is no
> > > > > > way to tell from the Device registers alone whether the controller
> > > > > > is
> > > > > > configured with LPM Errata support or not.
> > > > > >
> > > > > > You can tell for sure if it is *not* enabled, by checking GSNPSID,
> > > > > > and
> > > > > > if the version is earlier than 2.40a, the feature wasn't available.
> > > > >
> > > > > alright, we can use this for something like:
> > > > >
> > > > > WARN(rev < 2.40a && (flags & LPM_ERRATA ||
> > > > > of_property_read_bool("lpm-errata")),
> > > > > "LPM Errata not available on dwc3 revisions <= 2.40a\n");
> > > > >
> > > > > > So for Device-mode only controllers, I guess you will need a DT
> > > > > > property
> > > > > > for this.
> > > > >
> > > > > right, DT property and platform_data feature flag, or something. I
> > > > > don't
> > > > > wanna call it a quirk though, although it _is_ an erratum WRT LPM
> > > > > support. Dunno. Any strong feelings ?
> > > >
> > > > Well, it's called LPM Errata because the feature was added to the USB
> > > > spec as an erratum. It's not an erratum to our controller. But I don't
> > > > have any strong feelings about how the driver support is implemented.
> > >
> > > how about adding a "has_lpm_erratum" to struct dwc3 which gets
> > > initialized either through pdata or DT ? Then above WARN() would become:
> > >
> > > WARN(dwc->revision < DWC3_REVISION_240A && dwc->has_lpm_erratum",
> > > "LPM Erratum not available on dwc3 revisisions < 2.40a\n");
> > >
> > > Then we're not really calling it a quirk. In fact Huang, when respining
> > > your series, let's convert your quirk bits into single-bit fields inside
> > > struct dwc3 and struct platform_data. Like so:
> > >
> >
> > Looks like a good suggestion.
> >
> > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > index 4d4e854..e233ce1 100644
> > > --- a/drivers/usb/dwc3/core.c
> > > +++ b/drivers/usb/dwc3/core.c
> > > @@ -695,11 +695,13 @@ static int dwc3_probe(struct platform_device *pdev)
> > >
> > > if (node) {
> > > dwc->maximum_speed = of_usb_get_maximum_speed(node);
> > > + dwc->has_lpm_erratum =
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
Hi,
On Sun, Sep 28, 2014 at 05:18:58PM +0800, Huang Rui wrote:
> > > > From: Felipe Balbi [mailto:ba...@ti.com]
> > > > Sent: Friday, September 26, 2014 5:54 PM
> > > >
> > > > On Fri, Sep 26, 2014 at 11:18:48PM +, Paul Zimmerman wrote:
> > > > > > From: Felipe Balbi [mailto:ba...@ti.com]
> > > > > > Sent: Friday, September 26, 2014 2:40 PM
> > > > > >
> > > > > > On Fri, Sep 26, 2014 at 08:57:19PM +, Paul Zimmerman wrote:
> > > > > > > > > diff --git a/drivers/usb/dwc3/gadget.c
> > > > > > > > > b/drivers/usb/dwc3/gadget.c
> > > > > > > > > index 0fcc0a3..8277065 100644
> > > > > > > > > --- a/drivers/usb/dwc3/gadget.c
> > > > > > > > > +++ b/drivers/usb/dwc3/gadget.c
> > > > > > > > > @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int
> > > > > > > > > irq, void *_dwc)
> > > > > > > > > */
> > > > > > > > > int dwc3_gadget_init(struct dwc3 *dwc)
> > > > > > > > > {
> > > > > > > > > + u32 reg;
> > > > > > > > > int ret;
> > > > > > > > >
> > > > > > > > > dwc->ctrl_req = dma_alloc_coherent(dwc->dev,
> > > > > > > > > sizeof(*dwc->ctrl_req),
> > > > > > > > > @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
> > > > > > > > > if (ret)
> > > > > > > > > goto err4;
> > > > > > > > >
> > > > > > > > > + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> > > > > > > > > + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> > > > > > > > > + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
> > > > > > > >
> > > > > > > > weird, why would Synopsys put this here ? It seems like this is
> > > > > > > > only
> > > > > > > > useful when LPM Errata is enabled and that's, apparently, a
> > > > > > > > host-side
> > > > > > > > thing.
> > > > > > > >
> > > > > > > > Paul, can you comment ?
> > > > > > >
> > > > > > > These bits contribute to how the device responds to an LPM
> > > > > > > transaction
> > > > > > > from the host. If DCFG.LPMCap=1, they set the BESL value above
> > > > > > > which
> > > > > > > the core will send a NYET. If DCFG.LPMCap=0, they have no effect.
> > > > > > > So
> > > > > > > it's definitely a device-side thing, but only if the core is
> > > > > > > configured
> > > > > > > with LPM Errata support enabled.
> > > > > >
> > > > > > right, and how can SW detect if LPM Errata was enabled ? From the
> > > > > > host
> > > > > > point of view, we can check bit 20 of xHCI capability register. What
> > > > > > about device ? I can't seem to find anything :-s
> > > > >
> > > > > I just talked to one of our RTL designers. You're right, there is no
> > > > > way to tell from the Device registers alone whether the controller is
> > > > > configured with LPM Errata support or not.
> > > > >
> > > > > You can tell for sure if it is *not* enabled, by checking GSNPSID, and
> > > > > if the version is earlier than 2.40a, the feature wasn't available.
> > > >
> > > > alright, we can use this for something like:
> > > >
> > > > WARN(rev < 2.40a && (flags & LPM_ERRATA ||
> > > > of_property_read_bool("lpm-errata")),
> > > > "LPM Errata not available on dwc3 revisions <= 2.40a\n");
> > > >
> > > > > So for Device-mode only controllers, I guess you will need a DT
> > > > > property
> > > > > for this.
> > > >
> > > > right, DT property and platform_data feature flag, or something. I don't
> > > > wanna call it a quirk though, although it _is_ an erratum WRT LPM
> > > > support. Dunno. Any strong feelings ?
> > >
> > > Well, it's called LPM Errata because the feature was added to the USB
> > > spec as an erratum. It's not an erratum to our controller. But I don't
> > > have any strong feelings about how the driver support is implemented.
> >
> > how about adding a "has_lpm_erratum" to struct dwc3 which gets
> > initialized either through pdata or DT ? Then above WARN() would become:
> >
> > WARN(dwc->revision < DWC3_REVISION_240A && dwc->has_lpm_erratum",
> > "LPM Erratum not available on dwc3 revisisions < 2.40a\n");
> >
> > Then we're not really calling it a quirk. In fact Huang, when respining
> > your series, let's convert your quirk bits into single-bit fields inside
> > struct dwc3 and struct platform_data. Like so:
> >
>
> Looks like a good suggestion.
>
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index 4d4e854..e233ce1 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -695,11 +695,13 @@ static int dwc3_probe(struct platform_device *pdev)
> >
> > if (node) {
> > dwc->maximum_speed = of_usb_get_maximum_speed(node);
> > + dwc->has_lpm_erratum = of_property_read_bool(node,
> > "snps,has-lpm-erratum");
> >
> > dwc->needs_fifo_resize = of_property_read_bool(node,
> > "tx-fifo-resize");
> > dwc->dr_mode = of_usb_get_dr_mode(node);
> > } else if (pdata) {
> > dwc->maximum_speed = pdata->maximum_speed;
> > +
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
Hi,
On Sun, Sep 28, 2014 at 11:11:23AM +0800, Huang Rui wrote:
> On Fri, Sep 26, 2014 at 09:35:21AM -0500, Felipe Balbi wrote:
> > On Fri, Sep 26, 2014 at 04:50:26PM +0800, Huang Rui wrote:
> > > On Thu, Sep 25, 2014 at 09:50:32AM -0500, Felipe Balbi wrote:
> > > > Hi,
> > > >
> > > > On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > > index b0f4d52..6138c5d 100644
> > > > > --- a/drivers/usb/dwc3/core.c
> > > > > +++ b/drivers/usb/dwc3/core.c
> > > > > @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
> > > > > }
> > > > >
> > > > > /**
> > > > > + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for NL
> > > > > + * @dwc: Pointer to our controller context structure
> > > > > + */
> > > > > +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
> > > > > +{
> > > > > + u32 reg = 0;
> > > > > +
> > > > > + reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK |
> > > > > DWC3_GUSB3PIPECTL_UX_EXITINPX
> > > > > + | DWC3_GUSB3PIPECTL_UX_EXITINPX |
> > > > > DWC3_GUSB3PIPECTL_U1U2EXITFAIL
> > > > > + | DWC3_GUSB3PIPECTL_DEPOCHANGE |
> > > > > DWC3_GUSB3PIPECTL_RX_DETOPOLL;
> > > >
> > > > UX_EXITINPX is supposed to be used with a faulty PHY, I need to see an
> > > > erratum before I can accept it. You have also duplicated the bit in this
> > > > initialization.
> > > >
> > > > U1U2EXITFAIL -> also a workaround bit and I need to see an erratum.
> > > >
> > > > RX_DETOPOLL -> it seems like it's safe to leave this one out as it can
> > > > only be proven to work with this bit after going through full USB
> > > > certification.
> > > >
> > >
> > > What do you mean of the faulty PHY?
> > > We would use AMD PHY to talk with DWC3 controller.
> >
> > Look at the description of each of those bits and you'll see it mentions
> > they're supposed to be used for workarounds. Here's UX_EXIT_IN_PX, as an
> > example:
> >
> > "
> > This bit is added for SS PHY workaround where SS PHY ...
> > "
> >
>
> Got it, so faulty PHY you meant that some special PHYs didn't not meet
> the standards someplace.
>
> For simulation board, we used vendor provided PHY. But on SOC, we
> will use AMD PHY. I will re-check again to confirm which workarounds
> need on simulation board and which workarounds need on SOC.
Thanks, that's great. I wonder if there's a way to detect that we're
running on FPGA. Can you check with your HW designers ? I'll go dig on
Synopsys databook myself too on Monday.
> > It's alright that AMD PHY needs this bit, but then, let's get
> > confirmation from IP/SoC/SilVal team and add a proper comment stating
> > why we need them. This is so we don't forget that $version of AMD's PHY
> > needs workarounds for A, B, and C silicon errata.
> >
>
> Yes, but currently, I needn't write AMD own phy driver. There isn't
> any requirement from HW side to program the phy register. So I used
> NOP USB transceiver driver till now.
NOP is a perfectly valid use-case :-) We still want to know that version
x of AMD's PHY is quirky on these or that case :-)
> > Also, I'd have to ask you to provide full boot logs of your platform
> > booting with my testing/next (where all the latest patches are) plus
> > your patches.
>
> I will provide the boot logs to you. Actually, I already did the
> gadget mass storage and gadget zero testing with my patches under 3.14
> before.
v3.14 is rather old, if you're sending patches upstream. I'd rather see
patches tested with either next or latest Linus' tag. My testing/next
branch is also usually fine too.
If you want, you can definitely defer a v2 until v3.18-rc1 is tagged.
I'll send a few fixes I have pending when that happens too. All my
latest fixes are on my testing/next branch, btw. I'll add Cc: stable to
most of them, but you might want to cherry-pick a few that I don't to
your vendor tree, if you have it.
> > Then, load g_mass_storage with a backing storage of your
> > choice and run my msc.c/msc.sh tools which you can get from [1] and [2];
> > post the logs for that too. Last, but not least, please USB30CV (chapter
> > 9 and Link Layer test, at least) just so we know there isn't anything
> > new with your version of the core, which I suppose is 2.80a, based on
> > the LPM Errata bits.
> >
>
> OK, will post the logs to you.
thanks :-)
> > This is just because I don't have access to the HW myself, so I can't
> > verify your patches. One thing I can tell you, with my testing/next,
> > dwc3 is really stable. I have every test passing except for Halt
> > Endpoint which I'm debugging right now.
> >
>
> OK, I got it. Will rebase my patches to testing/next.
that'll help me, thanks
> Felipe, it's pleasure to leverage your dwc3 ip driver on AMD platform
> for me. Thanks to support. :)
hey, don't mention it. I'm happy to have several users on a single
driver. Everybody can benefit from fixes ;-)
--
balbi
signature.asc
Descript
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
On Fri, Sep 26, 2014 at 11:30:44PM -0500, Felipe Balbi wrote:
> Hi,
>
> On Sat, Sep 27, 2014 at 01:05:46AM +, Paul Zimmerman wrote:
> > > From: Felipe Balbi [mailto:ba...@ti.com]
> > > Sent: Friday, September 26, 2014 5:54 PM
> > >
> > > On Fri, Sep 26, 2014 at 11:18:48PM +, Paul Zimmerman wrote:
> > > > > From: Felipe Balbi [mailto:ba...@ti.com]
> > > > > Sent: Friday, September 26, 2014 2:40 PM
> > > > >
> > > > > On Fri, Sep 26, 2014 at 08:57:19PM +, Paul Zimmerman wrote:
> > > > > > > > diff --git a/drivers/usb/dwc3/gadget.c
> > > > > > > > b/drivers/usb/dwc3/gadget.c
> > > > > > > > index 0fcc0a3..8277065 100644
> > > > > > > > --- a/drivers/usb/dwc3/gadget.c
> > > > > > > > +++ b/drivers/usb/dwc3/gadget.c
> > > > > > > > @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int
> > > > > > > > irq, void *_dwc)
> > > > > > > > */
> > > > > > > > int dwc3_gadget_init(struct dwc3 *dwc)
> > > > > > > > {
> > > > > > > > + u32 reg;
> > > > > > > > int ret;
> > > > > > > >
> > > > > > > > dwc->ctrl_req = dma_alloc_coherent(dwc->dev,
> > > > > > > > sizeof(*dwc->ctrl_req),
> > > > > > > > @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
> > > > > > > > if (ret)
> > > > > > > > goto err4;
> > > > > > > >
> > > > > > > > + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> > > > > > > > + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> > > > > > > > + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
> > > > > > >
> > > > > > > weird, why would Synopsys put this here ? It seems like this is
> > > > > > > only
> > > > > > > useful when LPM Errata is enabled and that's, apparently, a
> > > > > > > host-side
> > > > > > > thing.
> > > > > > >
> > > > > > > Paul, can you comment ?
> > > > > >
> > > > > > These bits contribute to how the device responds to an LPM
> > > > > > transaction
> > > > > > from the host. If DCFG.LPMCap=1, they set the BESL value above which
> > > > > > the core will send a NYET. If DCFG.LPMCap=0, they have no effect. So
> > > > > > it's definitely a device-side thing, but only if the core is
> > > > > > configured
> > > > > > with LPM Errata support enabled.
> > > > >
> > > > > right, and how can SW detect if LPM Errata was enabled ? From the host
> > > > > point of view, we can check bit 20 of xHCI capability register. What
> > > > > about device ? I can't seem to find anything :-s
> > > >
> > > > I just talked to one of our RTL designers. You're right, there is no
> > > > way to tell from the Device registers alone whether the controller is
> > > > configured with LPM Errata support or not.
> > > >
> > > > You can tell for sure if it is *not* enabled, by checking GSNPSID, and
> > > > if the version is earlier than 2.40a, the feature wasn't available.
> > >
> > > alright, we can use this for something like:
> > >
> > > WARN(rev < 2.40a && (flags & LPM_ERRATA ||
> > > of_property_read_bool("lpm-errata")),
> > > "LPM Errata not available on dwc3 revisions <= 2.40a\n");
> > >
> > > > So for Device-mode only controllers, I guess you will need a DT property
> > > > for this.
> > >
> > > right, DT property and platform_data feature flag, or something. I don't
> > > wanna call it a quirk though, although it _is_ an erratum WRT LPM
> > > support. Dunno. Any strong feelings ?
> >
> > Well, it's called LPM Errata because the feature was added to the USB
> > spec as an erratum. It's not an erratum to our controller. But I don't
> > have any strong feelings about how the driver support is implemented.
>
> how about adding a "has_lpm_erratum" to struct dwc3 which gets
> initialized either through pdata or DT ? Then above WARN() would become:
>
> WARN(dwc->revision < DWC3_REVISION_240A && dwc->has_lpm_erratum",
> "LPM Erratum not available on dwc3 revisisions < 2.40a\n");
>
> Then we're not really calling it a quirk. In fact Huang, when respining
> your series, let's convert your quirk bits into single-bit fields inside
> struct dwc3 and struct platform_data. Like so:
>
Looks like a good suggestion.
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index 4d4e854..e233ce1 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -695,11 +695,13 @@ static int dwc3_probe(struct platform_device *pdev)
>
> if (node) {
> dwc->maximum_speed = of_usb_get_maximum_speed(node);
> + dwc->has_lpm_erratum = of_property_read_bool(node,
> "snps,has-lpm-erratum");
>
> dwc->needs_fifo_resize = of_property_read_bool(node,
> "tx-fifo-resize");
> dwc->dr_mode = of_usb_get_dr_mode(node);
> } else if (pdata) {
> dwc->maximum_speed = pdata->maximum_speed;
> + dwc->has_lpm_erratum = pdata->has_lpm_erratum; >
> dwc->needs_fifo_resize = pdata->tx_fifo_resize;
> dwc->dr_mode = pdata->dr_mode;
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
On Fri, Sep 26, 2014 at 09:35:21AM -0500, Felipe Balbi wrote: > On Fri, Sep 26, 2014 at 04:50:26PM +0800, Huang Rui wrote: > > On Thu, Sep 25, 2014 at 09:50:32AM -0500, Felipe Balbi wrote: > > > Hi, > > > > > > On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote: > > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > > > > index b0f4d52..6138c5d 100644 > > > > --- a/drivers/usb/dwc3/core.c > > > > +++ b/drivers/usb/dwc3/core.c > > > > @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc) > > > > } > > > > > > > > + reg |= DWC3_GCTL_GBLHIBERNATIONEN; > > > > > > looks like this should always be set for all versions of the core > > > configured with hibernation. > > > > > > > yes, amd nl chip will support hibernation. > > in that case, we do this: > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > index 4d4e854..584dcde 100644 > --- a/drivers/usb/dwc3/core.c > +++ b/drivers/usb/dwc3/core.c > @@ -449,6 +449,7 @@ static int dwc3_core_init(struct dwc3 *dwc) > case DWC3_GHWPARAMS1_EN_PWROPT_HIB: > /* enable hibernation here */ > dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4); > + reg |= DWC3_GCTL_GBLHIBERNATIONEN; > break; > default: > dev_dbg(dwc->dev, "No power optimization available\n"); > > that'll work for everybody with hibernation enabled in coreConsultant. > Right, I will do it like this on V2. Thanks, Rui -- 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: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
On Fri, Sep 26, 2014 at 09:35:21AM -0500, Felipe Balbi wrote:
> On Fri, Sep 26, 2014 at 04:50:26PM +0800, Huang Rui wrote:
> > On Thu, Sep 25, 2014 at 09:50:32AM -0500, Felipe Balbi wrote:
> > > Hi,
> > >
> > > On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> > > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > > index b0f4d52..6138c5d 100644
> > > > --- a/drivers/usb/dwc3/core.c
> > > > +++ b/drivers/usb/dwc3/core.c
> > > > @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
> > > > }
> > > >
> > > > /**
> > > > + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for NL
> > > > + * @dwc: Pointer to our controller context structure
> > > > + */
> > > > +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
> > > > +{
> > > > + u32 reg = 0;
> > > > +
> > > > + reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK |
> > > > DWC3_GUSB3PIPECTL_UX_EXITINPX
> > > > + | DWC3_GUSB3PIPECTL_UX_EXITINPX |
> > > > DWC3_GUSB3PIPECTL_U1U2EXITFAIL
> > > > + | DWC3_GUSB3PIPECTL_DEPOCHANGE |
> > > > DWC3_GUSB3PIPECTL_RX_DETOPOLL;
> > >
> > > UX_EXITINPX is supposed to be used with a faulty PHY, I need to see an
> > > erratum before I can accept it. You have also duplicated the bit in this
> > > initialization.
> > >
> > > U1U2EXITFAIL -> also a workaround bit and I need to see an erratum.
> > >
> > > RX_DETOPOLL -> it seems like it's safe to leave this one out as it can
> > > only be proven to work with this bit after going through full USB
> > > certification.
> > >
> >
> > What do you mean of the faulty PHY?
> > We would use AMD PHY to talk with DWC3 controller.
>
> Look at the description of each of those bits and you'll see it mentions
> they're supposed to be used for workarounds. Here's UX_EXIT_IN_PX, as an
> example:
>
> "
> This bit is added for SS PHY workaround where SS PHY ...
> "
>
Got it, so faulty PHY you meant that some special PHYs didn't not meet
the standards someplace.
For simulation board, we used vendor provided PHY. But on SOC, we
will use AMD PHY. I will re-check again to confirm which workarounds
need on simulation board and which workarounds need on SOC.
> It's alright that AMD PHY needs this bit, but then, let's get
> confirmation from IP/SoC/SilVal team and add a proper comment stating
> why we need them. This is so we don't forget that $version of AMD's PHY
> needs workarounds for A, B, and C silicon errata.
>
Yes, but currently, I needn't write AMD own phy driver. There isn't
any requirement from HW side to program the phy register. So I used
NOP USB transceiver driver till now.
> Also, I'd have to ask you to provide full boot logs of your platform
> booting with my testing/next (where all the latest patches are) plus
> your patches.
I will provide the boot logs to you. Actually, I already did the
gadget mass storage and gadget zero testing with my patches under 3.14
before.
> Then, load g_mass_storage with a backing storage of your
> choice and run my msc.c/msc.sh tools which you can get from [1] and [2];
> post the logs for that too. Last, but not least, please USB30CV (chapter
> 9 and Link Layer test, at least) just so we know there isn't anything
> new with your version of the core, which I suppose is 2.80a, based on
> the LPM Errata bits.
>
OK, will post the logs to you.
> This is just because I don't have access to the HW myself, so I can't
> verify your patches. One thing I can tell you, with my testing/next,
> dwc3 is really stable. I have every test passing except for Halt
> Endpoint which I'm debugging right now.
>
OK, I got it. Will rebase my patches to testing/next.
> > > > + reg |= DWC3_GUSB3PIPECTL_DEP1P2P3(1) |
> > > > DWC3_GUSB3PIPECTL_TX_DEEPH(1);
> > >
> > > DEP1P2P3 and DEP0CHANGE seem to be required only for faulty PHYs too, I
> > > need to see an erratum.
> > >
> > > > + dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
> > > > +
> > > > + mdelay(100);
> > > > +}
> > > > +
> > > > +/**
> > > > * dwc3_free_one_event_buffer - Frees one event buffer
> > > > * @dwc: Pointer to our controller context structure
> > > > * @evt: Pointer to event buffer to be freed
> > > > @@ -412,9 +431,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
> > > > if (ret)
> > > > goto err0;
> > > >
> > > > + if (dwc->quirks & DWC3_AMD_NL_PLAT)
> > > > + dwc3_core_nl_set_pipe3(dwc);
> > > > +
> > > > reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> > > > +
> > > > reg &= ~DWC3_GCTL_SCALEDOWN_MASK;
> > > > - reg &= ~DWC3_GCTL_DISSCRAMBLE;
> > > > +
> > > > + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> > > > + reg |= DWC3_GCTL_DISSCRAMBLE;
> > >
> > > you're disabling scrambling ? What about EMI ? Why doesn't your device
> > > work with scrambling ? I need to see an erratum before I can accept
> > > this.
> > >
> >
> > Sorry, disabling scrambling is workaround for debugg
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
Hi,
On Sat, Sep 27, 2014 at 01:05:46AM +, Paul Zimmerman wrote:
> > From: Felipe Balbi [mailto:ba...@ti.com]
> > Sent: Friday, September 26, 2014 5:54 PM
> >
> > On Fri, Sep 26, 2014 at 11:18:48PM +, Paul Zimmerman wrote:
> > > > From: Felipe Balbi [mailto:ba...@ti.com]
> > > > Sent: Friday, September 26, 2014 2:40 PM
> > > >
> > > > On Fri, Sep 26, 2014 at 08:57:19PM +, Paul Zimmerman wrote:
> > > > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > > > > > index 0fcc0a3..8277065 100644
> > > > > > > --- a/drivers/usb/dwc3/gadget.c
> > > > > > > +++ b/drivers/usb/dwc3/gadget.c
> > > > > > > @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq,
> > > > > > > void *_dwc)
> > > > > > > */
> > > > > > > int dwc3_gadget_init(struct dwc3 *dwc)
> > > > > > > {
> > > > > > > + u32 reg;
> > > > > > > int ret;
> > > > > > >
> > > > > > > dwc->ctrl_req = dma_alloc_coherent(dwc->dev,
> > > > > > > sizeof(*dwc->ctrl_req),
> > > > > > > @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
> > > > > > > if (ret)
> > > > > > > goto err4;
> > > > > > >
> > > > > > > + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> > > > > > > + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> > > > > > > + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
> > > > > >
> > > > > > weird, why would Synopsys put this here ? It seems like this is only
> > > > > > useful when LPM Errata is enabled and that's, apparently, a
> > > > > > host-side
> > > > > > thing.
> > > > > >
> > > > > > Paul, can you comment ?
> > > > >
> > > > > These bits contribute to how the device responds to an LPM transaction
> > > > > from the host. If DCFG.LPMCap=1, they set the BESL value above which
> > > > > the core will send a NYET. If DCFG.LPMCap=0, they have no effect. So
> > > > > it's definitely a device-side thing, but only if the core is
> > > > > configured
> > > > > with LPM Errata support enabled.
> > > >
> > > > right, and how can SW detect if LPM Errata was enabled ? From the host
> > > > point of view, we can check bit 20 of xHCI capability register. What
> > > > about device ? I can't seem to find anything :-s
> > >
> > > I just talked to one of our RTL designers. You're right, there is no
> > > way to tell from the Device registers alone whether the controller is
> > > configured with LPM Errata support or not.
> > >
> > > You can tell for sure if it is *not* enabled, by checking GSNPSID, and
> > > if the version is earlier than 2.40a, the feature wasn't available.
> >
> > alright, we can use this for something like:
> >
> > WARN(rev < 2.40a && (flags & LPM_ERRATA ||
> > of_property_read_bool("lpm-errata")),
> > "LPM Errata not available on dwc3 revisions <= 2.40a\n");
> >
> > > So for Device-mode only controllers, I guess you will need a DT property
> > > for this.
> >
> > right, DT property and platform_data feature flag, or something. I don't
> > wanna call it a quirk though, although it _is_ an erratum WRT LPM
> > support. Dunno. Any strong feelings ?
>
> Well, it's called LPM Errata because the feature was added to the USB
> spec as an erratum. It's not an erratum to our controller. But I don't
> have any strong feelings about how the driver support is implemented.
how about adding a "has_lpm_erratum" to struct dwc3 which gets
initialized either through pdata or DT ? Then above WARN() would become:
WARN(dwc->revision < DWC3_REVISION_240A && dwc->has_lpm_erratum",
"LPM Erratum not available on dwc3 revisisions < 2.40a\n");
Then we're not really calling it a quirk. In fact Huang, when respining
your series, let's convert your quirk bits into single-bit fields inside
struct dwc3 and struct platform_data. Like so:
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 4d4e854..e233ce1 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -695,11 +695,13 @@ static int dwc3_probe(struct platform_device *pdev)
if (node) {
dwc->maximum_speed = of_usb_get_maximum_speed(node);
+ dwc->has_lpm_erratum = of_property_read_bool(node,
"snps,has-lpm-erratum");
dwc->needs_fifo_resize = of_property_read_bool(node,
"tx-fifo-resize");
dwc->dr_mode = of_usb_get_dr_mode(node);
} else if (pdata) {
dwc->maximum_speed = pdata->maximum_speed;
+ dwc->has_lpm_erratum = pdata->has_lpm_erratum;
dwc->needs_fifo_resize = pdata->tx_fifo_resize;
dwc->dr_mode = pdata->dr_mode;
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 66f6256..23e1504 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -661,6 +661,8 @@ struct dwc3_scratchpad_array {
* @ep0_bounced: true when we used bounce buffer
* @ep0_expect_in: true when we expect a DATA IN transfer
* @has_hibernation: true when dwc3 was configured with Hib
RE: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Friday, September 26, 2014 5:54 PM
>
> On Fri, Sep 26, 2014 at 11:18:48PM +, Paul Zimmerman wrote:
> > > From: Felipe Balbi [mailto:ba...@ti.com]
> > > Sent: Friday, September 26, 2014 2:40 PM
> > >
> > > On Fri, Sep 26, 2014 at 08:57:19PM +, Paul Zimmerman wrote:
> > > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > > > > index 0fcc0a3..8277065 100644
> > > > > > --- a/drivers/usb/dwc3/gadget.c
> > > > > > +++ b/drivers/usb/dwc3/gadget.c
> > > > > > @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq,
> > > > > > void *_dwc)
> > > > > > */
> > > > > > int dwc3_gadget_init(struct dwc3 *dwc)
> > > > > > {
> > > > > > + u32 reg;
> > > > > > int ret;
> > > > > >
> > > > > > dwc->ctrl_req = dma_alloc_coherent(dwc->dev,
> > > > > > sizeof(*dwc->ctrl_req),
> > > > > > @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
> > > > > > if (ret)
> > > > > > goto err4;
> > > > > >
> > > > > > + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> > > > > > + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> > > > > > + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
> > > > >
> > > > > weird, why would Synopsys put this here ? It seems like this is only
> > > > > useful when LPM Errata is enabled and that's, apparently, a host-side
> > > > > thing.
> > > > >
> > > > > Paul, can you comment ?
> > > >
> > > > These bits contribute to how the device responds to an LPM transaction
> > > > from the host. If DCFG.LPMCap=1, they set the BESL value above which
> > > > the core will send a NYET. If DCFG.LPMCap=0, they have no effect. So
> > > > it's definitely a device-side thing, but only if the core is configured
> > > > with LPM Errata support enabled.
> > >
> > > right, and how can SW detect if LPM Errata was enabled ? From the host
> > > point of view, we can check bit 20 of xHCI capability register. What
> > > about device ? I can't seem to find anything :-s
> >
> > I just talked to one of our RTL designers. You're right, there is no
> > way to tell from the Device registers alone whether the controller is
> > configured with LPM Errata support or not.
> >
> > You can tell for sure if it is *not* enabled, by checking GSNPSID, and
> > if the version is earlier than 2.40a, the feature wasn't available.
>
> alright, we can use this for something like:
>
> WARN(rev < 2.40a && (flags & LPM_ERRATA ||
> of_property_read_bool("lpm-errata")),
> "LPM Errata not available on dwc3 revisions <= 2.40a\n");
>
> > So for Device-mode only controllers, I guess you will need a DT property
> > for this.
>
> right, DT property and platform_data feature flag, or something. I don't
> wanna call it a quirk though, although it _is_ an erratum WRT LPM
> support. Dunno. Any strong feelings ?
Well, it's called LPM Errata because the feature was added to the USB
spec as an erratum. It's not an erratum to our controller. But I don't
have any strong feelings about how the driver support is implemented.
--
Paul
--
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: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
Hi,
On Fri, Sep 26, 2014 at 11:18:48PM +, Paul Zimmerman wrote:
> > From: Felipe Balbi [mailto:ba...@ti.com]
> > Sent: Friday, September 26, 2014 2:40 PM
> >
> > On Fri, Sep 26, 2014 at 08:57:19PM +, Paul Zimmerman wrote:
> > > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > > > index 0fcc0a3..8277065 100644
> > > > > --- a/drivers/usb/dwc3/gadget.c
> > > > > +++ b/drivers/usb/dwc3/gadget.c
> > > > > @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq, void
> > > > > *_dwc)
> > > > > */
> > > > > int dwc3_gadget_init(struct dwc3 *dwc)
> > > > > {
> > > > > + u32 reg;
> > > > > int ret;
> > > > >
> > > > > dwc->ctrl_req = dma_alloc_coherent(dwc->dev,
> > > > > sizeof(*dwc->ctrl_req),
> > > > > @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
> > > > > if (ret)
> > > > > goto err4;
> > > > >
> > > > > + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> > > > > + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> > > > > + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
> > > >
> > > > weird, why would Synopsys put this here ? It seems like this is only
> > > > useful when LPM Errata is enabled and that's, apparently, a host-side
> > > > thing.
> > > >
> > > > Paul, can you comment ?
> > >
> > > These bits contribute to how the device responds to an LPM transaction
> > > from the host. If DCFG.LPMCap=1, they set the BESL value above which
> > > the core will send a NYET. If DCFG.LPMCap=0, they have no effect. So
> > > it's definitely a device-side thing, but only if the core is configured
> > > with LPM Errata support enabled.
> >
> > right, and how can SW detect if LPM Errata was enabled ? From the host
> > point of view, we can check bit 20 of xHCI capability register. What
> > about device ? I can't seem to find anything :-s
>
> I just talked to one of our RTL designers. You're right, there is no
> way to tell from the Device registers alone whether the controller is
> configured with LPM Errata support or not.
>
> You can tell for sure if it is *not* enabled, by checking GSNPSID, and
> if the version is earlier than 2.40a, the feature wasn't available.
alright, we can use this for something like:
WARN(rev < 2.40a && (flags & LPM_ERRATA || of_property_read_bool("lpm-errata")),
"LPM Errata not available on dwc3 revisions <= 2.40a\n");
> So for Device-mode only controllers, I guess you will need a DT property
> for this.
right, DT property and platform_data feature flag, or something. I don't
wanna call it a quirk though, although it _is_ an erratum WRT LPM
support. Dunno. Any strong feelings ?
--
balbi
signature.asc
Description: Digital signature
RE: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
> From: Felipe Balbi [mailto:ba...@ti.com]
> Sent: Friday, September 26, 2014 2:40 PM
>
> On Fri, Sep 26, 2014 at 08:57:19PM +, Paul Zimmerman wrote:
> > > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > > index 0fcc0a3..8277065 100644
> > > > --- a/drivers/usb/dwc3/gadget.c
> > > > +++ b/drivers/usb/dwc3/gadget.c
> > > > @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq, void
> > > > *_dwc)
> > > > */
> > > > int dwc3_gadget_init(struct dwc3 *dwc)
> > > > {
> > > > + u32 reg;
> > > > int ret;
> > > >
> > > > dwc->ctrl_req = dma_alloc_coherent(dwc->dev,
> > > > sizeof(*dwc->ctrl_req),
> > > > @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
> > > > if (ret)
> > > > goto err4;
> > > >
> > > > + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> > > > + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> > > > + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
> > >
> > > weird, why would Synopsys put this here ? It seems like this is only
> > > useful when LPM Errata is enabled and that's, apparently, a host-side
> > > thing.
> > >
> > > Paul, can you comment ?
> >
> > These bits contribute to how the device responds to an LPM transaction
> > from the host. If DCFG.LPMCap=1, they set the BESL value above which
> > the core will send a NYET. If DCFG.LPMCap=0, they have no effect. So
> > it's definitely a device-side thing, but only if the core is configured
> > with LPM Errata support enabled.
>
> right, and how can SW detect if LPM Errata was enabled ? From the host
> point of view, we can check bit 20 of xHCI capability register. What
> about device ? I can't seem to find anything :-s
I just talked to one of our RTL designers. You're right, there is no
way to tell from the Device registers alone whether the controller is
configured with LPM Errata support or not.
You can tell for sure if it is *not* enabled, by checking GSNPSID, and
if the version is earlier than 2.40a, the feature wasn't available.
So for Device-mode only controllers, I guess you will need a DT property
for this.
--
Paul
--
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: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
Hi,
On Fri, Sep 26, 2014 at 08:57:19PM +, Paul Zimmerman wrote:
> > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > index 0fcc0a3..8277065 100644
> > > --- a/drivers/usb/dwc3/gadget.c
> > > +++ b/drivers/usb/dwc3/gadget.c
> > > @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq, void
> > > *_dwc)
> > > */
> > > int dwc3_gadget_init(struct dwc3 *dwc)
> > > {
> > > + u32 reg;
> > > int ret;
> > >
> > > dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req),
> > > @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
> > > if (ret)
> > > goto err4;
> > >
> > > + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> > > + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> > > + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
> >
> > weird, why would Synopsys put this here ? It seems like this is only
> > useful when LPM Errata is enabled and that's, apparently, a host-side
> > thing.
> >
> > Paul, can you comment ?
>
> These bits contribute to how the device responds to an LPM transaction
> from the host. If DCFG.LPMCap=1, they set the BESL value above which
> the core will send a NYET. If DCFG.LPMCap=0, they have no effect. So
> it's definitely a device-side thing, but only if the core is configured
> with LPM Errata support enabled.
right, and how can SW detect if LPM Errata was enabled ? From the host
point of view, we can check bit 20 of xHCI capability register. What
about device ? I can't seem to find anything :-s
cheers
--
balbi
signature.asc
Description: Digital signature
RE: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb-ow...@vger.kernel.org]
On Behalf Of Felipe Balbi
Sent: Thursday, September 25, 2014 7:51 AM
> On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > index 0fcc0a3..8277065 100644
> > --- a/drivers/usb/dwc3/gadget.c
> > +++ b/drivers/usb/dwc3/gadget.c
> > @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq, void *_dwc)
> > */
> > int dwc3_gadget_init(struct dwc3 *dwc)
> > {
> > + u32 reg;
> > int ret;
> >
> > dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req),
> > @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
> > if (ret)
> > goto err4;
> >
> > + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> > + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> > + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
>
> weird, why would Synopsys put this here ? It seems like this is only
> useful when LPM Errata is enabled and that's, apparently, a host-side
> thing.
>
> Paul, can you comment ?
These bits contribute to how the device responds to an LPM transaction
from the host. If DCFG.LPMCap=1, they set the BESL value above which
the core will send a NYET. If DCFG.LPMCap=0, they have no effect. So
it's definitely a device-side thing, but only if the core is configured
with LPM Errata support enabled.
--
Paul
--
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: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
Hi, On Fri, Sep 26, 2014 at 09:35:21AM -0500, Felipe Balbi wrote: > This is just because I don't have access to the HW myself, so I can't > verify your patches. One thing I can tell you, with my testing/next, > dwc3 is really stable. I have every test passing except for Halt > Endpoint which I'm debugging right now. alright, found the problem. I'll push to my testing/next in a bit. -- balbi signature.asc Description: Digital signature
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
On Fri, Sep 26, 2014 at 04:50:26PM +0800, Huang Rui wrote:
> On Thu, Sep 25, 2014 at 09:50:32AM -0500, Felipe Balbi wrote:
> > Hi,
> >
> > On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> > > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > > index b0f4d52..6138c5d 100644
> > > --- a/drivers/usb/dwc3/core.c
> > > +++ b/drivers/usb/dwc3/core.c
> > > @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
> > > }
> > >
> > > /**
> > > + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for NL
> > > + * @dwc: Pointer to our controller context structure
> > > + */
> > > +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
> > > +{
> > > + u32 reg = 0;
> > > +
> > > + reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK | DWC3_GUSB3PIPECTL_UX_EXITINPX
> > > + | DWC3_GUSB3PIPECTL_UX_EXITINPX | DWC3_GUSB3PIPECTL_U1U2EXITFAIL
> > > + | DWC3_GUSB3PIPECTL_DEPOCHANGE | DWC3_GUSB3PIPECTL_RX_DETOPOLL;
> >
> > UX_EXITINPX is supposed to be used with a faulty PHY, I need to see an
> > erratum before I can accept it. You have also duplicated the bit in this
> > initialization.
> >
> > U1U2EXITFAIL -> also a workaround bit and I need to see an erratum.
> >
> > RX_DETOPOLL -> it seems like it's safe to leave this one out as it can
> > only be proven to work with this bit after going through full USB
> > certification.
> >
>
> What do you mean of the faulty PHY?
> We would use AMD PHY to talk with DWC3 controller.
Look at the description of each of those bits and you'll see it mentions
they're supposed to be used for workarounds. Here's UX_EXIT_IN_PX, as an
example:
"
This bit is added for SS PHY workaround where SS PHY ...
"
It's alright that AMD PHY needs this bit, but then, let's get
confirmation from IP/SoC/SilVal team and add a proper comment stating
why we need them. This is so we don't forget that $version of AMD's PHY
needs workarounds for A, B, and C silicon errata.
Also, I'd have to ask you to provide full boot logs of your platform
booting with my testing/next (where all the latest patches are) plus
your patches. Then, load g_mass_storage with a backing storage of your
choice and run my msc.c/msc.sh tools which you can get from [1] and [2];
post the logs for that too. Last, but not least, please USB30CV (chapter
9 and Link Layer test, at least) just so we know there isn't anything
new with your version of the core, which I suppose is 2.80a, based on
the LPM Errata bits.
This is just because I don't have access to the HW myself, so I can't
verify your patches. One thing I can tell you, with my testing/next,
dwc3 is really stable. I have every test passing except for Halt
Endpoint which I'm debugging right now.
> > > + reg |= DWC3_GUSB3PIPECTL_DEP1P2P3(1) | DWC3_GUSB3PIPECTL_TX_DEEPH(1);
> >
> > DEP1P2P3 and DEP0CHANGE seem to be required only for faulty PHYs too, I
> > need to see an erratum.
> >
> > > + dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
> > > +
> > > + mdelay(100);
> > > +}
> > > +
> > > +/**
> > > * dwc3_free_one_event_buffer - Frees one event buffer
> > > * @dwc: Pointer to our controller context structure
> > > * @evt: Pointer to event buffer to be freed
> > > @@ -412,9 +431,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
> > > if (ret)
> > > goto err0;
> > >
> > > + if (dwc->quirks & DWC3_AMD_NL_PLAT)
> > > + dwc3_core_nl_set_pipe3(dwc);
> > > +
> > > reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> > > +
> > > reg &= ~DWC3_GCTL_SCALEDOWN_MASK;
> > > - reg &= ~DWC3_GCTL_DISSCRAMBLE;
> > > +
> > > + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> > > + reg |= DWC3_GCTL_DISSCRAMBLE;
> >
> > you're disabling scrambling ? What about EMI ? Why doesn't your device
> > work with scrambling ? I need to see an erratum before I can accept
> > this.
> >
>
> Sorry, disabling scrambling is workaround for debugging the temporary
> simulation board, needn't be set for the SoC chip. Will update in v2.
oh, alright. Then let's not merge this in mainline. I guess I have an
idea which simulation board you're using :-)
> > > + reg |= DWC3_GCTL_U2EXIT_LFPS;
> >
> > hmm, seems like this bit was added due to a faulty host which was found.
> > I need to see an erratum before I can accept this.
> >
> > > + reg |= DWC3_GCTL_GBLHIBERNATIONEN;
> >
> > looks like this should always be set for all versions of the core
> > configured with hibernation.
> >
>
> yes, amd nl chip will support hibernation.
in that case, we do this:
diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index 4d4e854..584dcde 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -449,6 +449,7 @@ static int dwc3_core_init(struct dwc3 *dwc)
case DWC3_GHWPARAMS1_EN_PWROPT_HIB:
/* enable hibernation here */
dwc->nr_scratch = DWC3_GHWPARAMS4_HIBER_SCRATCHBUFS(hwparams4);
+ reg |= DWC3_GCTL_GBLHIBERNATIONEN;
break;
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
On Thu, Sep 25, 2014 at 09:50:32AM -0500, Felipe Balbi wrote:
> Hi,
>
> On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> > index b0f4d52..6138c5d 100644
> > --- a/drivers/usb/dwc3/core.c
> > +++ b/drivers/usb/dwc3/core.c
> > @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
> > }
> >
> > /**
> > + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for NL
> > + * @dwc: Pointer to our controller context structure
> > + */
> > +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
> > +{
> > + u32 reg = 0;
> > +
> > + reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK | DWC3_GUSB3PIPECTL_UX_EXITINPX
> > + | DWC3_GUSB3PIPECTL_UX_EXITINPX | DWC3_GUSB3PIPECTL_U1U2EXITFAIL
> > + | DWC3_GUSB3PIPECTL_DEPOCHANGE | DWC3_GUSB3PIPECTL_RX_DETOPOLL;
>
> UX_EXITINPX is supposed to be used with a faulty PHY, I need to see an
> erratum before I can accept it. You have also duplicated the bit in this
> initialization.
>
> U1U2EXITFAIL -> also a workaround bit and I need to see an erratum.
>
> RX_DETOPOLL -> it seems like it's safe to leave this one out as it can
> only be proven to work with this bit after going through full USB
> certification.
>
What do you mean of the faulty PHY?
We would use AMD PHY to talk with DWC3 controller.
> > + reg |= DWC3_GUSB3PIPECTL_DEP1P2P3(1) | DWC3_GUSB3PIPECTL_TX_DEEPH(1);
>
> DEP1P2P3 and DEP0CHANGE seem to be required only for faulty PHYs too, I
> need to see an erratum.
>
> > + dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
> > +
> > + mdelay(100);
> > +}
> > +
> > +/**
> > * dwc3_free_one_event_buffer - Frees one event buffer
> > * @dwc: Pointer to our controller context structure
> > * @evt: Pointer to event buffer to be freed
> > @@ -412,9 +431,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
> > if (ret)
> > goto err0;
> >
> > + if (dwc->quirks & DWC3_AMD_NL_PLAT)
> > + dwc3_core_nl_set_pipe3(dwc);
> > +
> > reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> > +
> > reg &= ~DWC3_GCTL_SCALEDOWN_MASK;
> > - reg &= ~DWC3_GCTL_DISSCRAMBLE;
> > +
> > + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> > + reg |= DWC3_GCTL_DISSCRAMBLE;
>
> you're disabling scrambling ? What about EMI ? Why doesn't your device
> work with scrambling ? I need to see an erratum before I can accept
> this.
>
Sorry, disabling scrambling is workaround for debugging the temporary
simulation board, needn't be set for the SoC chip. Will update in v2.
> > + reg |= DWC3_GCTL_U2EXIT_LFPS;
>
> hmm, seems like this bit was added due to a faulty host which was found.
> I need to see an erratum before I can accept this.
>
> > + reg |= DWC3_GCTL_GBLHIBERNATIONEN;
>
> looks like this should always be set for all versions of the core
> configured with hibernation.
>
yes, amd nl chip will support hibernation.
> > diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
> > index cebbd01..7f471ff 100644
> > --- a/drivers/usb/dwc3/dwc3-pci.c
> > +++ b/drivers/usb/dwc3/dwc3-pci.c
> > @@ -25,6 +25,9 @@
> > #include
> > #include
> >
> > +#include "platform_data.h"
> > +#include "core.h"
>
> you should never need to include "core.h", why are you including
> "core.h" ???
>
Because I defined DWC3_AMD_NL_PLAT in dwc3 struture at core.h. I think
this quirk might be included on most of the source files like core.c,
gadget.c. If I added into platform_data.h, it would make these source
files include "platform_data.h" either. So I add DWC3_AMD_NL_PLAT into
core.h.
So should I define DWC3_AMD_NL_PLAT and other QUIRKS at
platform_data.h or create a quirks.h file?
> > @@ -102,6 +105,9 @@ static int dwc3_pci_probe(struct pci_dev *pci,
> > struct dwc3_pci *glue;
> > int ret;
> > struct device *dev = &pci->dev;
> > + struct dwc3_platform_data dwc3_pdata;
> > +
> > + memset(&dwc3_pdata, 0x00, sizeof(dwc3_pdata));
> >
> > glue = devm_kzalloc(dev, sizeof(*glue), GFP_KERNEL);
> > if (!glue)
> > @@ -148,6 +154,14 @@ static int dwc3_pci_probe(struct pci_dev *pci,
> >
> > pci_set_drvdata(pci, glue);
> >
> > + if (pci->vendor == PCI_VENDOR_ID_AMD &&
> > + pci->device == PCI_DEVICE_ID_AMD_NL)
> > + dwc3_pdata.quirks |= DWC3_AMD_NL_PLAT;
>
> this looks wrong. It looks like you need several quirk bits for each of
> the workarounds you need. Having a single bit for "my device" is wrong
> and if your next device fixes one or two of these errata, you'd still
> have to introduce a new "my new device" bit, instead of just clearing
> the ones which were fixed.
>
I got it, so do you mean:
if I set "disabling scrambling" workaround, I should use a macro as
DWC3_DISSCRAMBING_QUIRK to present this specific setting. Then use the
Deivce ID to enable these kinds of the quirks I need, is that right?
> > diff --git a/drivers/usb/dwc3/gadget
Re: [RFC PATCH 3/4] usb: dwc3: add quirk to be compatible for AMD NL
Hi,
On Thu, Sep 25, 2014 at 03:21:46PM +0800, Huang Rui wrote:
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
> index b0f4d52..6138c5d 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -115,6 +115,25 @@ static int dwc3_core_soft_reset(struct dwc3 *dwc)
> }
>
> /**
> + * dwc3_core_nl_set_pipe3 - Configure USB3 PHY Interface for NL
> + * @dwc: Pointer to our controller context structure
> + */
> +static void dwc3_core_nl_set_pipe3(struct dwc3 *dwc)
> +{
> + u32 reg = 0;
> +
> + reg |= DWC3_GUSB3PIPECTL_U2SSINP3OK | DWC3_GUSB3PIPECTL_UX_EXITINPX
> + | DWC3_GUSB3PIPECTL_UX_EXITINPX | DWC3_GUSB3PIPECTL_U1U2EXITFAIL
> + | DWC3_GUSB3PIPECTL_DEPOCHANGE | DWC3_GUSB3PIPECTL_RX_DETOPOLL;
UX_EXITINPX is supposed to be used with a faulty PHY, I need to see an
erratum before I can accept it. You have also duplicated the bit in this
initialization.
U1U2EXITFAIL -> also a workaround bit and I need to see an erratum.
RX_DETOPOLL -> it seems like it's safe to leave this one out as it can
only be proven to work with this bit after going through full USB
certification.
> + reg |= DWC3_GUSB3PIPECTL_DEP1P2P3(1) | DWC3_GUSB3PIPECTL_TX_DEEPH(1);
DEP1P2P3 and DEP0CHANGE seem to be required only for faulty PHYs too, I
need to see an erratum.
> + dwc3_writel(dwc->regs, DWC3_GUSB3PIPECTL(0), reg);
> +
> + mdelay(100);
> +}
> +
> +/**
> * dwc3_free_one_event_buffer - Frees one event buffer
> * @dwc: Pointer to our controller context structure
> * @evt: Pointer to event buffer to be freed
> @@ -412,9 +431,19 @@ static int dwc3_core_init(struct dwc3 *dwc)
> if (ret)
> goto err0;
>
> + if (dwc->quirks & DWC3_AMD_NL_PLAT)
> + dwc3_core_nl_set_pipe3(dwc);
> +
> reg = dwc3_readl(dwc->regs, DWC3_GCTL);
> +
> reg &= ~DWC3_GCTL_SCALEDOWN_MASK;
> - reg &= ~DWC3_GCTL_DISSCRAMBLE;
> +
> + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> + reg |= DWC3_GCTL_DISSCRAMBLE;
you're disabling scrambling ? What about EMI ? Why doesn't your device
work with scrambling ? I need to see an erratum before I can accept
this.
> + reg |= DWC3_GCTL_U2EXIT_LFPS;
hmm, seems like this bit was added due to a faulty host which was found.
I need to see an erratum before I can accept this.
> + reg |= DWC3_GCTL_GBLHIBERNATIONEN;
looks like this should always be set for all versions of the core
configured with hibernation.
> diff --git a/drivers/usb/dwc3/dwc3-pci.c b/drivers/usb/dwc3/dwc3-pci.c
> index cebbd01..7f471ff 100644
> --- a/drivers/usb/dwc3/dwc3-pci.c
> +++ b/drivers/usb/dwc3/dwc3-pci.c
> @@ -25,6 +25,9 @@
> #include
> #include
>
> +#include "platform_data.h"
> +#include "core.h"
you should never need to include "core.h", why are you including
"core.h" ???
> @@ -102,6 +105,9 @@ static int dwc3_pci_probe(struct pci_dev *pci,
> struct dwc3_pci *glue;
> int ret;
> struct device *dev = &pci->dev;
> + struct dwc3_platform_data dwc3_pdata;
> +
> + memset(&dwc3_pdata, 0x00, sizeof(dwc3_pdata));
>
> glue = devm_kzalloc(dev, sizeof(*glue), GFP_KERNEL);
> if (!glue)
> @@ -148,6 +154,14 @@ static int dwc3_pci_probe(struct pci_dev *pci,
>
> pci_set_drvdata(pci, glue);
>
> + if (pci->vendor == PCI_VENDOR_ID_AMD &&
> + pci->device == PCI_DEVICE_ID_AMD_NL)
> + dwc3_pdata.quirks |= DWC3_AMD_NL_PLAT;
this looks wrong. It looks like you need several quirk bits for each of
the workarounds you need. Having a single bit for "my device" is wrong
and if your next device fixes one or two of these errata, you'd still
have to introduce a new "my new device" bit, instead of just clearing
the ones which were fixed.
> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> index 0fcc0a3..8277065 100644
> --- a/drivers/usb/dwc3/gadget.c
> +++ b/drivers/usb/dwc3/gadget.c
> @@ -2635,6 +2635,7 @@ static irqreturn_t dwc3_interrupt(int irq, void *_dwc)
> */
> int dwc3_gadget_init(struct dwc3 *dwc)
> {
> + u32 reg;
> int ret;
>
> dwc->ctrl_req = dma_alloc_coherent(dwc->dev, sizeof(*dwc->ctrl_req),
> @@ -2689,6 +2690,13 @@ int dwc3_gadget_init(struct dwc3 *dwc)
> if (ret)
> goto err4;
>
> + if (dwc->quirks & DWC3_AMD_NL_PLAT) {
> + reg = dwc3_readl(dwc->regs, DWC3_DCTL);
> + reg |= DWC3_DCTL_LPM_ERRATA(0xf);
weird, why would Synopsys put this here ? It seems like this is only
useful when LPM Errata is enabled and that's, apparently, a host-side
thing.
Paul, can you comment ?
--
balbi
signature.asc
Description: Digital signature
