Re: [PATCH 5/7] Documentation: DT: net: cpsw: allow to specify descriptors pool size

Grygorii Strashko
2016-12-07T19:41:49Z

On 12/02/2016 11:21 AM, Grygorii Strashko wrote:
>
>
> On 12/02/2016 05:28 AM, Ivan Khoronzhuk wrote:
>> On Thu, Dec 01, 2016 at 05:34:30PM -0600, Grygorii Strashko wrote:
>>> Add optional property "descs_pool_size" to specify buffer descriptor's
>>> pool size. The "descs_pool_size" should define total number of CPDMA
>>> CPPI descriptors to be used for both ingress/egress packets
>>> processing. If not specified - the default value 256 will be used
>>> which will allow to place descriptor's pool into the internal CPPI
>>> RAM on most of TI SoC.
>>>
>>> Signed-off-by: Grygorii Strashko <grygorii.strashko@ti.com>
>>> ---
>>> Documentation/devicetree/bindings/net/cpsw.txt | 5 +++++
>>> 1 file changed, 5 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/net/cpsw.txt b/Documentation/devicetree/bindings/net/cpsw.txt
>>> index 5ad439f..b99d196 100644
>>> --- a/Documentation/devicetree/bindings/net/cpsw.txt
>>> +++ b/Documentation/devicetree/bindings/net/cpsw.txt
>>> @@ -35,6 +35,11 @@ Optional properties:
>>> For example in dra72x-evm, pcf gpio has to be
>>> driven low so that cpsw slave 0 and phy data
>>> lines are connected via mux.
>>> +- descs_pool_size : total number of CPDMA CPPI descriptors to be used for
>>> + both ingress/egress packets processing. if not
>>> + specified the default value 256 will be used which
>>> + will allow to place descriptors pool into the
>>> + internal CPPI RAM.
>> Does it describe h/w? Why now module parameter? or even smth like ethtool num
>> ring entries?
>>
>
> It can be module parameter too. for the use cases i'm aware of -
> this is one-time boot setting only.
>
> ----- OR
> So, do you propose to use
> ethtool -g ethX
>
> ethtool -G ethX [rx N] [tx N]
> ?
>
> Now cpdma has one pool for all RX/TX channels, so changing this settings
> by ethtool will require: pause interfaces, reallocate cpdma pool,
> re-arrange buffers between channels, resume interface. Correct?
>
> How do you think - we can move forward with one pool or better to have two (Rx and Tx)?
>
> Wouldn't it be reasonable to still have DT (or module) parameter to avoid
> cpdma reconfiguration on system startup (pause/resume interfaces) (faster boot)?
>
> How about cpdma re-allocation policy (with expectation that is shouldn't happen too often)?
> - increasing of Rx, Tx will grow total number of physically allocated buffers (total_desc_num)
> - decreasing of Rx, Tx will just change number of available buffers (no memory re-allocation)
>
> ----- OR ----
> Can we move forward with current patch (total number of CPDMA CPPI descriptors defined in DT)
> and add ethtool -G ethX [rx N] [tx N] which will allow to re-split descs between RX and TX?
>
>

if no comments here, I'll rework patches to use module parameter for descs_pool_size and
will add possibility to re-split RX/TX buffers using ethtool -G ethX [rx N] [tx N]

--
regards,
-grygorii