Messages in this thread | | | From | Christian Herber <> | Subject | Re: Re: [PATCH net-next 0/1] Add BASE-T1 PHY support | Date | Mon, 26 Aug 2019 07:57:18 +0000 |
| |
On 24.08.2019 17:03, Heiner Kallweit wrote: > > On 22.08.2019 09:18, Christian Herber wrote: >> 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 >>> network? >>> >>> 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. >>> >>> Andrew >>> >> >> 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) >> - BASE-T1 >> - 10BASE-T1 >> - NGBASE-T1 >> >> And surely there are some on the way or already there that I am not >> aware of. >> >> 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 >> two IMHO. >> > BASE-T1 seems to be based on Clause 45 (at least Clause 45 MDIO), > but it's not fully compliant with Clause 45. Taking AN link status > as an example: 45.2.7.2.7 states that link-up is signaled in bit 7.1.2. > If BASE-T1 uses a different register, then it's not fully Clause 45 > compatible.
Clause 45 defines e.g. bit 7.1.2 just like it defines the BASE-T1 auto-neg registers. Any bit that i have used in my patch is 100% standardized in IEEE 802.3-2018, Clause 45. By definition, BASE-T1 PHYs have to use the Clause 45 BASE-T1 registers, otherwise they are not IEEE compliant.
> Therefore also my question for the datasheet of an actual BASE-T1 PHY, > as I would be curious whether it shadows the link-up bit from 7.513.2 > to 7.1.2 to be Clause 45 compliant. Definitely reading bit 7.513.2 > is nothing that belongs into a genphy_c45_ function.
For now, there is no such public data sheet. However, IEEE 802.3-2018 is public. This should be the basis for a generic driver. Datasheets are needed for the device specific drivers. If Linux cares to support BASE-T1, it should implement a driver that works with a standard compliant PHY and that can be done on the basis of IEEE.
> The extension to genphy_c45_pma_read_abilities() looks good to me, > for the other parts I'd like to see first how real world BASE-T1 PHYs > handle it. If they shadow the T1-specific bits to the Clause 45 > standard ones, we should be fine. Otherwise IMO we have to add > separate T1 functions to phylib. > > Heiner >
There is not requirement in IEEE to shadow the BASE-T1 registers into the "standard" ones. Thus, such an assumption should not be done for a generic driver, as it will quite certainly not work with all devices. Fyi, both registers are standard, just that the historically first ones are for classic 10/100/1000/10000 PHYs and BASE-T1 registers are for the single twisted pair PHYs.
| |