[PATCH RFT] usb: dwc2: gadget: Fix fifo size configuration

[PATCH RFT] usb: dwc2: gadget: Fix fifo size configuration

[Stefan Wahren]

Currently the upper limit for the endpoint index during fifo size
config was always 16 instead of the available endpoints. So fix this
by using the determined amount of endpoints and avoid a warning about
"insufficient fifo memory" on bcm2835 which has only 8 endpoints.

Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
Fixes: 0a176279db68 ("usb: dwc2: gadget: configure fifos from device tree")
---
 drivers/usb/dwc2/gadget.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Since revert of ("usb: dwc2: gadget: fix TX FIFO size and address") which
caused regressions on some platforms this is the second attempt to fix
gadget support for bcm2835.

diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
index b95930f..b00184c 100644
--- a/drivers/usb/dwc2/gadget.c
+++ b/drivers/usb/dwc2/gadget.c
@@ -228,7 +228,7 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
 	 * them to endpoints dynamically according to maxpacket size value of
 	 * given endpoint.
 	 */
-	for (ep = 1; ep < MAX_EPS_CHANNELS; ep++) {
+	for (ep = 1; ep < hsotg->num_of_eps; ep++) {
 		if (!txfsz[ep])
 			continue;
 		val = addr;
-- 
1.7.9.5

Re: [PATCH RFT] usb: dwc2: gadget: Fix fifo size configuration

[Sevak Arakelyan]

On 12/19/2016 8:39 PM, Stefan Wahren wrote:
> Currently the upper limit for the endpoint index during fifo size
> config was always 16 instead of the available endpoints. So fix this
> by using the determined amount of endpoints and avoid a warning about
> "insufficient fifo memory" on bcm2835 which has only 8 endpoints.
>
> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> Fixes: 0a176279db68 ("usb: dwc2: gadget: configure fifos from device tree")
> ---
>  drivers/usb/dwc2/gadget.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Since revert of ("usb: dwc2: gadget: fix TX FIFO size and address") which
> caused regressions on some platforms this is the second attempt to fix
> gadget support for bcm2835.
>
> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> index b95930f..b00184c 100644
> --- a/drivers/usb/dwc2/gadget.c
> +++ b/drivers/usb/dwc2/gadget.c
> @@ -228,7 +228,7 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
>  	 * them to endpoints dynamically according to maxpacket size value of
>  	 * given endpoint.
>  	 */
> -	for (ep = 1; ep < MAX_EPS_CHANNELS; ep++) {
> +	for (ep = 1; ep < hsotg->num_of_eps; ep++) {
>  		if (!txfsz[ep])
>  			continue;
>  		val = addr;

Hi Stefan,

    This patch is only temporary, not full solution for bcm2835 and in
the loop you need to use hsotg->num_of_eps in shared FIFO mode and the
number of device IN EPs which is in GHWCFG4 with mask
GHWCFG4_NUM_IN_EPS_MASK in dedicated FIFO mode.

    The upper limit is up to 15 but the actual number of FIFOa are being
configured according to the given legacy array
DWC2_G_P_LEGACY_TX_FIFO_SIZE {256, 256, 256, 256, 768, 768, 768, 768, 0,
0, 0, 0, 0, 0, 0}, in that case it is 8 FIFOs. The legacy values are the
main issue. We must do correct calculation according to FIFO operation
mode, device operation mode, take into consideration the EP Info Block
which must be at the end of total FIFO and after that configure correct
number of FIFOs with correct size of depth on every setup. We prepared a
patch series to fix that issue and it is under internal review and
testing process and will be submitted soon.
  
Thanks,
Sevak

Re: [PATCH RFT] usb: dwc2: gadget: Fix fifo size configuration

[Stefan Wahren]

Hi Sevak,

> Sevak Arakelyan <SEVAK.ARAKELYAN@synopsys.com> hat am 20. Dezember 2016 um 13:57 geschrieben:
> 
> 
> On 12/19/2016 8:39 PM, Stefan Wahren wrote:
> > Currently the upper limit for the endpoint index during fifo size
> > config was always 16 instead of the available endpoints. So fix this
> > by using the determined amount of endpoints and avoid a warning about
> > "insufficient fifo memory" on bcm2835 which has only 8 endpoints.
> >
> > Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
> > Fixes: 0a176279db68 ("usb: dwc2: gadget: configure fifos from device tree")
> > ---
> >  drivers/usb/dwc2/gadget.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Since revert of ("usb: dwc2: gadget: fix TX FIFO size and address") which
> > caused regressions on some platforms this is the second attempt to fix
> > gadget support for bcm2835.
> >
> > diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
> > index b95930f..b00184c 100644
> > --- a/drivers/usb/dwc2/gadget.c
> > +++ b/drivers/usb/dwc2/gadget.c
> > @@ -228,7 +228,7 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
> >  	 * them to endpoints dynamically according to maxpacket size value of
> >  	 * given endpoint.
> >  	 */
> > -	for (ep = 1; ep < MAX_EPS_CHANNELS; ep++) {
> > +	for (ep = 1; ep < hsotg->num_of_eps; ep++) {
> >  		if (!txfsz[ep])
> >  			continue;
> >  		val = addr;
> 
> Hi Stefan,
> 
>     This patch is only temporary, not full solution for bcm2835 and in
> the loop you need to use hsotg->num_of_eps in shared FIFO mode and the
> number of device IN EPs which is in GHWCFG4 with mask
> GHWCFG4_NUM_IN_EPS_MASK in dedicated FIFO mode.

i don't have any documentation for this. So i tried to fix this obviously error.

> 
>     The upper limit is up to 15 but the actual number of FIFOa are being
> configured according to the given legacy array
> DWC2_G_P_LEGACY_TX_FIFO_SIZE {256, 256, 256, 256, 768, 768, 768, 768, 0,
> 0, 0, 0, 0, 0, 0}, in that case it is 8 FIFOs. The legacy values are the
> main issue. We must do correct calculation according to FIFO operation
> mode, device operation mode, take into consideration the EP Info Block
> which must be at the end of total FIFO and after that configure correct
> number of FIFOs with correct size of depth on every setup. We prepared a
> patch series to fix that issue and it is under internal review and
> testing process and will be submitted soon.

I wanted to avoid this by providing the TX FIFO sizes via DT [1].

Sorry for my impatience.

[1] - https://github.com/lategoodbye/rpi-zero/commit/a6ec75dd8ea6a7ce0609ae082d3fa748beaba22c

>   
> Thanks,
> Sevak
>

Re: [PATCH RFT] usb: dwc2: gadget: Fix fifo size configuration

[Sevak Arakelyan]

On 12/20/2016 6:20 PM, Stefan Wahren wrote:
> Hi Sevak,
>
>> Sevak Arakelyan <SEVAK.ARAKELYAN@synopsys.com> hat am 20. Dezember 2016 um 13:57 geschrieben:
>>
>>
>> On 12/19/2016 8:39 PM, Stefan Wahren wrote:
>>> Currently the upper limit for the endpoint index during fifo size
>>> config was always 16 instead of the available endpoints. So fix this
>>> by using the determined amount of endpoints and avoid a warning about
>>> "insufficient fifo memory" on bcm2835 which has only 8 endpoints.
>>>
>>> Signed-off-by: Stefan Wahren <stefan.wahren@i2se.com>
>>> Fixes: 0a176279db68 ("usb: dwc2: gadget: configure fifos from device tree")
>>> ---
>>>  drivers/usb/dwc2/gadget.c |    2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> Since revert of ("usb: dwc2: gadget: fix TX FIFO size and address") which
>>> caused regressions on some platforms this is the second attempt to fix
>>> gadget support for bcm2835.
>>>
>>> diff --git a/drivers/usb/dwc2/gadget.c b/drivers/usb/dwc2/gadget.c
>>> index b95930f..b00184c 100644
>>> --- a/drivers/usb/dwc2/gadget.c
>>> +++ b/drivers/usb/dwc2/gadget.c
>>> @@ -228,7 +228,7 @@ static void dwc2_hsotg_init_fifo(struct dwc2_hsotg *hsotg)
>>>  	 * them to endpoints dynamically according to maxpacket size value of
>>>  	 * given endpoint.
>>>  	 */
>>> -	for (ep = 1; ep < MAX_EPS_CHANNELS; ep++) {
>>> +	for (ep = 1; ep < hsotg->num_of_eps; ep++) {
>>>  		if (!txfsz[ep])
>>>  			continue;
>>>  		val = addr;
>> Hi Stefan,
>>
>>     This patch is only temporary, not full solution for bcm2835 and in
>> the loop you need to use hsotg->num_of_eps in shared FIFO mode and the
>> number of device IN EPs which is in GHWCFG4 with mask
>> GHWCFG4_NUM_IN_EPS_MASK in dedicated FIFO mode.
> i don't have any documentation for this. So i tried to fix this obviously error.
>
>>     The upper limit is up to 15 but the actual number of FIFOa are being
>> configured according to the given legacy array
>> DWC2_G_P_LEGACY_TX_FIFO_SIZE {256, 256, 256, 256, 768, 768, 768, 768, 0,
>> 0, 0, 0, 0, 0, 0}, in that case it is 8 FIFOs. The legacy values are the
>> main issue. We must do correct calculation according to FIFO operation
>> mode, device operation mode, take into consideration the EP Info Block
>> which must be at the end of total FIFO and after that configure correct
>> number of FIFOs with correct size of depth on every setup. We prepared a
>> patch series to fix that issue and it is under internal review and
>> testing process and will be submitted soon.
> I wanted to avoid this by providing the TX FIFO sizes via DT [1].
>
> Sorry for my impatience.
>
> [1] - https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_lategoodbye_rpi-2Dzero_commit_a6ec75dd8ea6a7ce0609ae082d3fa748beaba22c&d=DgICaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=SniH1MMI5_rJOm61HpL5IYZquCv0UB108rKN0g5Qj8Q&m=ENp09nlehChUcfWd9a_imoGZ2oImaklDC0eqqLbhXIs&s=-Sn2hwPd6iMDrb8hFHDiXURw62nYhMi5CY4oW8h1w94&e= 
>
>>   
>> Thanks,
>> Sevak
>>
The upper described flow is only for setting default FIFO depths instead
of legacy values. It is dividing available total FIFO depth between TX
FIFOs, but if you want to give your own depths you can give them via DT,
updated flow only will do validation of given DT values and will set to
default ones only if the given ones are getting out of FIFO bounds. So
you can give your calculated TX FIFO sizes via DT if our patch series
will not fix the existing issue on bcm2835.