All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7
@ 2017-10-07 10:16 Stefan Wahren
       [not found] ` <1507371414-21688-1-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Wahren @ 2017-10-07 10:16 UTC (permalink / raw)
  To: Eric Anholt, Rob Herring, Mark Rutland
  Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

In case the RPi Zero has at least a device connected to the OTG port
at boot time, the upper limit of tx fifo size for endpoint 6 and 7 is
also reduced to 512 bytes. So fix this accordingly.

Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
Fixes: 1aa1d858f582 ("ARM: dts: bcm283x: Add dtsi for OTG mode")
---
 arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi b/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi
index e7d217c..b9dff34 100644
--- a/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi
+++ b/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi
@@ -6,5 +6,5 @@
 	 * According to dwc2 the sum of all device EP
 	 * fifo sizes shouldn't exceed 3776 bytes.
 	 */
-	g-tx-fifo-size = <256 256 512 512 512 768 768>;
+	g-tx-fifo-size = <256 256 512 512 512 512 512>;
 };
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7
       [not found] ` <1507371414-21688-1-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
@ 2017-10-27 20:58   ` Stefan Wahren
       [not found]     ` <160210128.18582.1509137904239-7tX72C7vayboQLBSYMtkGA@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Wahren @ 2017-10-27 20:58 UTC (permalink / raw)
  To: Eric Anholt, Rob Herring, Mark Rutland
  Cc: Phil Elwell, Florian Fainelli,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA


> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> hat am 7. Oktober 2017 um 12:16 geschrieben:
> 
> 
> In case the RPi Zero has at least a device connected to the OTG port
> at boot time, the upper limit of tx fifo size for endpoint 6 and 7 is
> also reduced to 512 bytes. So fix this accordingly.
> 
> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> Fixes: 1aa1d858f582 ("ARM: dts: bcm283x: Add dtsi for OTG mode")

gentle ping ...

> ---
>  arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi b/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi
> index e7d217c..b9dff34 100644
> --- a/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi
> +++ b/arch/arm/boot/dts/bcm283x-rpi-usb-otg.dtsi
> @@ -6,5 +6,5 @@
>  	 * According to dwc2 the sum of all device EP
>  	 * fifo sizes shouldn't exceed 3776 bytes.
>  	 */
> -	g-tx-fifo-size = <256 256 512 512 512 768 768>;
> +	g-tx-fifo-size = <256 256 512 512 512 512 512>;
>  };
> -- 
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7
       [not found]     ` <160210128.18582.1509137904239-7tX72C7vayboQLBSYMtkGA@public.gmane.org>
@ 2017-10-31  0:40       ` Eric Anholt
       [not found]         ` <871slkw3ms.fsf-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Eric Anholt @ 2017-10-31  0:40 UTC (permalink / raw)
  To: Stefan Wahren, Rob Herring, Mark Rutland
  Cc: Phil Elwell, Florian Fainelli,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> writes:

>> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> hat am 7. Oktober 2017 um 12:16 geschrieben:
>> 
>> 
>> In case the RPi Zero has at least a device connected to the OTG port
>> at boot time, the upper limit of tx fifo size for endpoint 6 and 7 is
>> also reduced to 512 bytes. So fix this accordingly.
>> 
>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>> Fixes: 1aa1d858f582 ("ARM: dts: bcm283x: Add dtsi for OTG mode")
>
> gentle ping ...

I've tried to make sense of this a couple of times, but I don't get it:
why does EP 6/7 get reduced to 512 bytes in this case?

However, I'm not an expert in the USB stuff, so if you're confident in
it, I'm willing to apply (once the merge window opens back up)

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7
       [not found]         ` <871slkw3ms.fsf-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
@ 2017-10-31  8:43           ` Stefan Wahren
       [not found]             ` <1506055926.93787.1509439381859-7tX72C7vayboQLBSYMtkGA@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Wahren @ 2017-10-31  8:43 UTC (permalink / raw)
  To: Eric Anholt, John Youn, Rob Herring, Mark Rutland, Minas Harutyunyan
  Cc: Phil Elwell, linux-usb-u79uwXL29TY76Z2rM5mHXA, Florian Fainelli,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Eric,

> Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> hat am 31. Oktober 2017 um 01:40 geschrieben:
> 
> 
> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> writes:
> 
> >> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> hat am 7. Oktober 2017 um 12:16 geschrieben:
> >> 
> >> 
> >> In case the RPi Zero has at least a device connected to the OTG port
> >> at boot time, the upper limit of tx fifo size for endpoint 6 and 7 is
> >> also reduced to 512 bytes. So fix this accordingly.
> >> 
> >> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> >> Fixes: 1aa1d858f582 ("ARM: dts: bcm283x: Add dtsi for OTG mode")
> >
> > gentle ping ...
> 
> I've tried to make sense of this a couple of times, but I don't get it:
> why does EP 6/7 get reduced to 512 bytes in this case?

i cannot give you an answer for this specific case.

Since the dwc2 databook isn't public, i started a thread on linux-usb [1] about proper fifo size configuration. But i didn't get any reply.

The problem here is there different contraints:
* the sum of all fifo values must not exceed 3776 bytes
* each slot have its individual upper limit (available in the BCM2835 datasheet)

During my tests for OTG mode i missed the specific case above. Now my determined limits of 512 for EP 6 and 7 are contrary to the BCM2835 datasheet. Maybe the Synopsys guys have an answer?

Btw the values in the downstream tree also violate the contraints. 

[1] - https://www.spinics.net/lists/linux-usb/msg157200.html
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7
       [not found]             ` <1506055926.93787.1509439381859-7tX72C7vayboQLBSYMtkGA@public.gmane.org>
@ 2017-11-16 22:33               ` Stefan Wahren
  2017-11-17  7:48                 ` Minas Harutyunyan
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Wahren @ 2017-11-16 22:33 UTC (permalink / raw)
  To: Eric Anholt, Rob Herring, Minas Harutyunyan, John Youn, Mark Rutland
  Cc: Phil Elwell, linux-usb-u79uwXL29TY76Z2rM5mHXA, Florian Fainelli,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Eric,

> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> hat am 31. Oktober 2017 um 09:43 geschrieben:
> 
> 
> Hi Eric,
> 
> > Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> hat am 31. Oktober 2017 um 01:40 geschrieben:
> > 
> > 
> > Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> writes:
> > 
> > >> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> hat am 7. Oktober 2017 um 12:16 geschrieben:
> > >> 
> > >> 
> > >> In case the RPi Zero has at least a device connected to the OTG port
> > >> at boot time, the upper limit of tx fifo size for endpoint 6 and 7 is
> > >> also reduced to 512 bytes. So fix this accordingly.
> > >> 
> > >> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> > >> Fixes: 1aa1d858f582 ("ARM: dts: bcm283x: Add dtsi for OTG mode")
> > >
> > > gentle ping ...
> > 
> > I've tried to make sense of this a couple of times, but I don't get it:
> > why does EP 6/7 get reduced to 512 bytes in this case?
> 
> i cannot give you an answer for this specific case.
> 
> Since the dwc2 databook isn't public, i started a thread on linux-usb [1] about proper fifo size configuration. But i didn't get any reply.
> 
> The problem here is there different contraints:
> * the sum of all fifo values must not exceed 3776 bytes
> * each slot have its individual upper limit (available in the BCM2835 datasheet)
> 
> During my tests for OTG mode i missed the specific case above. Now my determined limits of 512 for EP 6 and 7 are contrary to the BCM2835 datasheet. Maybe the Synopsys guys have an answer?
> 
> Btw the values in the downstream tree also violate the contraints. 
> 
> [1] - https://www.spinics.net/lists/linux-usb/msg157200.html

still concerns about this patch, because it's not included in dt-fixes?
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7
  2017-11-16 22:33               ` Stefan Wahren
@ 2017-11-17  7:48                 ` Minas Harutyunyan
       [not found]                   ` <410670D7E743164D87FA6160E7907A560113A3F8F0-ouFQeoKRuQ8bQ7k9MBbv4fufCSb+aD3WLzEdoUbNIic@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Minas Harutyunyan @ 2017-11-17  7:48 UTC (permalink / raw)
  To: Stefan Wahren, Eric Anholt, Rob Herring, Minas Harutyunyan,
	John Youn, Mark Rutland
  Cc: Phil Elwell, linux-usb-u79uwXL29TY76Z2rM5mHXA, Florian Fainelli,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

On 11/17/2017 2:35 AM, Stefan Wahren wrote:
> Hi Eric,
> 
>> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> hat am 31. Oktober 2017 um 09:43 geschrieben:
>>
>>
>> Hi Eric,
>>
>>> Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> hat am 31. Oktober 2017 um 01:40 geschrieben:
>>>
>>>
>>> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> writes:
>>>
>>>>> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> hat am 7. Oktober 2017 um 12:16 geschrieben:
>>>>>
>>>>>
>>>>> In case the RPi Zero has at least a device connected to the OTG port
>>>>> at boot time, the upper limit of tx fifo size for endpoint 6 and 7 is
>>>>> also reduced to 512 bytes. So fix this accordingly.
>>>>>
>>>>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
>>>>> Fixes: 1aa1d858f582 ("ARM: dts: bcm283x: Add dtsi for OTG mode")
>>>>
>>>> gentle ping ...
>>>
>>> I've tried to make sense of this a couple of times, but I don't get it:
>>> why does EP 6/7 get reduced to 512 bytes in this case?
>>
>> i cannot give you an answer for this specific case.
>>
>> Since the dwc2 databook isn't public, i started a thread on linux-usb [1] about proper fifo size configuration. But i didn't get any reply.
>>
>> The problem here is there different contraints:
>> * the sum of all fifo values must not exceed 3776 bytes
>> * each slot have its individual upper limit (available in the BCM2835 datasheet)
>>
>> During my tests for OTG mode i missed the specific case above. Now my determined limits of 512 for EP 6 and 7 are contrary to the BCM2835 datasheet. Maybe the Synopsys guys have an answer?
>>
>> Btw the values in the downstream tree also violate the contraints.
>>
>> [1] - https://urldefense.proofpoint.com/v2/url?u=https-3A__www.spinics.net_lists_linux-2Dusb_msg157200.html&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=6z9Al9FrHR_ZqbbtSAsD16pvOL2S3XHxQnSzq8kusyI&m=O2ngUGajgx3MfHk4IQKp5iRlwOOpG62gkji4R2gE64k&s=8ClOBwWN5B69mL1TDjnN3l78JBrvd1YHunRdzei-xrA&e=
> 
> still concerns about this patch, because it's not included in dt-fixes?
> 

Hi Stefan,

According BCM2835 datasheet Total Data FIFO RAM Depth is 4096. I assume 
that in datasheet assumed 4096 dwords, not a bytes. DFIFO depth in 
dwords stored GHWCFG3[31:16].

So, for buffer DMA mode max space in dwords which available to allocate 
to FIFO's is 4096-8*2=4080, where 8 EP count including EP0 and 2 for 
both directions of EP's. Based on bcm283x-rpi-usb-otg.dtsi:
g-rx-fifo-size + g-np-tx-fifo-size + g-tx-fifo-size[1..7] = 
256+32+256+256+512+512+512+768+768 = 3872 < 4080.
So, I don't see any reason to change EP 6,7 TxFIFO sizes.

More probably comment in dtsi not correct:
"* fifo sizes shouldn't exceed 3776 bytes."
Please check FIFO depth in GHWCFG3 and if it 4096 then update the comment.

Thanks,
Minas




--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7
       [not found]                   ` <410670D7E743164D87FA6160E7907A560113A3F8F0-ouFQeoKRuQ8bQ7k9MBbv4fufCSb+aD3WLzEdoUbNIic@public.gmane.org>
@ 2017-11-18 20:21                     ` Stefan Wahren
       [not found]                       ` <410670D7E743164D87FA6160E7907A560113A409A8@am04wembxa.internal.synopsys.com>
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Wahren @ 2017-11-18 20:21 UTC (permalink / raw)
  To: John Youn, Eric Anholt, Minas Harutyunyan
  Cc: Phil Elwell, linux-usb-u79uwXL29TY76Z2rM5mHXA, Florian Fainelli,
	Rob Herring, Mark Rutland,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Minas,

> Minas Harutyunyan <Minas.Harutyunyan-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> hat am 17. November 2017 um 08:48 geschrieben:
> 
> 
> On 11/17/2017 2:35 AM, Stefan Wahren wrote:
> > Hi Eric,
> > 
> >> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> hat am 31. Oktober 2017 um 09:43 geschrieben:
> >>
> >>
> >> Hi Eric,
> >>
> >>> Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> hat am 31. Oktober 2017 um 01:40 geschrieben:
> >>>
> >>>
> >>> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> writes:
> >>>
> >>>>> Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org> hat am 7. Oktober 2017 um 12:16 geschrieben:
> >>>>>
> >>>>>
> >>>>> In case the RPi Zero has at least a device connected to the OTG port
> >>>>> at boot time, the upper limit of tx fifo size for endpoint 6 and 7 is
> >>>>> also reduced to 512 bytes. So fix this accordingly.
> >>>>>
> >>>>> Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
> >>>>> Fixes: 1aa1d858f582 ("ARM: dts: bcm283x: Add dtsi for OTG mode")
> >>>>
> >>>> gentle ping ...
> >>>
> >>> I've tried to make sense of this a couple of times, but I don't get it:
> >>> why does EP 6/7 get reduced to 512 bytes in this case?
> >>
> >> i cannot give you an answer for this specific case.
> >>
> >> Since the dwc2 databook isn't public, i started a thread on linux-usb [1] about proper fifo size configuration. But i didn't get any reply.
> >>
> >> The problem here is there different contraints:
> >> * the sum of all fifo values must not exceed 3776 bytes
> >> * each slot have its individual upper limit (available in the BCM2835 datasheet)
> >>
> >> During my tests for OTG mode i missed the specific case above. Now my determined limits of 512 for EP 6 and 7 are contrary to the BCM2835 datasheet. Maybe the Synopsys guys have an answer?
> >>
> >> Btw the values in the downstream tree also violate the contraints.
> >>
> >> [1] - https://urldefense.proofpoint.com/v2/url?u=https-3A__www.spinics.net_lists_linux-2Dusb_msg157200.html&d=DwICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=6z9Al9FrHR_ZqbbtSAsD16pvOL2S3XHxQnSzq8kusyI&m=O2ngUGajgx3MfHk4IQKp5iRlwOOpG62gkji4R2gE64k&s=8ClOBwWN5B69mL1TDjnN3l78JBrvd1YHunRdzei-xrA&e=
> > 
> > still concerns about this patch, because it's not included in dt-fixes?
> > 
> 
> Hi Stefan,
> 
> According BCM2835 datasheet Total Data FIFO RAM Depth is 4096. I assume 
> that in datasheet assumed 4096 dwords, not a bytes. DFIFO depth in 
> dwords stored GHWCFG3[31:16].
> 
> So, for buffer DMA mode max space in dwords which available to allocate 
> to FIFO's is 4096-8*2=4080, where 8 EP count including EP0 and 2 for 
> both directions of EP's. Based on bcm283x-rpi-usb-otg.dtsi:
> g-rx-fifo-size + g-np-tx-fifo-size + g-tx-fifo-size[1..7] = 
> 256+32+256+256+512+512+512+768+768 = 3872 < 4080.
> So, I don't see any reason to change EP 6,7 TxFIFO sizes.

that's a nice calculation, but not relevant in my case.

Let's step back to the original problem. If i try to boot the Raspberry Pi Zero (Linux 4.14, U-Boot 2017-07) in OTG mode with at least 1 USB device connected, i will get the following warnings:

[    2.997476] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter g_tx_fifo_size[6]=768
[    3.015754] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter g_tx_fifo_size[7]=768

These warnings doesn't appear if there are no devices connected during boot.

In order to understand how i came to the "real" constraints 3776 bytes and to the new values for EP 6,7 i add some debug:

--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -462,6 +462,8 @@ static void dwc2_check_param_tx_fifo_sizes(struct dwc2_hsotg *hsotg)
 	for (fifo = 1; fifo <= fifo_count; fifo++)
 		total += hsotg->params.g_tx_fifo_size[fifo];
 
+	dev_warn(hsotg->dev, "%s: g-tx-fifo-size: %d\n", __func__, dwc2_hsotg_tx_fifo_total_depth(hsotg));
+
 	if (total > dwc2_hsotg_tx_fifo_total_depth(hsotg) || !total) {
 		dev_warn(hsotg->dev, "%s: Invalid parameter g-tx-fifo-size, setting to default average\n",
 			 __func__);
@@ -472,6 +474,8 @@ static void dwc2_check_param_tx_fifo_sizes(struct dwc2_hsotg *hsotg)
 		dptxfszn = (dwc2_readl(hsotg->regs + DPTXFSIZN(fifo)) &
 			FIFOSIZE_DEPTH_MASK) >> FIFOSIZE_DEPTH_SHIFT;
 
+		dev_warn(hsotg->dev, "%s: g_tx_fifo_size[%d]=%d\n", __func__, fifo, dptxfszn);
+
 		if (hsotg->params.g_tx_fifo_size[fifo] < min ||
 		    hsotg->params.g_tx_fifo_size[fifo] >  dptxfszn) {
 			dev_warn(hsotg->dev, "%s: Invalid parameter g_tx_fifo_size[%d]=%d\n",

After applying the patch i will get the following output if 1 device is connected during boot:

[    2.939493] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g-tx-fifo-size: 3776
[    2.947650] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g_tx_fifo_size[1]=512
[    2.955972] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g_tx_fifo_size[2]=512
[    2.964294] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g_tx_fifo_size[3]=512
[    2.972630] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g_tx_fifo_size[4]=512
[    2.980949] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g_tx_fifo_size[5]=512
[    2.989174] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g_tx_fifo_size[6]=512
[    2.997476] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter g_tx_fifo_size[6]=768
[    3.007439] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: g_tx_fifo_size[7]=512
[    3.015754] dwc2 20980000.usb: dwc2_check_param_tx_fifo_sizes: Invalid parameter g_tx_fifo_size[7]=768
[    3.025763] dwc2 20980000.usb: EPs: 8, dedicated fifos, 4080 entries in SPRAM
[    3.034534] dwc2 20980000.usb: DWC OTG Controller
[    3.039802] dwc2 20980000.usb: new USB bus registered, assigned bus number 1
[    3.047399] dwc2 20980000.usb: irq 33, io mem 0x20980000

As can see from dwc2 perspective the sum of EP 1 - 7 must be 3776 bytes and the upper limits for EP 6 + 7 changed to 512 instead of 768. Datasheets are nice, but as long as they apply only in specific cases they aren't helpful.

This might be not the right fix, but as long as i didn't get any reply to my mails this is my best solution. Sorry but as BCM2835 maintainer the dwc2 driver is a real problem child to us.

Best regards
Stefan

> 
> More probably comment in dtsi not correct:
> "* fifo sizes shouldn't exceed 3776 bytes."
> Please check FIFO depth in GHWCFG3 and if it 4096 then update the comment.
> 
> Thanks,
> Minas
> 
> 
> 
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7
       [not found]                         ` <410670D7E743164D87FA6160E7907A560113A409A8-ouFQeoKRuQ8bQ7k9MBbv4fufCSb+aD3WLzEdoUbNIic@public.gmane.org>
@ 2017-11-20 12:48                           ` Stefan Wahren
  2017-11-21 12:02                             ` Minas Harutyunyan
  0 siblings, 1 reply; 10+ messages in thread
From: Stefan Wahren @ 2017-11-20 12:48 UTC (permalink / raw)
  To: Minas Harutyunyan
  Cc: John Youn, Eric Anholt, Phil Elwell,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Florian Fainelli,
	Mark Rutland, linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Minas,

Am 20.11.2017 um 12:59 schrieb Minas Harutyunyan:
> Hi Stefan,
> Looks like I know cause of issue... but I'm overloaded today and will able to check it tomorrow. Sorry for delay.

thanks for your reply. There is no need to hurry, because the merge
window isn't open yet, but i like to get this fixed for the next kernel
release.

I have the suspicion this is related to the fact that u-boot already
initialize the USB IP, before dwc2 driver is loaded.

Regards
Stefan

>
> Thanks,
> Minas
>

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7
  2017-11-20 12:48                           ` Stefan Wahren
@ 2017-11-21 12:02                             ` Minas Harutyunyan
       [not found]                               ` <410670D7E743164D87FA6160E7907A560113A41084-ouFQeoKRuQ8bQ7k9MBbv4fufCSb+aD3WLzEdoUbNIic@public.gmane.org>
  0 siblings, 1 reply; 10+ messages in thread
From: Minas Harutyunyan @ 2017-11-21 12:02 UTC (permalink / raw)
  To: Stefan Wahren, Minas Harutyunyan
  Cc: John Youn, Eric Anholt, Phil Elwell,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Florian Fainelli,
	Mark Rutland, linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

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

On 11/20/2017 4:48 PM, Stefan Wahren wrote:
> Hi Minas,
> 
> Am 20.11.2017 um 12:59 schrieb Minas Harutyunyan:
>> Hi Stefan,
>> Looks like I know cause of issue... but I'm overloaded today and will able to check it tomorrow. Sorry for delay.
> 
> thanks for your reply. There is no need to hurry, because the merge
> window isn't open yet, but i like to get this fixed for the next kernel
> release.
> 
> I have the suspicion this is related to the fact that u-boot already
> initialize the USB IP, before dwc2 driver is loaded.
> 
> Regards
> Stefan
> 
>>
>> Thanks,
>> Minas
>>
> 
> 
Hi Stefan,

We have prepared patch for this issue in July-August'17.
Find attached 2 patch files. Please apply patches and test. If issue 
gone, we will send these patches to LKML by regular flow.

Thanks,
Minas


[-- Attachment #2: 8eff278.diff --]
[-- Type: text/plain, Size: 2553 bytes --]

From 8eff278552aaf248161df9af04f8b93f03526bcd Mon Sep 17 00:00:00 2001
From: Gevorg Sahakyan <sahakyan@synopsys.com>
Date: Mon, 31 Jul 2017 17:19:51 +0400
Subject: [PATCH] usb: dwc2: Fix TxFIFO setup issue

In host mode reading from DPTXSIZn returning invalid value(0) in
dwc2_check_param_tx_fifo_sizes function.

Added g_tx_fifo_size array in dwc2_hw_params structure in which stored
power on reset valus of DPTXSIZn registers in device mode (forced to
device).

Updated dwc2_get_hwparams function to write DPTXFSIZn to array.

Modyfied dwc2_check_param_tx_fifo_sizes function accordingly.

Change-Id: I61d3db753b1bc06f0f2caf40df350a09655f18fd
Signed-off-by: Gevorg Sahakyan <sahakyan@synopsys.com>
---

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 5029dde..3b71b49 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -533,6 +533,7 @@
 	unsigned utmi_phy_data_width:2;
 	u32 snpsid;
 	u32 dev_ep_dirs;
+	u32 g_tx_fifo_size[MAX_EPS_CHANNELS];
 };
 
 /* Size of control and EP0 buffers */
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index 701516e..79080a9 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -452,7 +452,6 @@
 	int fifo;
 	int min;
 	u32 total = 0;
-	u32 dptxfszn;
 
 	fifo_count = dwc2_hsotg_tx_fifo_count(hsotg);
 	min = hsotg->hw_params.en_multiple_tx_fifo ? 16 : 4;
@@ -467,15 +466,15 @@
 	}
 
 	for (fifo = 1; fifo <= fifo_count; fifo++) {
-		dptxfszn = (dwc2_readl(hsotg, DPTXFSIZN(fifo)) &
-			FIFOSIZE_DEPTH_MASK) >> FIFOSIZE_DEPTH_SHIFT;
 
 		if (hsotg->params.g_tx_fifo_size[fifo] < min ||
-		    hsotg->params.g_tx_fifo_size[fifo] >  dptxfszn) {
+		    hsotg->params.g_tx_fifo_size[fifo] >
+		    hsotg->hw_params.g_tx_fifo_size[fifo]) {
 			dev_warn(hsotg->dev, "%s: Invalid parameter g_tx_fifo_size[%d]=%d\n",
 				 __func__, fifo,
 				 hsotg->params.g_tx_fifo_size[fifo]);
-			hsotg->params.g_tx_fifo_size[fifo] = dptxfszn;
+			hsotg->params.g_tx_fifo_size[fifo] =
+		    hsotg->hw_params.g_tx_fifo_size[fifo];
 		}
 	}
 }
@@ -607,6 +606,7 @@
 {
 	struct dwc2_hw_params *hw = &hsotg->hw_params;
 	unsigned int width;
+	int fifo, fifo_count;
 	u32 hwcfg1, hwcfg2, hwcfg3, hwcfg4;
 	u32 grxfsiz;
 
@@ -675,6 +675,12 @@
 	hw->rx_fifo_size = (grxfsiz & GRXFSIZ_DEPTH_MASK) >>
 				GRXFSIZ_DEPTH_SHIFT;
 
+	fifo_count = dwc2_hsotg_tx_fifo_count(hsotg);
+
+	for (fifo = 1; fifo <= fifo_count; fifo++) {
+		hw->g_tx_fifo_size[fifo] = (dwc2_readl(hsotg, DPTXFSIZN(fifo)) &
+			FIFOSIZE_DEPTH_MASK) >> FIFOSIZE_DEPTH_SHIFT;
+	}
 	return 0;
 }
 

[-- Attachment #3: c6f7e1c.diff --]
[-- Type: text/plain, Size: 2138 bytes --]

From c6f7e1c790c4ef30ba818b119a177fdcd9ca7b34 Mon Sep 17 00:00:00 2001
From: Gevorg Sahakyan <sahakyan@synopsys.com>
Date: Tue, 08 Aug 2017 15:45:32 +0400
Subject: [PATCH] usb: dwc2: Fix tx_fifo_total_depth calculation

Removed ep_info subtraction during calculation tx_addr_max in
dwc2_hsotg_tx_fifo_total_depth function, because its already
done in hardware.

Also removed dwc2_hsotg_ep_info_size function as no more need.

Change-Id: If9a9f8ab115a6e998736ab991056f374eab9f747
Signed-off-by: Gevorg Sahakyan <sahakyan@synopsys.com>
---

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index 98a4a79..3c52f46 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -206,47 +206,11 @@
 }
 
 /**
- * dwc2_hsotg_ep_info_size - return Endpoint Info Control block size in DWORDs
- */
-static int dwc2_hsotg_ep_info_size(struct dwc2_hsotg *hsotg)
-{
-	int val = 0;
-	int i;
-	u32 ep_dirs;
-
-	/*
-	 * Don't need additional space for ep info control registers in
-	 * slave mode.
-	 */
-	if (!using_dma(hsotg)) {
-		dev_dbg(hsotg->dev, "Buffer DMA ep info size 0\n");
-		return 0;
-	}
-
-	/*
-	 * Buffer DMA mode - 1 location per endpoit
-	 * Descriptor DMA mode - 4 locations per endpoint
-	 */
-	ep_dirs = hsotg->hw_params.dev_ep_dirs;
-
-	for (i = 0; i <= hsotg->hw_params.num_dev_ep; i++) {
-		val += ep_dirs & 3 ? 1 : 2;
-		ep_dirs >>= 2;
-	}
-
-	if (using_desc_dma(hsotg))
-		val = val * 4;
-
-	return val;
-}
-
-/**
  * dwc2_hsotg_tx_fifo_total_depth - return total FIFO depth available for
  * device mode TX FIFOs
  */
 int dwc2_hsotg_tx_fifo_total_depth(struct dwc2_hsotg *hsotg)
 {
-	int ep_info_size;
 	int addr;
 	int tx_addr_max;
 	u32 np_tx_fifo_size;
@@ -254,9 +218,7 @@
 	np_tx_fifo_size = min_t(u32, hsotg->hw_params.dev_nperio_tx_fifo_size,
 				hsotg->params.g_np_tx_fifo_size);
 
-	/* Get Endpoint Info Control block size in DWORDs. */
-	ep_info_size = dwc2_hsotg_ep_info_size(hsotg);
-	tx_addr_max = hsotg->hw_params.total_fifo_size - ep_info_size;
+	tx_addr_max = hsotg->hw_params.total_fifo_size;
 
 	addr = hsotg->params.g_rx_fifo_size + np_tx_fifo_size;
 	if (tx_addr_max <= addr)

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7
       [not found]                               ` <410670D7E743164D87FA6160E7907A560113A41084-ouFQeoKRuQ8bQ7k9MBbv4fufCSb+aD3WLzEdoUbNIic@public.gmane.org>
@ 2017-11-22 11:21                                 ` Stefan Wahren
  0 siblings, 0 replies; 10+ messages in thread
From: Stefan Wahren @ 2017-11-22 11:21 UTC (permalink / raw)
  To: Minas Harutyunyan
  Cc: Phil Elwell, John Youn, Eric Anholt, Rob Herring,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Florian Fainelli, Mark Rutland,
	linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	devicetree-u79uwXL29TY76Z2rM5mHXA

Hi Minas,

> Minas Harutyunyan <Minas.Harutyunyan-HKixBCOQz3hWk0Htik3J/w@public.gmane.org> hat am 21. November 2017 um 13:02 geschrieben:
>
> Hi Stefan,
> 
> We have prepared patch for this issue in July-August'17.
> Find attached 2 patch files. Please apply patches and test. If issue 
> gone, we will send these patches to LKML by regular flow.

thanks, but the first patch doesn't apply. My version see below, i hope i didn't break anything.

Unfortunately after applying both patches the issue still persists (EP 1-7 fifo size to 512) and the EP 1-7 TX total size increases from 3776 to 3792. I will follow my u-boot theory  ...

>From 12fcc090bc7588275c1d942009676cb3fa5129f2 Mon Sep 17 00:00:00 2001
From: Gevorg Sahakyan <sahakyan-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Date: Wed, 22 Nov 2017 11:15:16 +0100
Subject: [PATCH] usb: dwc2: Fix TxFIFO setup issue

In host mode reading from DPTXSIZn returning invalid value(0) in
dwc2_check_param_tx_fifo_sizes function.

Added g_tx_fifo_size array in dwc2_hw_params structure in which stored
power on reset valus of DPTXSIZn registers in device mode (forced to
device).

Updated dwc2_get_hwparams function to write DPTXFSIZn to array.

Modyfied dwc2_check_param_tx_fifo_sizes function accordingly.

Change-Id: I61d3db753b1bc06f0f2caf40df350a09655f18fd
Signed-off-by: Gevorg Sahakyan <sahakyan-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
Signed-off-by: Stefan Wahren <stefan.wahren-eS4NqCHxEME@public.gmane.org>
---
 drivers/usb/dwc2/core.h   |  1 +
 drivers/usb/dwc2/params.c | 11 +++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
index 8367d4f..47e9092 100644
--- a/drivers/usb/dwc2/core.h
+++ b/drivers/usb/dwc2/core.h
@@ -587,6 +587,7 @@ struct dwc2_hw_params {
 	unsigned utmi_phy_data_width:2;
 	u32 snpsid;
 	u32 dev_ep_dirs;
+	u32 g_tx_fifo_size[MAX_EPS_CHANNELS];
 };
 
 /* Size of control and EP0 buffers */
diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
index a3ffe97..04f1868 100644
--- a/drivers/usb/dwc2/params.c
+++ b/drivers/usb/dwc2/params.c
@@ -469,8 +469,7 @@ static void dwc2_check_param_tx_fifo_sizes(struct dwc2_hsotg *hsotg)
 	}
 
 	for (fifo = 1; fifo <= fifo_count; fifo++) {
-		dptxfszn = (dwc2_readl(hsotg->regs + DPTXFSIZN(fifo)) &
-			FIFOSIZE_DEPTH_MASK) >> FIFOSIZE_DEPTH_SHIFT;
+		dptxfszn = hsotg->hw_params.g_tx_fifo_size[fifo];
 
 		if (hsotg->params.g_tx_fifo_size[fifo] < min ||
 		    hsotg->params.g_tx_fifo_size[fifo] >  dptxfszn) {
@@ -617,6 +616,7 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
 {
 	struct dwc2_hw_params *hw = &hsotg->hw_params;
 	unsigned int width;
+	int fifo, fifo_count;
 	u32 hwcfg1, hwcfg2, hwcfg3, hwcfg4;
 	u32 grxfsiz;
 
@@ -705,6 +705,13 @@ int dwc2_get_hwparams(struct dwc2_hsotg *hsotg)
 	hw->rx_fifo_size = (grxfsiz & GRXFSIZ_DEPTH_MASK) >>
 				GRXFSIZ_DEPTH_SHIFT;
 
+	fifo_count = dwc2_hsotg_tx_fifo_count(hsotg);
+
+	for (fifo = 1; fifo <= fifo_count; fifo++) {
+		u32 val = dwc2_readl(hsotg->regs + DPTXFSIZN(fifo));
+		hw->g_tx_fifo_size[fifo] = (val & FIFOSIZE_DEPTH_MASK) >>
+					   FIFOSIZE_DEPTH_SHIFT;
+	}
 	return 0;
 }
 
-- 
2.7.4
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-11-22 11:21 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-07 10:16 [PATCH] ARM: dts: bcm283x: Fix fifo size for EP 6,7 Stefan Wahren
     [not found] ` <1507371414-21688-1-git-send-email-stefan.wahren-eS4NqCHxEME@public.gmane.org>
2017-10-27 20:58   ` Stefan Wahren
     [not found]     ` <160210128.18582.1509137904239-7tX72C7vayboQLBSYMtkGA@public.gmane.org>
2017-10-31  0:40       ` Eric Anholt
     [not found]         ` <871slkw3ms.fsf-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
2017-10-31  8:43           ` Stefan Wahren
     [not found]             ` <1506055926.93787.1509439381859-7tX72C7vayboQLBSYMtkGA@public.gmane.org>
2017-11-16 22:33               ` Stefan Wahren
2017-11-17  7:48                 ` Minas Harutyunyan
     [not found]                   ` <410670D7E743164D87FA6160E7907A560113A3F8F0-ouFQeoKRuQ8bQ7k9MBbv4fufCSb+aD3WLzEdoUbNIic@public.gmane.org>
2017-11-18 20:21                     ` Stefan Wahren
     [not found]                       ` <410670D7E743164D87FA6160E7907A560113A409A8@am04wembxa.internal.synopsys.com>
     [not found]                         ` <410670D7E743164D87FA6160E7907A560113A409A8-ouFQeoKRuQ8bQ7k9MBbv4fufCSb+aD3WLzEdoUbNIic@public.gmane.org>
2017-11-20 12:48                           ` Stefan Wahren
2017-11-21 12:02                             ` Minas Harutyunyan
     [not found]                               ` <410670D7E743164D87FA6160E7907A560113A41084-ouFQeoKRuQ8bQ7k9MBbv4fufCSb+aD3WLzEdoUbNIic@public.gmane.org>
2017-11-22 11:21                                 ` Stefan Wahren

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.