Re: Re: [PATCH net-next 0/1] Add BASE-T1 PHY support
From: Christian Herber
Date: Thu Aug 22 2019 - 09:44:24 EST
On 21.08.2019 20:57, Andrew Lunn wrote:
>> The current patch set IMO is a little bit hacky. I'm not 100% happy
>> with the implicit assumption that there can't be devices supporting
>> T1 and classic BaseT modes or fiber modes.
>> Andrew: Do you have an opinion on that?
> Hi Heiner
> I would also like cleaner integration. I doubt here is anything in the
> standard which says you cannot combine these modes. It is more a
> marketing question if anybody would build such a device. Maybe not
> directly into a vehicle, but you could imaging a mobile test device
> which uses T1 to talk to the car and T4 to connect to the garage
> So i don't think we should limit ourselves. phylib should provide a
> clean, simple set of helpers to perform standard operations for
> various modes. Drivers can make use of those helpers. That much should
> be clear. If we try to make genphy support them all simultaneously, is
> less clear.
If you want to go down this path, then i think we have to ask some more
questions. Clause 45 is a very scalable register scheme, it is not a
specific class of devices and will be extended and extended.
Currently, the phy-c45.c supports 10/100/1000/2500/5000/10000 Mbps
consumer/enterprise PHYs. This is also an implicit assumption. The
register set (e.g. on auto-neg) used for this will also only support
these modes and nothing more, as it is done scaling.
Currently not supported, but already present in IEEE 802.3:
- MultiGBASE-T (25/40 Gbps) (see e.g. MultiGBASE-T AN control 1 register)
And surely there are some on the way or already there that I am not
To me, one architectural decision point is if you want to have generic
support for all C45 PHYs in one file, or if you want to split it by
device class. I went down the first path with my patch, as this is the
road gone also with the existing code.
If you want to split BASE-T1, i think you will need one basic C45
library (genphy_c45_pma_read_abilities() is a good example of a function
that is not specific to a device class). On the other hand,
genphy_c45_pma_setup_forced() is not a generic function at this point as
it supports only a subset of devices managed in C45.
I tend to agree with you that splitting is the best way to go in the
long run, but that also requires a split of the existing phy-c45.c into