RE: [PATCH] USB3/DWC3: Enable undefined length INCR burst type

[Jerry Huang <jerry.huang@nxp.com>]

Hi, Balbi,
> -----Original Message-----
> From: Felipe Balbi [mailto:balbi@kernel.org]
> Sent: Friday, December 16, 2016 7:44 PM
> To: Jerry Huang <jerry.huang@nxp.com>; gregkh@linuxfoundation.org
> Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; Rajesh Bhagat
> <rajesh.bhagat@nxp.com>
> Subject: RE: [PATCH] USB3/DWC3: Enable undefined length INCR burst type
> 
> 
> Hi,
> 
> Jerry Huang <jerry.huang@nxp.com> writes:
> >> there's no need for that. This patch is in good format. I do have a
> >> question,
> >> however: how do you know this will work for all users? Burst size is
> >> a function of how wide the interconnect where dwc3 is attached to, is.
> > So I need to generate one new property in usb node to identify my
> platform?
> 
> Well, we probably need a property to be passed, yes. But let's go through it
> all first :-)
I think "snps,quirk-frame-length-adjustment" is one good reference, which can pass the required value to driver from DTS file.

> >> You could very well be degrading performance for some users here. Can
> >> you send me the result of the following commands *without* this patch
> applied?
> >>
> >> # mkdir -p /d
> >> # mount -t debugfs none /d
> >> # cat /d/*dwc3*/regdump
> >>
> > Below is the regdump:
> > root@ls1043ardb:/d/3000000.usb3# cat regdump
> > GSBUSCFG0 = 0x00100080
> 
> so you already have INCR256 here. There's one note in the databook which
> just caught my attention. It states the following:
> 
> 	"Undefined burst length has priority over all other burst lenghts."
> 
> This means that setting both INCR16 and undefined INCR is unnecessary.
When bit0 = 1 (Undefined Length INCR Burst Type Enable), which means:
 1: INCR (undefined length) burst mode
- AHB configurations: HBURST uses SINGLE or INCR
of any length less than or equal to the largest-enabled
burst length of INCR4/8/16/32/64/128/256.
- AXI configurations: ARLEN/AWLEN uses any length
less than or equal to the largest-enabled burst length
of INCR4/8/16/32/64/128/256.
So, after enable undefined length INCR burst and enable INCR16, controller will use less than or equal to 16byte.

> Only Undefined INCR will be taken into consideration. Can you check with
> your HW engineers what's the largest burst the interconnect is supposed to
> support?
I will check it with IP designer.

> > GSBUSCFG1 = 0x00000700
> 
> 8 AXI pipelined requests
> 
> > GSNPSID = 0x5533280a
> 
> 2.80a cool :-)
> 
> I'll check these settings on my platform as well and see if there's any setting
> which would improve transfer speed. This is a very good idea, btw, but we
> need to be careful about how to play with it.
> 
> --
> balbi