Re: [PATCH v6 3/6] net: phy: at803x: add QCA8084 ethernet phy support

Jie Luo
2023-12-01T08:06:32Z

On 11/29/2023 8:04 PM, Vladimir Oltean wrote:
> On Wed, Nov 29, 2023 at 06:34:16PM +0800, Jie Luo wrote:
>>>> The PCS drivers in drivers/net/pcs/ should be in PHY side, such as
>>>> pcs-lynx.c and pcs-xpcs.c, they are configuring the MDIO device
>>>> registers.
>>>
>>> Wrong. No they are not. Just because they are accessed via MDIO does
>>> not mean they are in the PHY. MDIO can be used for more than just the
>>> PHY, and is on a lot of platforms.
>>>
>>> LX2160A for example has many MDIO buses, and the PCSes (of which there
>>> are multiple inside the chip, and use pcs-lynx) are accessed through
>>> the MDIO bus specific to each port. They are not MMIO mapped.
>>>
>>> The same is true on stmmac platforms, where xpcs is used - xpcs is the
>>> _MAC_ side PCS.
>>>
>>> Sorry but you are wrong.
>>>
>>
>> OK, but it creates the PCS driver based on the MDIO device in pcs-lynx.c
>> looks like this PCS is located in PHY device from hardware perspective.
>
> In some ways, this contradiction has a potato-patato aspect to it.
> As Russell says, NXP devices do have internal SGMII/USXGMII/10GBASE-R
> ports which use pcs-lynx.c to access the registers of the PCS layer
> (which are on MDIO buses internal to the SoC). They could legally be
> called PHYs, because they have all the layers that 802.3 says a PHY
> should have: a PCS, a PMA and a PMD.
>
> But what phylib understands a phy_device to be is a more restricted
> definition than just "a PHY - any PHY". Originally, phylib considered a
> struct phy_device to be something (a discrete chip) that has pins and a
> phy_interface_t towards its host side, and pins + an ethtool_link_mode_bit_indices
> on its media side.
>
> Traditionally, the media side is exclusively copper (BASE-T, BASE-T1) or
> fiber (BASE-SX/LX).
>
> A struct phy_device was then also used with PHY_INTERFACE_MODE_INTERNAL
> to represent the built-in BASE-T PHYs that are embedded within certain
> small/medium business Ethernet switches. And then, more and more other
> similar embedded copper PHYs.
>
> The idea is that (1) a phy_device connects to a remote system, and
> (2) the phylib API does not have insight into the components of the
> PHY it controls: PCS, PMA, PMD. It's all just a monolithic struct phy_device.
>
> Because there are serial phy_interface_t modes where the MAC also need a
> PHY to even connect to the phylib PHY, a problem presented itself:
> phylib only has support for a single phy_device. So a new framework
> appeared: phylink, which uses the unmodified phylib layer for the
> external PHY, but models the MAC-side PHY using a different API. Later
> on, that API became the phylink_pcs.
>
> To muddy the waters, a phylink_pcs structure usually connects to another
> local component as described above, like a phylib PHY (on-board or on an
> SFP module). But it can also connect directly to a remote system (like a
> phy_device would). But the phylink_pcs is always integrated in silicon
> with the MAC, and the "media side" of it is a phy_interface_t type, not
> an ethtool_link_mode_bit_indices type.
>
> Having a separate phylink_pcs is what allows us to work around phylib's
> limitation of having a single phy_device. The reverse is also true: you
> can have a single phylink_pcs, and that belongs to the client MAC driver.
>
> The other layers (PMA/PMD) of the MAC-side PHY are modeled in the kernel
> as a struct phy (https://docs.kernel.org/driver-api/phy/index.html), and
> we have the phy_set_mode_ext() API for reconfiguring this layer to a
> different mode. Again, this is not applicable for phylib PHYs, which are
> monolithic.
>
> Given the above definitions, what NXP has and drives with pcs-lynx.c is
> not a struct phy_device, but a MAC-side PCS represented by a phylink_pcs.
> It absolutely does not matter that the register access method for the
> PCS is an internal MDIO bus. FWIW, the PMA/PMD layer is at
> drivers/phy/freescale/phy-fsl-lynx-28g.c.
>
> So, if put into the proper context, what Russell is saying is correct,
> but I think you need a bit of history to not get even more confused
> about why it is the way it is.

Thanks Vladimir for the detail information, i just get this message,
which is helpful to me.