Messages in this thread | | | From | Vladimir Oltean <> | Subject | Re: [RFC PATCH net] Revert "net: phy: Uniform PHY driver access" | Date | Tue, 14 Sep 2021 12:06:18 +0000 |
| |
On Mon, Sep 13, 2021 at 08:49:19PM +0200, Andrew Lunn wrote: > > I am not sure why "to_phy_driver" needs cleanup. Au contraire, I think > > the PHY library's usage of struct phy_device :: drv is what is strange > > and potentially buggy, it is the only subsystem I know of that keeps its > > own driver pointer rather than looking at struct device :: driver. > > There is one odd driver in the mix. Take a look at xilinx_gmii2rgmii.c. > > It probably could be done a better way, but that is what we have.
Interesting, to say the least. Also, is there any connection between that and the revert I'm proposing?
So compared to other vendors, where the RGMII gasket is part of the MAC device, with Xilinx Zynq it is accessible via MDIO?
This is not all that different from dpaa2-eth and dpaa2-mac which are different devices on the bus, with different drivers, and the phy-handle is present on the dpaa2-mac OF node, but the net_device is registered by the dpaa2-eth device, is it? It seems to be even simpler in fact, because the dpaa2-mac and dpaa2-eth can even connect/disconnect from each other at runtime, something which does not appear possible with the Xilinx MAC and its RGMII gasket.
What was done in that case was that all drivers which could possibly connect to the DPMAC would need to call dpaa2_mac_connect(), this is done currently from dpaa2-eth and dpaa2-switch.
It looks like it is said that this GMII2RGMII converter can be placed in front of any GMII MAC. Nice that there are zero in-tree users of "xlnx,gmii-to-rgmii-1.0" so that I could figure out exactly how that plays out in practice. If it is only a few drivers that use the GMII2RGMII converter, maybe something like that (export a few symbols from this driver, make all MAC drivers call them) could take less of a toll on the overall PHY library architecture.
Note that the usage of priv->phy_dev, priv->phy_drv, priv->conv_phy_drv beats me. Why is "phy_dev" kept inside "priv" even though it is accessed only inside xgmiitorgmii_probe? Why does xgmiitorgmii_configure() need to be called from xgmiitorgmii_read_status() which in turn hooks into the attached PHY driver's phy_read_status()? Why does xgmiitorgmii_configure not get exported and called from an .adjust_link method or the phylink equivalent, like any other MAC-side hardware linked with the PHY library in the kernel?
| |