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

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

[Changming Huang]

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 <jerry.huang@nxp.com>
Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
---
 drivers/usb/dwc3/core.c |    6 ++++++
 drivers/usb/dwc3/core.h |   13 +++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
index fea4469..0e11891 100644
--- a/drivers/usb/dwc3/core.c
+++ b/drivers/usb/dwc3/core.c
@@ -621,6 +621,12 @@ static int dwc3_core_init(struct dwc3 *dwc)
 		goto err0;
 	}
 
+	/* Enable Undefined Length INCR Burst Type and Enable INCR16 Burst */
+	reg = dwc3_readl(dwc->regs, DWC3_GSBUSCFG0);
+	reg &= ~DWC3_GSBUSCFG0_INCRBRSTMASK;
+	reg |= DWC3_GSBUSCFG0_INCR16BRSTENA | DWC3_GSBUSCFG0_INCRBRSTENA;
+	dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, reg);
+
 	/*
 	 * Write Linux Version Code to our GUID register so it's easy to figure
 	 * out which kernel version a bug was found.
diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 6b60e42..8bfdb77 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -156,6 +156,19 @@
 
 /* Bit fields */
 
+/* Global SoC Bus Configuration Register 0 */
+#define DWC3_GSBUSCFG0_DATABIGEND	(1 << 11)
+#define DWC3_GSBUSCFG0_DESCBIGEND	(1 << 10)
+#define DWC3_GSBUSCFG0_INCR256BRSTENA	(1 << 7)
+#define DWC3_GSBUSCFG0_INCR128BRSTENA	(1 << 6)
+#define DWC3_GSBUSCFG0_INCR64BRSTENA	(1 << 5)
+#define DWC3_GSBUSCFG0_INCR32BRSTENA	(1 << 4)
+#define DWC3_GSBUSCFG0_INCR16BRSTENA	(1 << 3)
+#define DWC3_GSBUSCFG0_INCR8BRSTENA	(1 << 2)
+#define DWC3_GSBUSCFG0_INCR4BRSTENA	(1 << 1)
+#define DWC3_GSBUSCFG0_INCRBRSTENA	(1 << 0)
+#define DWC3_GSBUSCFG0_INCRBRSTMASK	0xff
+
 /* Global Debug Queue/FIFO Space Available Register */
 #define DWC3_GDBGFIFOSPACE_NUM(n)	((n) & 0x1f)
 #define DWC3_GDBGFIFOSPACE_TYPE(n)	(((n) << 5) & 0x1e0)
-- 
1.7.9.5

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

[Jerry Huang]

> -----Original Message-----
> From: Changming Huang [mailto:jerry.huang@nxp.com]
> Sent: Tuesday, December 13, 2016 5:06 PM
> To: balbi@kernel.org; gregkh@linuxfoundation.org
> Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; Jerry Huang
> <jerry.huang@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>
> Subject: [PATCH] USB3/DWC3: Enable undefined length INCR 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 <jerry.huang@nxp.com>
> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> ---
>  drivers/usb/dwc3/core.c |    6 ++++++
>  drivers/usb/dwc3/core.h |   13 +++++++++++++
>  2 files changed, 19 insertions(+)
> 
> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index
> fea4469..0e11891 100644
> --- a/drivers/usb/dwc3/core.c
> +++ b/drivers/usb/dwc3/core.c
> @@ -621,6 +621,12 @@ static int dwc3_core_init(struct dwc3 *dwc)
>  		goto err0;
>  	}
> 
> +	/* Enable Undefined Length INCR Burst Type and Enable INCR16
> Burst */
> +	reg = dwc3_readl(dwc->regs, DWC3_GSBUSCFG0);
> +	reg &= ~DWC3_GSBUSCFG0_INCRBRSTMASK;
> +	reg |= DWC3_GSBUSCFG0_INCR16BRSTENA |
> DWC3_GSBUSCFG0_INCRBRSTENA;
> +	dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, reg);
> +
>  	/*
>  	 * Write Linux Version Code to our GUID register so it's easy to figure
>  	 * out which kernel version a bug was found.
> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index
> 6b60e42..8bfdb77 100644
> --- a/drivers/usb/dwc3/core.h
> +++ b/drivers/usb/dwc3/core.h
> @@ -156,6 +156,19 @@
> 
>  /* Bit fields */
> 
> +/* Global SoC Bus Configuration Register 0 */
> +#define DWC3_GSBUSCFG0_DATABIGEND	(1 << 11)
> +#define DWC3_GSBUSCFG0_DESCBIGEND	(1 << 10)
> +#define DWC3_GSBUSCFG0_INCR256BRSTENA	(1 << 7)
> +#define DWC3_GSBUSCFG0_INCR128BRSTENA	(1 << 6)
> +#define DWC3_GSBUSCFG0_INCR64BRSTENA	(1 << 5)
> +#define DWC3_GSBUSCFG0_INCR32BRSTENA	(1 << 4)
> +#define DWC3_GSBUSCFG0_INCR16BRSTENA	(1 << 3)
> +#define DWC3_GSBUSCFG0_INCR8BRSTENA	(1 << 2)
> +#define DWC3_GSBUSCFG0_INCR4BRSTENA	(1 << 1)
> +#define DWC3_GSBUSCFG0_INCRBRSTENA	(1 << 0)
> +#define DWC3_GSBUSCFG0_INCRBRSTMASK	0xff
> +
>  /* Global Debug Queue/FIFO Space Available Register */
>  #define DWC3_GDBGFIFOSPACE_NUM(n)	((n) & 0x1f)
>  #define DWC3_GDBGFIFOSPACE_TYPE(n)	(((n) << 5) & 0x1e0)
> --
I will split this patch to two, one is for the performance tune, the other for macro definition in header file.

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 3215 bytes --]


Hi,

Jerry Huang <jerry.huang@nxp.com> writes:
>> -----Original Message-----
>> From: Changming Huang [mailto:jerry.huang@nxp.com]
>> Sent: Tuesday, December 13, 2016 5:06 PM
>> To: balbi@kernel.org; gregkh@linuxfoundation.org
>> Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; Jerry Huang
>> <jerry.huang@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>
>> Subject: [PATCH] USB3/DWC3: Enable undefined length INCR 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 <jerry.huang@nxp.com>
>> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
>> ---
>>  drivers/usb/dwc3/core.c |    6 ++++++
>>  drivers/usb/dwc3/core.h |   13 +++++++++++++
>>  2 files changed, 19 insertions(+)
>> 
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index
>> fea4469..0e11891 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -621,6 +621,12 @@ static int dwc3_core_init(struct dwc3 *dwc)
>>  		goto err0;
>>  	}
>> 
>> +	/* Enable Undefined Length INCR Burst Type and Enable INCR16
>> Burst */
>> +	reg = dwc3_readl(dwc->regs, DWC3_GSBUSCFG0);
>> +	reg &= ~DWC3_GSBUSCFG0_INCRBRSTMASK;
>> +	reg |= DWC3_GSBUSCFG0_INCR16BRSTENA |
>> DWC3_GSBUSCFG0_INCRBRSTENA;
>> +	dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, reg);
>> +
>>  	/*
>>  	 * Write Linux Version Code to our GUID register so it's easy to figure
>>  	 * out which kernel version a bug was found.
>> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index
>> 6b60e42..8bfdb77 100644
>> --- a/drivers/usb/dwc3/core.h
>> +++ b/drivers/usb/dwc3/core.h
>> @@ -156,6 +156,19 @@
>> 
>>  /* Bit fields */
>> 
>> +/* Global SoC Bus Configuration Register 0 */
>> +#define DWC3_GSBUSCFG0_DATABIGEND	(1 << 11)
>> +#define DWC3_GSBUSCFG0_DESCBIGEND	(1 << 10)
>> +#define DWC3_GSBUSCFG0_INCR256BRSTENA	(1 << 7)
>> +#define DWC3_GSBUSCFG0_INCR128BRSTENA	(1 << 6)
>> +#define DWC3_GSBUSCFG0_INCR64BRSTENA	(1 << 5)
>> +#define DWC3_GSBUSCFG0_INCR32BRSTENA	(1 << 4)
>> +#define DWC3_GSBUSCFG0_INCR16BRSTENA	(1 << 3)
>> +#define DWC3_GSBUSCFG0_INCR8BRSTENA	(1 << 2)
>> +#define DWC3_GSBUSCFG0_INCR4BRSTENA	(1 << 1)
>> +#define DWC3_GSBUSCFG0_INCRBRSTENA	(1 << 0)
>> +#define DWC3_GSBUSCFG0_INCRBRSTMASK	0xff
>> +
>>  /* Global Debug Queue/FIFO Space Available Register */
>>  #define DWC3_GDBGFIFOSPACE_NUM(n)	((n) & 0x1f)
>>  #define DWC3_GDBGFIFOSPACE_TYPE(n)	(((n) << 5) & 0x1e0)
>> --
> I will split this patch to two, one is for the performance tune, the
> other for macro definition in header file.

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.

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

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

[Jerry Huang]

> -----Original Message-----
> From: Felipe Balbi [mailto:balbi@kernel.org]
> Sent: Friday, December 16, 2016 5:17 PM
> To: Jerry Huang <jerry.huang@nxp.com>; 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:
> >> -----Original Message-----
> >> From: Changming Huang [mailto:jerry.huang@nxp.com]
> >> Sent: Tuesday, December 13, 2016 5:06 PM
> >> To: balbi@kernel.org; gregkh@linuxfoundation.org
> >> Cc: linux-usb@vger.kernel.org; linux-kernel@vger.kernel.org; Jerry
> >> Huang <jerry.huang@nxp.com>; Rajesh Bhagat <rajesh.bhagat@nxp.com>
> >> Subject: [PATCH] USB3/DWC3: Enable undefined length INCR 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 <jerry.huang@nxp.com>
> >> Signed-off-by: Rajesh Bhagat <rajesh.bhagat@nxp.com>
> >> ---
> >>  drivers/usb/dwc3/core.c |    6 ++++++
> >>  drivers/usb/dwc3/core.h |   13 +++++++++++++
> >>  2 files changed, 19 insertions(+)
> >>
> >> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c index
> >> fea4469..0e11891 100644
> >> --- a/drivers/usb/dwc3/core.c
> >> +++ b/drivers/usb/dwc3/core.c
> >> @@ -621,6 +621,12 @@ static int dwc3_core_init(struct dwc3 *dwc)
> >>  		goto err0;
> >>  	}
> >>
> >> +	/* Enable Undefined Length INCR Burst Type and Enable INCR16
> >> Burst */
> >> +	reg = dwc3_readl(dwc->regs, DWC3_GSBUSCFG0);
> >> +	reg &= ~DWC3_GSBUSCFG0_INCRBRSTMASK;
> >> +	reg |= DWC3_GSBUSCFG0_INCR16BRSTENA |
> >> DWC3_GSBUSCFG0_INCRBRSTENA;
> >> +	dwc3_writel(dwc->regs, DWC3_GSBUSCFG0, reg);
> >> +
> >>  	/*
> >>  	 * Write Linux Version Code to our GUID register so it's easy to figure
> >>  	 * out which kernel version a bug was found.
> >> diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h index
> >> 6b60e42..8bfdb77 100644
> >> --- a/drivers/usb/dwc3/core.h
> >> +++ b/drivers/usb/dwc3/core.h
> >> @@ -156,6 +156,19 @@
> >>
> >>  /* Bit fields */
> >>
> >> +/* Global SoC Bus Configuration Register 0 */
> >> +#define DWC3_GSBUSCFG0_DATABIGEND	(1 << 11)
> >> +#define DWC3_GSBUSCFG0_DESCBIGEND	(1 << 10)
> >> +#define DWC3_GSBUSCFG0_INCR256BRSTENA	(1 << 7)
> >> +#define DWC3_GSBUSCFG0_INCR128BRSTENA	(1 << 6)
> >> +#define DWC3_GSBUSCFG0_INCR64BRSTENA	(1 << 5)
> >> +#define DWC3_GSBUSCFG0_INCR32BRSTENA	(1 << 4)
> >> +#define DWC3_GSBUSCFG0_INCR16BRSTENA	(1 << 3)
> >> +#define DWC3_GSBUSCFG0_INCR8BRSTENA	(1 << 2)
> >> +#define DWC3_GSBUSCFG0_INCR4BRSTENA	(1 << 1)
> >> +#define DWC3_GSBUSCFG0_INCRBRSTENA	(1 << 0)
> >> +#define DWC3_GSBUSCFG0_INCRBRSTMASK	0xff
> >> +
> >>  /* Global Debug Queue/FIFO Space Available Register */
> >>  #define DWC3_GDBGFIFOSPACE_NUM(n)	((n) & 0x1f)
> >>  #define DWC3_GDBGFIFOSPACE_TYPE(n)	(((n) << 5) & 0x1e0)
> >> --
> > I will split this patch to two, one is for the performance tune, the
> > other for macro definition in header file.
> 
> 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?

> 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
GSBUSCFG1 = 0x00000700
GTXTHRCFG = 0x00000000
GRXTHRCFG = 0x00000000
GCTL = 0x30c11004
GEVTEN = 0x00000000
GSTS = 0x3e800001
GUCTL1 = 0x0000018a
GSNPSID = 0x5533280a
GGPIO = 0x00000000
GUID = 0x00040900
GUCTL = 0x02008010
GBUSERRADDR0 = 0x00000000
GBUSERRADDR1 = 0x00000000
GPRTBIMAP0 = 0x00000000
GPRTBIMAP1 = 0x00000000
GHWPARAMS0 = 0x4020400a
GHWPARAMS1 = 0x81e0c93b
GHWPARAMS2 = 0x0130280a
GHWPARAMS3 = 0x04108485
GHWPARAMS4 = 0x47822004
GHWPARAMS5 = 0x04204108
GHWPARAMS6 = 0x09049c20
GHWPARAMS7 = 0x0308044d
GDBGFIFOSPACE = 0x00820000
GDBGLTSSM = 0x40000042
GPRTBIMAP_HS0 = 0x00000000
GPRTBIMAP_HS1 = 0x00000000
GPRTBIMAP_FS0 = 0x00000000
GPRTBIMAP_FS1 = 0x00000000
GUSB2PHYCFG(0) = 0x40102440
GUSB2PHYCFG(1) = 0x00000000
GUSB2PHYCFG(2) = 0x00000000
GUSB2PHYCFG(3) = 0x00000000
GUSB2PHYCFG(4) = 0x00000000
GUSB2PHYCFG(5) = 0x00000000
GUSB2PHYCFG(6) = 0x00000000
GUSB2PHYCFG(7) = 0x00000000
GUSB2PHYCFG(8) = 0x00000000
GUSB2PHYCFG(9) = 0x00000000
GUSB2PHYCFG(10) = 0x00000000
GUSB2PHYCFG(11) = 0x00000000
GUSB2PHYCFG(12) = 0x00000000
GUSB2PHYCFG(13) = 0x00000000
GUSB2PHYCFG(14) = 0x00000000
GUSB2PHYCFG(15) = 0x00000000
GUSB2I2CCTL(0) = 0x00000000
GUSB2I2CCTL(1) = 0x00000000
GUSB2I2CCTL(2) = 0x00000000
GUSB2I2CCTL(3) = 0x00000000
GUSB2I2CCTL(4) = 0x00000000
GUSB2I2CCTL(5) = 0x00000000
GUSB2I2CCTL(6) = 0x00000000
GUSB2I2CCTL(7) = 0x00000000
GUSB2I2CCTL(8) = 0x00000000
GUSB2I2CCTL(9) = 0x00000000
GUSB2I2CCTL(10) = 0x00000000
GUSB2I2CCTL(11) = 0x00000000
GUSB2I2CCTL(12) = 0x00000000
GUSB2I2CCTL(13) = 0x00000000
GUSB2I2CCTL(14) = 0x00000000
GUSB2I2CCTL(15) = 0x00000000
GUSB2PHYACC(0) = 0x00000000
GUSB2PHYACC(1) = 0x00000000
GUSB2PHYACC(2) = 0x00000000
GUSB2PHYACC(3) = 0x00000000
GUSB2PHYACC(4) = 0x00000000
GUSB2PHYACC(5) = 0x00000000
GUSB2PHYACC(6) = 0x00000000
GUSB2PHYACC(7) = 0x00000000
GUSB2PHYACC(8) = 0x00000000
GUSB2PHYACC(9) = 0x00000000
GUSB2PHYACC(10) = 0x00000000
GUSB2PHYACC(11) = 0x00000000
GUSB2PHYACC(12) = 0x00000000
GUSB2PHYACC(13) = 0x00000000
GUSB2PHYACC(14) = 0x00000000
GUSB2PHYACC(15) = 0x00000000
GUSB3PIPECTL(0) = 0x110e0002
GUSB3PIPECTL(1) = 0x00000000
GUSB3PIPECTL(2) = 0x00000000
GUSB3PIPECTL(3) = 0x00000000
GUSB3PIPECTL(4) = 0x00000000
GUSB3PIPECTL(5) = 0x00000000
GUSB3PIPECTL(6) = 0x00000000
GUSB3PIPECTL(7) = 0x00000000
GUSB3PIPECTL(8) = 0x00000000
GUSB3PIPECTL(9) = 0x00000000
GUSB3PIPECTL(10) = 0x00000000
GUSB3PIPECTL(11) = 0x00000000
GUSB3PIPECTL(12) = 0x00000000
GUSB3PIPECTL(13) = 0x00000000
GUSB3PIPECTL(14) = 0x00000000
GUSB3PIPECTL(15) = 0x00000000
GTXFIFOSIZ(0) = 0x00000082
GTXFIFOSIZ(1) = 0x00820103
GTXFIFOSIZ(2) = 0x01850205
GTXFIFOSIZ(3) = 0x038a0022
GTXFIFOSIZ(4) = 0x00000000
GTXFIFOSIZ(5) = 0x00000000
GTXFIFOSIZ(6) = 0x00000000
GTXFIFOSIZ(7) = 0x00000000
GTXFIFOSIZ(8) = 0x00000000
GTXFIFOSIZ(9) = 0x00000000
GTXFIFOSIZ(10) = 0x00000000
GTXFIFOSIZ(11) = 0x00000000
GTXFIFOSIZ(12) = 0x00000000
GTXFIFOSIZ(13) = 0x00000000
GTXFIFOSIZ(14) = 0x00000000
GTXFIFOSIZ(15) = 0x00000000
GTXFIFOSIZ(16) = 0x00000000
GTXFIFOSIZ(17) = 0x00000000
GTXFIFOSIZ(18) = 0x00000000
GTXFIFOSIZ(19) = 0x00000000
GTXFIFOSIZ(20) = 0x00000000
GTXFIFOSIZ(21) = 0x00000000
GTXFIFOSIZ(22) = 0x00000000
GTXFIFOSIZ(23) = 0x00000000
GTXFIFOSIZ(24) = 0x00000000
GTXFIFOSIZ(25) = 0x00000000
GTXFIFOSIZ(26) = 0x00000000
GTXFIFOSIZ(27) = 0x00000000
GTXFIFOSIZ(28) = 0x00000000
GTXFIFOSIZ(29) = 0x00000000
GTXFIFOSIZ(30) = 0x00000000
GTXFIFOSIZ(31) = 0x00000000
GRXFIFOSIZ(0) = 0x00000084
GRXFIFOSIZ(1) = 0x00840104
GRXFIFOSIZ(2) = 0x01880180
GRXFIFOSIZ(3) = 0x00000000
GRXFIFOSIZ(4) = 0x00000000
GRXFIFOSIZ(5) = 0x00000000
GRXFIFOSIZ(6) = 0x00000000
GRXFIFOSIZ(7) = 0x00000000
GRXFIFOSIZ(8) = 0x00000000
GRXFIFOSIZ(9) = 0x00000000
GRXFIFOSIZ(10) = 0x00000000
GRXFIFOSIZ(11) = 0x00000000
GRXFIFOSIZ(12) = 0x00000000
GRXFIFOSIZ(13) = 0x00000000
GRXFIFOSIZ(14) = 0x00000000
GRXFIFOSIZ(15) = 0x00000000
GRXFIFOSIZ(16) = 0x00000000
GRXFIFOSIZ(17) = 0x00000000
GRXFIFOSIZ(18) = 0x00000000
GRXFIFOSIZ(19) = 0x00000000
GRXFIFOSIZ(20) = 0x00000000
GRXFIFOSIZ(21) = 0x00000000
GRXFIFOSIZ(22) = 0x00000000
GRXFIFOSIZ(23) = 0x00000000
GRXFIFOSIZ(24) = 0x00000000
GRXFIFOSIZ(25) = 0x00000000
GRXFIFOSIZ(26) = 0x00000000
GRXFIFOSIZ(27) = 0x00000000
GRXFIFOSIZ(28) = 0x00000000
GRXFIFOSIZ(29) = 0x00000000
GRXFIFOSIZ(30) = 0x00000000
GRXFIFOSIZ(31) = 0x00000000
GEVNTADRLO(0) = 0x00000000
GEVNTADRHI(0) = 0x00000000
GEVNTSIZ(0) = 0x00000100
GEVNTCOUNT(0) = 0x00000000
GHWPARAMS8 = 0x00000904
DCFG = 0x00080804
DCTL = 0x00f00000
DEVTEN = 0x00000000
DSTS = 0x00c2bbf4
DGCMDPAR = 0x00000000
DGCMD = 0x00000000
DALEPENA = 0x00000000
DEPCMDPAR2(0) = 0x00000000
DEPCMDPAR1(0) = 0x00000002
DEPCMDPAR0(0) = 0xfec64001
DEPCMD(0) = 0x00000000
DEPCMDPAR2(1) = 0x00000000
DEPCMDPAR1(1) = 0x00000000
DEPCMDPAR0(1) = 0x00000000
DEPCMD(1) = 0x00000000
DEPCMDPAR2(2) = 0xfec63000
DEPCMDPAR1(2) = 0x00000000
DEPCMDPAR0(2) = 0x0000007f
DEPCMD(2) = 0x00000000
DEPCMDPAR2(3) = 0x00000000
DEPCMDPAR1(3) = 0x00000000
DEPCMDPAR0(3) = 0x00000000
DEPCMD(3) = 0x00000000
DEPCMDPAR2(4) = 0xfec67000
DEPCMDPAR1(4) = 0x00000000
DEPCMDPAR0(4) = 0xfec668a8
DEPCMD(4) = 0x00000000
DEPCMDPAR2(5) = 0x00000000
DEPCMDPAR1(5) = 0x00000000
DEPCMDPAR0(5) = 0x00000000
DEPCMD(5) = 0x00000000
DEPCMDPAR2(6) = 0x00000000
DEPCMDPAR1(6) = 0x00000000
DEPCMDPAR0(6) = 0x00000000
DEPCMD(6) = 0x00000000
DEPCMDPAR2(7) = 0x00000000
DEPCMDPAR1(7) = 0x00000000
DEPCMDPAR0(7) = 0x00000000
DEPCMD(7) = 0x00000000
DEPCMDPAR2(8) = 0x00000000
DEPCMDPAR1(8) = 0x00000000
DEPCMDPAR0(8) = 0x00000000
DEPCMD(8) = 0x00000000
DEPCMDPAR2(9) = 0x00000000
DEPCMDPAR1(9) = 0x00000000
DEPCMDPAR0(9) = 0x00000000
DEPCMD(9) = 0x00000000
DEPCMDPAR2(10) = 0x00000000
DEPCMDPAR1(10) = 0x00000000
DEPCMDPAR0(10) = 0x00000000
DEPCMD(10) = 0x00000000
DEPCMDPAR2(11) = 0x00000000
DEPCMDPAR1(11) = 0x00000000
DEPCMDPAR0(11) = 0x00000000
DEPCMD(11) = 0x00000000
DEPCMDPAR2(12) = 0x00000000
DEPCMDPAR1(12) = 0x00000000
DEPCMDPAR0(12) = 0x00000000
DEPCMD(12) = 0x00000000
DEPCMDPAR2(13) = 0x00000000
DEPCMDPAR1(13) = 0x00000000
DEPCMDPAR0(13) = 0x00000000
DEPCMD(13) = 0x00000000
DEPCMDPAR2(14) = 0x00000000
DEPCMDPAR1(14) = 0x00000000
DEPCMDPAR0(14) = 0x00000000
DEPCMD(14) = 0x00000000
DEPCMDPAR2(15) = 0x00000000
DEPCMDPAR1(15) = 0x00000000
DEPCMDPAR0(15) = 0x00000000
DEPCMD(15) = 0x00000000
DEPCMDPAR2(16) = 0x00000000
DEPCMDPAR1(16) = 0x00000000
DEPCMDPAR0(16) = 0x00000000
DEPCMD(16) = 0x00000000
DEPCMDPAR2(17) = 0x00000000
DEPCMDPAR1(17) = 0x00000000
DEPCMDPAR0(17) = 0x00000000
DEPCMD(17) = 0x00000000
DEPCMDPAR2(18) = 0x00000000
DEPCMDPAR1(18) = 0x00000000
DEPCMDPAR0(18) = 0x00000000
DEPCMD(18) = 0x00000000
DEPCMDPAR2(19) = 0x00000000
DEPCMDPAR1(19) = 0x00000000
DEPCMDPAR0(19) = 0x00000000
DEPCMD(19) = 0x00000000
DEPCMDPAR2(20) = 0x00000000
DEPCMDPAR1(20) = 0x00000000
DEPCMDPAR0(20) = 0x00000000
DEPCMD(20) = 0x00000000
DEPCMDPAR2(21) = 0x00000000
DEPCMDPAR1(21) = 0x00000000
DEPCMDPAR0(21) = 0x00000000
DEPCMD(21) = 0x00000000
DEPCMDPAR2(22) = 0x00000000
DEPCMDPAR1(22) = 0x00000000
DEPCMDPAR0(22) = 0x00000000
DEPCMD(22) = 0x00000000
DEPCMDPAR2(23) = 0x00000000
DEPCMDPAR1(23) = 0x00000000
DEPCMDPAR0(23) = 0x00000000
DEPCMD(23) = 0x00000000
DEPCMDPAR2(24) = 0x00000000
DEPCMDPAR1(24) = 0x00000000
DEPCMDPAR0(24) = 0x00000000
DEPCMD(24) = 0x00000000
DEPCMDPAR2(25) = 0x00000000
DEPCMDPAR1(25) = 0x00000000

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1554 bytes --]


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 :-)

>> 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. 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?

> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

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

[Jerry Huang]

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

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

[Felipe Balbi]

Hi,

Jerry Huang <jerry.huang@nxp.com> writes:
> 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.

that's not for burst increment, though.

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

interesting, it doesn't describe what happens to OCP or PCI.

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

cool, thanks :-)

>> > 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
> --
> To unsubscribe from this list: send the line "unsubscribe linux-usb" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

-- 
balbi

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

[Jerry Huang]

Hi, Balbi,

> -----Original Message-----
> From: Felipe Balbi [mailto:balbi@kernel.org]
> Sent: Saturday, December 17, 2016 1:02 AM
> 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:
> > 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.
> 
> that's not for burst increment, though.
I created one new property " snps,incr-burst-type-adjustment = <x>, <y>" to identify it from usb node, and will send out the v3 patch.

> >> >> 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.
> 
> interesting, it doesn't describe what happens to OCP or PCI.
Yes, just mention AHB and AXI from DWC superspeed USB3.0 controller databook (Version 2.50a, November 2012, maybe it is too old version for this IP).

> > 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.
> 
> cool, thanks :-)
Have confirmed with hardware engineer,  the maximum INCR burst size of NXP platform is 16 byte

> >> > 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
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-usb"
> > in the body of a message to majordomo@vger.kernel.org More
> majordomo
> > info at  http://vger.kernel.org/majordomo-info.html
> 
> --
> balbi

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

[Felipe Balbi]

[-- Attachment #1: Type: text/plain, Size: 1514 bytes --]


Hi,

Jerry Huang <jerry.huang@nxp.com> writes:
>> >> 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.
>> 
>> that's not for burst increment, though.
>
> I created one new property " snps,incr-burst-type-adjustment = <x>,
> <y>" to identify it from usb node, and will send out the v3 patch.

okay, don't forget to Cc devicetree@vger.kernel.org. Let's see what
those guys say.

>> > 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.
>> 
>> cool, thanks :-)
>
> Have confirmed with hardware engineer, the maximum INCR burst size of
> NXP platform is 16 byte

good, thanks for that.

-- 
balbi

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]