linux-kernel - Re: [RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20210914120617.iaqaukal3riridew@skbuf>
Date:   Tue, 14 Sep 2021 12:06:18 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Andrew Lunn <andrew@...n.ch>
CC:     Vladimir Oltean <olteanv@...il.com>,
        Gerhard Engleder <gerhard@...leder-embedded.com>,
        netdev <netdev@...r.kernel.org>,
        Heiner Kallweit <hkallweit1@...il.com>,
        "David S. Miller" <davem@...emloft.net>,
        Russell King <linux@...linux.org.uk>,
        Jakub Kicinski <kuba@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC PATCH net] Revert "net: phy: Uniform PHY driver access"

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?