On 06/08/2016 09:06 AM, Ivan Khoronzhuk wrote:

On 08.06.16 17:01, Ivan Khoronzhuk wrote:

quoted

Hi Schuyer,

On 07.06.16 18:26, Schuyler Patton wrote:

quoted

Hi,

On 06/07/2016 08:59 AM, Ivan Khoronzhuk wrote:

quoted

There is no reason in rx_descs property because davinici_cpdma
driver splits pool of descriptors equally between tx and rx channels.
So, this patch series makes driver to use available number of
descriptors for rx channels.

I agree with the idea of consolidating how the descriptors are 
defined because of
the two variable components, number and size of the pool can be 
confusing to
end users. I would like to request to change how it is being 
proposed here.

I think the number of descriptors should be left in the device tree 
source file as
is and remove the BD size variable and have the driver calculate the 
size of the
pool necessary to support the descriptor request. From an user 
perspective it is
easier I think to be able to list the number of descriptors 
necessary vs. the size
of the pool.

Since the patch series points out how it is used so in the driver so 
to make that
consistent is perhaps change rx_descs to total_descs.

Regards,
Schuyler

The DT entry for cpsw doesn't have property for size of the pool.
It contains only BD ram size, if you mean this. The size of the pool is
software decision. Current version of DT entry contain only rx desc 
number.
That is not correct, as it depends on the size of the descriptor, 
which is also
h/w parameter. The DT entry has to describe only h/w part and 
shouldn't contain
driver implementation details, and I'm looking on it from this 
perspective.

Besides, rx_descs describes only rx number of descriptors, that are 
taken from
the same pool as tx descriptors, and setting rx desc to some new 
value doesn't
mean that rest of them are freed for tx. Also, I'm going to send 
series that
adds multi channel support to the driver, and in this case, splitting 
of the
pool will be more sophisticated than now, after what setting those 
parameters
for user (he should do this via device tree) can be even more 
confusing. But,

should -> shouldn't

quoted

as it's supposed, it's software decision that shouldn't leak to the DT.

If this rx-desc field is removed how will the number of descriptors be set?

This field has been used to increase the number of descriptors so high
volume short packets are not dropped due to descriptor exhaustion. The 
current
default number of 64 rx descriptors is too low for gigabit networks. 
Some users
have a strong requirement for zero loss of UDP packets setting this 
field to a
larger number and setting the descriptors off-chip was a means to solve
the problem.

quoted

quoted

quoted

Based on master branch

Since v1:
- separate device tree and driver patches
- return number of rx buffers from cpdma driver

Ivan Khoronzhuk (2):
   net: ethernet: ti: cpsw: remove rx_descs property
   Documentation: DT: cpsw: remove rx_descs property

  Documentation/devicetree/bindings/net/cpsw.txt |  1 -
  arch/arm/boot/dts/am33xx.dtsi                  |  1 -
  arch/arm/boot/dts/am4372.dtsi                  |  1 -
  arch/arm/boot/dts/dm814x.dtsi                  |  1 -
  arch/arm/boot/dts/dra7.dtsi                    |  1 -
  drivers/net/ethernet/ti/cpsw.c                 | 13 +++----------
  drivers/net/ethernet/ti/cpsw.h                 |  1 -
  drivers/net/ethernet/ti/davinci_cpdma.c        |  6 ++++++
  drivers/net/ethernet/ti/davinci_cpdma.h        |  1 +
  9 files changed, 10 insertions(+), 16 deletions(-)