[PATCH v3 2/3] USB3/DWC3: Add property "snps, incr-burst-type-adjustment" for INCR burst type

Hi, Rob,

-----Original Message-----
From: Rob Herring [mailto:robh at kernel.org]
Sent: Wednesday, January 04, 2017 5:24 AM
To: Jerry Huang <redacted>
Cc: balbi at kernel.org; mark.rutland at arm.com; catalin.marinas at arm.com;
will.deacon at arm.com; linux at armlinux.org.uk; devicetree at vger.kernel.org;
linux-usb at vger.kernel.org; linux-kernel at vger.kernel.org; linux-arm-
kernel at lists.infradead.org
Subject: Re: [PATCH v3 2/3] USB3/DWC3: Add property "snps, incr-burst-
type-adjustment" for INCR burst type

On Thu, Dec 22, 2016 at 8:52 PM, Jerry Huang [off-list ref] wrote:

quoted

Hi, Rob,

quoted

-----Original Message-----
From: Rob Herring [mailto:robh at kernel.org]
Sent: Friday, December 23, 2016 2:45 AM
To: Jerry Huang <redacted>
Cc: balbi at kernel.org; mark.rutland at arm.com; catalin.marinas at arm.com;
will.deacon at arm.com; linux at armlinux.org.uk;
devicetree at vger.kernel.org; linux-usb at vger.kernel.org;
linux-kernel at vger.kernel.org; linux-arm- kernel at lists.infradead.org
Subject: Re: [PATCH v3 2/3] USB3/DWC3: Add property "snps,
incr-burst- type-adjustment" for INCR burst type

On Mon, Dec 19, 2016 at 05:25:53PM +0800, Changming Huang wrote:

quoted

New property "snps,incr-burst-type-adjustment = <x>, <y>" for
USB3.0

DWC3.

quoted

Field "x": 1/0 - undefined length INCR burst type enable or not;
Field
"y": INCR4/INCR8/INCR16/INCR32/INCR64/INCR128/INCR256 burst type.

While enabling undefined length INCR burst type and INCR16 burst
type, get better write performance on NXP Layerscape platform:
around 3% improvement (from 364MB/s to 375MB/s).

Signed-off-by: Changming Huang <redacted>
---
Changes in v3:
  - add new property for INCR burst in usb node.

 Documentation/devicetree/bindings/usb/dwc3.txt |    5 +++++
 arch/arm/boot/dts/ls1021a.dtsi                 |    1 +
 arch/arm64/boot/dts/freescale/fsl-ls1043a.dtsi |    3 +++
 arch/arm64/boot/dts/freescale/fsl-ls2080a.dtsi |    2 ++
 4 files changed, 11 insertions(+)

diff --git a/Documentation/devicetree/bindings/usb/dwc3.txt

b/Documentation/devicetree/bindings/usb/dwc3.txt
index e3e6983..8c405a3 100644

--- a/Documentation/devicetree/bindings/usb/dwc3.txt
+++ b/Documentation/devicetree/bindings/usb/dwc3.txt

@@ -55,6 +55,10 @@ Optional properties:
    fladj_30mhz_sdbnd signal is invalid or incorrect.

  - <DEPRECATED> tx-fifo-resize: determines if the FIFO *has* to be

reallocated.

quoted

+ - snps,incr-burst-type-adjustment: Value for INCR burst type of

GSBUSCFG0

quoted

+   register, undefined length INCR burst type enable and INCRx type.
+   First field is for undefined length INCR burst type enable or not.
+   Second field is for largest INCRx type enabled.

Why do you need the first field? Is the 2nd field used if the 1st is 0?
If not, then just use the presence of the property to enable or not.

The first field is one switch.
When it is 1, means undefined length INCR burst type enabled, we can use

any length less than or equal to the largest-enabled burst length of
INCR4/8/16/32/64/128/256.

quoted

When it is zero, means INCRx burst mode enabled, we can use one fixed

burst length of 1/4/8/16/32/64/128/256 byte.

quoted

So, the 2nd field is used if the 1st is 0, we need to select one largest burst

length the USB controller can support.

quoted

If we don't want to change the value of this register (use the default value),

we don't need to add this property to usb node.

Just make this a single value with 0 meaning INCR and 4/8/16/etc being INCRx.

Maybe, I didn't describe it clearly. 
According to DWC3 spec, the value "0" of field INCRBrstEna means INCRx burst mode, 1 means INCR burst mode.
Regardless of the value of INCRBrstEna [bit0], we need to modify the other field bit[1,2,3,4,5,6,7] to one INCR burst type  for the platform supported.
Ad you mentioned, if we just use a single value with 0 meaning INCR and 4/8/16/etc being INCRx.
I understand totally that when it is none-zero, we can use it for INCR burst mode. 
Then, when it is 0, how to select the INCRx value?

So, I think we still need two vaue to specify INCRBrstEna and INCRx burst type.