[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ede222d4-11da-4b95-a685-17cb480694dd@lunn.ch>
Date: Sun, 17 Dec 2023 17:57:10 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: Maxime Chevallier <maxime.chevallier@...tlin.com>, davem@...emloft.net,
netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
thomas.petazzoni@...tlin.com, Jakub Kicinski <kuba@...nel.org>,
Eric Dumazet <edumazet@...gle.com>, Paolo Abeni <pabeni@...hat.com>,
Russell King <linux@...linux.org.uk>,
linux-arm-kernel@...ts.infradead.org,
Christophe Leroy <christophe.leroy@...roup.eu>,
Herve Codina <herve.codina@...tlin.com>,
Florian Fainelli <f.fainelli@...il.com>,
Heiner Kallweit <hkallweit1@...il.com>,
Köry Maincent <kory.maincent@...tlin.com>,
Jesse Brandeburg <jesse.brandeburg@...el.com>,
Jonathan Corbet <corbet@....net>,
Marek Behún <kabel@...nel.org>,
Piergiorgio Beruto <piergiorgio.beruto@...il.com>,
Oleksij Rempel <o.rempel@...gutronix.de>,
Nicolò Veronese <nicveronese@...il.com>
Subject: Re: [PATCH net-next v4 01/13] net: phy: Introduce ethernet link
topology representation
On Fri, Dec 15, 2023 at 11:45:23PM +0200, Vladimir Oltean wrote:
> On Fri, Dec 15, 2023 at 06:12:23PM +0100, Maxime Chevallier wrote:
> > diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> > index d8e9335d415c..89daaccc9276 100644
> > --- a/drivers/net/phy/phy_device.c
> > +++ b/drivers/net/phy/phy_device.c
> > @@ -1491,6 +1500,11 @@ int phy_attach_direct(struct net_device *dev, struct phy_device *phydev,
> >
> > if (phydev->sfp_bus_attached)
> > dev->sfp_bus = phydev->sfp_bus;
> > +
> > + err = phy_link_topo_add_phy(&dev->link_topo, phydev,
> > + PHY_UPSTREAM_MAC, dev);
> > + if (err)
> > + goto error;
> > }
> >
> > /* Some Ethernet drivers try to connect to a PHY device before
> > @@ -1816,6 +1830,7 @@ void phy_detach(struct phy_device *phydev)
> > if (dev) {
> > phydev->attached_dev->phydev = NULL;
> > phydev->attached_dev = NULL;
> > + phy_link_topo_del_phy(&dev->link_topo, phydev);
> > }
> > phydev->phylink = NULL;
> >
> > diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
> > new file mode 100644
> > index 000000000000..22f6372d002c
> > --- /dev/null
> > +++ b/drivers/net/phy/phy_link_topology.c
> > +int phy_link_topo_add_phy(struct phy_link_topology *topo,
> > + struct phy_device *phy,
> > + enum phy_upstream upt, void *upstream)
> > +{
> > + struct phy_device_node *pdn;
> > + int ret;
> > +
> > + /* Protects phy and upstream */
> > + ASSERT_RTNL();
>
> Something to think for the PHY library maintainers. This is probably
> the first time when the rtnl_lock() requirement is asserted at
> phy_attach_direct() time.
There are two use cases here for plain MAC drivers.
1) phy_attach_direct() is called from probe. RTNL is normally not
held, the driver would have to take it before making the call.
2) phy_attach_direct() is called from ndo_open. In that case,
__dev_open() has a ASSERT_RTNL() so we can assume RTNL has been taken.
So i don't think we can assume RTNL is held, but it might be held.
We need a better understanding what is being protected here.
Andrew
Powered by blists - more mailing lists