Re: [PATCH net-next v2 4/9] dt-bindings: net: add OPEN Alliance 10BASE-T1x MAC-PHY Serial Interface

[Parthiban.Veerasooran]

Hi Andrew,

On 27/10/23 6:02 pm, Andrew Lunn wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
>>> Device tree described hardware. Its not supposed to be used to
>>> describe configuration. So it is not clear to me if any of these are
>>> valid in DT.
>>>
>>> It seems to me, the amount of control transfers should be very small
>>> compared to data transfers. So why not just set protection enable to
>>> be true?
>> Yes having protection enabled for control transfer doesn't hurt
>> anything. The only intention for keeping this as configurable is, it is
>> defined in the OPEN Alliance specification to enable/disable.
> 
> Standards often have options which nobody ever use, or are only useful
> in particular niches. Its often best to keep it simple, get the basic
> feature working, and then add these optional features if anybody
> actually needs them.
> 
>>> What is the effect of chunk payload size ? Is there a reason to use a
>>> lower value than the default 64? I assume smaller sizes make data
>>> transfer more expensive, since you need more DMA setup and completion
>>> handing etc.
>> Again the intention for keeping this as configurable is, it is defined
>> in the OPEN Alliance specification as user configurable. They can be 8,
>> 16, 32 and 64. And the default is 64. Also Microchip's LAN8650 supports
>> for 32 and 64.
> 
> Do you have any idea why the standard has different sizes? Why would
> you want to use 32? If you can answer this, it helps decide how
> important it is to support multiple sizes, or just hard code it to 64.
> 
> There are plenty of old research on Ethernet frame sizes, but they are
> for LAN/Internet usage. You typically see two peeks, one around 64-80
> bytes, and other around the full frame size. The small packets are TCP
> ACKS, and the rest is TCP data. However, this is a T1S device for
> automotive. I personally have no idea if the same traffic distribution
> is seen in that application?
> 
> Are there protocols running which use a lot of frames smaller than 64
> bytes? If so, 64 byte chunk size i assume could be wasteful, if there
> are lots of 32 byte frames.
> 
> The other potential issue is latency and the way the SPI bus
> works. Its a synchronised bi-directional bus. You can receive and
> transmit at the same time, but you have to setup the transfer to do
> that. If you are busy doing a receive only, and there is a new packet
> to send, you have to wait for the chunk transfer to complete before
> you can start a bi-directional chunk transfer. So a 32 byte chunk
> might make your link more efficient if you have heavy but bursty
> traffic. However, you have to consider the overheads of setting up the
> transfer and running the completion handler afterwards. This can be
> costly.
> 
> Do you have real use cases for using different chunks sizes? If not, i
> probably would just hard code it to 64, until somebody comes along
> needing something else.
> 
>>> An Ethernet driver is allowed to have driver specific private
>>> flags. See ethtool(1) --show-priv-flags and --set-priv-flags You could
>>> maybe use these to configure cut through?
>> So you mean, we have to implement the support in the ethtool interface
>> to enable/disable tx/rx cut through feature, isn't it?
>>
>> If you feel like the above configurations are not needed, so by keeping
>> protection true always, chunk payload size (cps) 64 always and moving
>> tx/rx cut through to ethtool, we can get rid of this DT bindings?
> 
> Again, do you have a real use case for cut through? Or maybe flip it
> around, Why would you not use cut through?
Thanks a lot for your detailed explanation. From this what I understand is,

1. Will make protection enabled always for control transfer.
2. Keep the default chunk payload size (64) as it is.
3. Default tx/rx cut through modes are disabled. So will not touch them.

So that we don't need DT bindings.

Best Regards,
Parthiban V
> 
>          Andrew