Re: [PATCH] arm64: dts: hi6220: improve g-tx-fifo-size setting for usb device

2017-08-15 Thread Wei Xu 
Hi Shawn, John,

On 2017/8/7 22:18, John Stultz wrote:
> On Sun, Aug 6, 2017 at 10:01 PM, Shawn Guo  wrote:
>> From: Shawn Guo 
>>
>> The current usb device g-tx-fifo-size setting in DT causes two problems
>> for kernel driver.
>>
>> 1. On hi6220, there are 15 tx_fifo dedicated for all EPs except EP0,
>>while DT only provides tx_fifo settings for 6 EPs.  It results in the
>>following annoying complaints from kernel.
>>
>> [4.451623] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[7]=0
>> [4.461303] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[8]=0
>> [4.470969] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[9]=0
>> [4.480632] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[10]=0
>> [4.490385] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[11]=0
>> [4.500140] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[12]=0
>> [4.509892] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[13]=0
>> [4.519646] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[14]=0
>> [4.529399] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
>> parameter g_tx_fifo_size[15]=0
>> [4.539244] dwc2 f72c.usb: EPs: 16, dedicated fifos, 1920 entries in 
>> SPRAM
>>
>>Besides of that, the total 1920 fifo entries isn't fully utilized.
>>Endpoint Info Control block consumes 128 entries, g-rx-fifo-size
>>is 512, and g-np-tx-fifo-size is 128.  So the fifi entries available
>>for tx_fifo is: 1920 - 128 - 512 - 128 = 1152.  Considering that
>>the minimal valid tx_fifo size for each EP is 16, it should be
>>reasonable to allocate 1152 entries as: 128 x 8 + 16 x 7 = 1136 (only
>>16 entries unused).  With this new setting, we can get more EPs to
>>use while removing the above warning messages in the meantime.
>>
>> 2. Another consequence of above invalid g_tx_fifo_size parameter is that
>>kernel driver will use values read from hardware register as the
>>fall-back.  The value is 2048 for each EP fifo.  That's obviously
>>invalid either, because even fifo entries for one EP exceeds the
>>total entries 1920.  That's why we see the following fat warning from
>>function dwc2_hsotg_init_fifo().  The new g-tx-fifo-size settings
>>help to remove the warning as well.
> 
> 
> Nice! This has been bothering me for awhile, and your fix seems to
> resolve it well! I can now remove one of the hack patches I've been
> carrying.
> 
> Tested-by: John Stultz 
> (if its useful: Acked-by: John Stultz )
> 
> Wei: Can we be sure to get this queued for 4.14?

Thanks!
Applied to the hisilicon dt tree.

Best Regards,
Wei

> 
> thanks
> -john
> 
> .
> 

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

Re: [PATCH] arm64: dts: hi6220: improve g-tx-fifo-size setting for usb device

2017-08-07 Thread John Stultz 
On Sun, Aug 6, 2017 at 10:01 PM, Shawn Guo  wrote:
> From: Shawn Guo 
>
> The current usb device g-tx-fifo-size setting in DT causes two problems
> for kernel driver.
>
> 1. On hi6220, there are 15 tx_fifo dedicated for all EPs except EP0,
>while DT only provides tx_fifo settings for 6 EPs.  It results in the
>following annoying complaints from kernel.
>
> [4.451623] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
> parameter g_tx_fifo_size[7]=0
> [4.461303] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
> parameter g_tx_fifo_size[8]=0
> [4.470969] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
> parameter g_tx_fifo_size[9]=0
> [4.480632] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
> parameter g_tx_fifo_size[10]=0
> [4.490385] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
> parameter g_tx_fifo_size[11]=0
> [4.500140] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
> parameter g_tx_fifo_size[12]=0
> [4.509892] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
> parameter g_tx_fifo_size[13]=0
> [4.519646] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
> parameter g_tx_fifo_size[14]=0
> [4.529399] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
> parameter g_tx_fifo_size[15]=0
> [4.539244] dwc2 f72c.usb: EPs: 16, dedicated fifos, 1920 entries in 
> SPRAM
>
>Besides of that, the total 1920 fifo entries isn't fully utilized.
>Endpoint Info Control block consumes 128 entries, g-rx-fifo-size
>is 512, and g-np-tx-fifo-size is 128.  So the fifi entries available
>for tx_fifo is: 1920 - 128 - 512 - 128 = 1152.  Considering that
>the minimal valid tx_fifo size for each EP is 16, it should be
>reasonable to allocate 1152 entries as: 128 x 8 + 16 x 7 = 1136 (only
>16 entries unused).  With this new setting, we can get more EPs to
>use while removing the above warning messages in the meantime.
>
> 2. Another consequence of above invalid g_tx_fifo_size parameter is that
>kernel driver will use values read from hardware register as the
>fall-back.  The value is 2048 for each EP fifo.  That's obviously
>invalid either, because even fifo entries for one EP exceeds the
>total entries 1920.  That's why we see the following fat warning from
>function dwc2_hsotg_init_fifo().  The new g-tx-fifo-size settings
>help to remove the warning as well.


Nice! This has been bothering me for awhile, and your fix seems to
resolve it well! I can now remove one of the hack patches I've been
carrying.

Tested-by: John Stultz 
(if its useful: Acked-by: John Stultz )

Wei: Can we be sure to get this queued for 4.14?

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

[PATCH] arm64: dts: hi6220: improve g-tx-fifo-size setting for usb device

2017-08-06 Thread Shawn Guo 
From: Shawn Guo 

The current usb device g-tx-fifo-size setting in DT causes two problems
for kernel driver.

1. On hi6220, there are 15 tx_fifo dedicated for all EPs except EP0,
   while DT only provides tx_fifo settings for 6 EPs.  It results in the
   following annoying complaints from kernel.

[4.451623] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
parameter g_tx_fifo_size[7]=0
[4.461303] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
parameter g_tx_fifo_size[8]=0
[4.470969] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
parameter g_tx_fifo_size[9]=0
[4.480632] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
parameter g_tx_fifo_size[10]=0
[4.490385] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
parameter g_tx_fifo_size[11]=0
[4.500140] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
parameter g_tx_fifo_size[12]=0
[4.509892] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
parameter g_tx_fifo_size[13]=0
[4.519646] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
parameter g_tx_fifo_size[14]=0
[4.529399] dwc2 f72c.usb: dwc2_check_param_tx_fifo_sizes: Invalid 
parameter g_tx_fifo_size[15]=0
[4.539244] dwc2 f72c.usb: EPs: 16, dedicated fifos, 1920 entries in 
SPRAM

   Besides of that, the total 1920 fifo entries isn't fully utilized.
   Endpoint Info Control block consumes 128 entries, g-rx-fifo-size
   is 512, and g-np-tx-fifo-size is 128.  So the fifi entries available
   for tx_fifo is: 1920 - 128 - 512 - 128 = 1152.  Considering that
   the minimal valid tx_fifo size for each EP is 16, it should be
   reasonable to allocate 1152 entries as: 128 x 8 + 16 x 7 = 1136 (only
   16 entries unused).  With this new setting, we can get more EPs to
   use while removing the above warning messages in the meantime.

2. Another consequence of above invalid g_tx_fifo_size parameter is that
   kernel driver will use values read from hardware register as the
   fall-back.  The value is 2048 for each EP fifo.  That's obviously
   invalid either, because even fifo entries for one EP exceeds the
   total entries 1920.  That's why we see the following fat warning from
   function dwc2_hsotg_init_fifo().  The new g-tx-fifo-size settings
   help to remove the warning as well.

[   65.431634] dwc2 f72c.usb: Do port resume before switching to device mode
[   65.624176] insufficient fifo memory
[   65.624369] [ cut here ]
[   65.633633] WARNING: CPU: 0 PID: 5 at drivers/usb/dwc2/gadget.c:330 
dwc2_hsotg_init_fifo+0x164/0x1ac
[   65.643808] CPU: 0 PID: 5 Comm: kworker/u16:0 Not tainted 
4.13.0-rc1-00022-g50861cf9dc1b-dirty #81
[   65.653769] Hardware name: HiKey Development Board (DT)
[   65.659624] Workqueue: dwc2 dwc2_conn_id_status_change
[   65.665377] task: ffc005f73400 task.stack: ffc005f98000
[   65.671987] PC is at dwc2_hsotg_init_fifo+0x164/0x1ac
[   65.677633] LR is at dwc2_hsotg_init_fifo+0x164/0x1ac
[   65.683275] pc : [] lr : [] pstate: 
61c5
[   65.691504] sp : ffc005f9bce0
[   65.695218] x29: ffc005f9bce0 x28: ffc005f6ac00
[   65.701172] x27: ffc005f73400 x26: 08000580
[   65.707124] x25: ff8008bb4af0 x24: ff8008d02b70
[   65.713074] x23: 003fcc831084 x22: ffc0337cf0bc
[   65.719024] x21: 0580 x20: ffc0337cf018
[   65.724976] x19: ffc0337cf098 x18: 
[   65.730926] x17:  x16: 
[   65.736873] x15:  x14: ff8008ca8900
[   65.742825] x13: 004035299000 x12: 34d5d91d
[   65.748775] x11:  x10: 08d0
[   65.754726] x9 : ffc005f9bce0 x8 : 01b5
[   65.760674] x7 : 66696620746e6569 x6 : ff8008d60050
[   65.766623] x5 :  x4 : 
[   65.772573] x3 : 0002 x2 : 0002
[   65.778521] x1 : 0001 x0 : 0018
[   65.784469] Call trace:
[   65.787236] Exception stack(0xffc005f9bb10 to 0xffc005f9bc40)
[   65.794420] bb00:   ffc0337cf098 
0080
[   65.803145] bb20: ffc005f9bce0 ff8008638044 ff8008bb4af0 
08000580
[   65.811870] bb40: ffc005f73400 ffc005f6ac00  
ff8008da2998
[   65.820595] bb60: ffc005f9bce0 ffc005f9bce0 ffc005f9bca0 
ffc8
[   65.829315] bb80: ffc005f9bbb0 ff80081046a0 ffc005f9bce0 
ffc005f9bce0
[   65.838038] bba0: ffc005f9bca0 ffc8 0018 
0001
[   65.846761] bbc0: 0002 0002  

[   65.855485] bbe0: ff8008d60050 66696620746e6569 01b5 
ffc005f9bce0
[   65.864207] bc00: 08d0  34d5d91d 
004035299000
[   65.872928] bc20: ff8008ca8900