netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Grygorii Strashko <grygorii.strashko@ti.com>
To: Ivan Khoronzhuk <ivan.khoronzhuk@linaro.org>
Cc: "David S. Miller" <davem@davemloft.net>, <netdev@vger.kernel.org>,
	Mugunthan V N <mugunthanvnm@ti.com>, Sekhar Nori <nsekhar@ti.com>,
	<linux-kernel@vger.kernel.org>, <linux-omap@vger.kernel.org>
Subject: Re: [PATCH 5/7] Documentation: DT: net: cpsw: allow to specify descriptors pool size
Date: Fri, 2 Dec 2016 11:22:28 -0600	[thread overview]
Message-ID: <31a048c2-74e4-772e-ab63-9eacc2f49953@ti.com> (raw)
In-Reply-To: <20161202112832.GB1213@khorivan>



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. in general this is expected to be 
 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?



-- 
regards,
-grygorii

  parent reply	other threads:[~2016-12-02 17:22 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-12-01 23:34 [PATCH 0/7] net: ethernet: ti: cpsw: support placing CPDMA descriptors into DDR Grygorii Strashko
2016-12-01 23:34 ` [PATCH 1/7] net: ethernet: ti: cpdma: am437x: allow descs to be plased in ddr Grygorii Strashko
2016-12-03 20:34   ` David Miller
2016-12-05 17:57     ` Grygorii Strashko
2016-12-01 23:34 ` [PATCH 2/7] net: ethernet: ti: cpdma: fix desc re-queuing Grygorii Strashko
2016-12-02 11:03   ` Ivan Khoronzhuk
2016-12-02 16:45     ` Grygorii Strashko
2016-12-02 23:28       ` Ivan Khoronzhuk
2016-12-05 18:22         ` Grygorii Strashko
2016-12-01 23:34 ` [PATCH 3/7] net: ethernet: ti: cpdma: minimize number of parameters in cpdma_desc_pool_create/destroy() Grygorii Strashko
2016-12-01 23:34 ` [PATCH 4/7] net: ethernet: ti: cpdma: use devm_ioremap Grygorii Strashko
2016-12-01 23:34 ` [PATCH 5/7] Documentation: DT: net: cpsw: allow to specify descriptors pool size Grygorii Strashko
2016-12-02 11:28   ` Ivan Khoronzhuk
2016-12-02 17:21     ` Grygorii Strashko
2016-12-07 19:41       ` Grygorii Strashko
2016-12-02 17:22     ` Grygorii Strashko [this message]
2016-12-03  0:37       ` Ivan Khoronzhuk
2016-12-01 23:34 ` [PATCH 6/7] net: ethernet: ti: cpsw: add support for descs_pool_size dt property Grygorii Strashko
2016-12-01 23:34 ` [PATCH 7/7] Documentation: DT: net: cpsw: remove no_bd_ram property Grygorii Strashko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=31a048c2-74e4-772e-ab63-9eacc2f49953@ti.com \
    --to=grygorii.strashko@ti.com \
    --cc=davem@davemloft.net \
    --cc=ivan.khoronzhuk@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=mugunthanvnm@ti.com \
    --cc=netdev@vger.kernel.org \
    --cc=nsekhar@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).