[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b1ef50ce-40e5-453a-a78a-55e373ad36db@gmail.com>
Date: Wed, 8 May 2024 07:43:43 +0200
From: Heiner Kallweit <hkallweit1@...il.com>
To: Maxime Chevallier <maxime.chevallier@...tlin.com>, davem@...emloft.net
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
thomas.petazzoni@...tlin.com, Andrew Lunn <andrew@...n.ch>,
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>,
Vladimir Oltean <vladimir.oltean@....com>,
Köry Maincent <kory.maincent@...tlin.com>,
Jesse Brandeburg <jesse.brandeburg@...el.com>, Marek Behún
<kabel@...nel.org>, Piergiorgio Beruto <piergiorgio.beruto@...il.com>,
Oleksij Rempel <o.rempel@...gutronix.de>,
Nicolò Veronese <nicveronese@...il.com>,
Simon Horman <horms@...nel.org>, mwojtas@...omium.org,
Nathan Chancellor <nathan@...nel.org>, Antoine Tenart <atenart@...nel.org>
Subject: Re: [PATCH net-next 1/2] net: phy: phy_link_topology: Pass netdevice
to phy_link_topo helpers
On 07.05.2024 12:28, Maxime Chevallier wrote:
> The phy link topology's main goal is to better track which PHYs are
> connected to a given netdevice. Make so that the helpers take the
> netdevice as a parameter directly.
>
The commit message should explain what the issue is that you're fixing,
and how this patch fixes it.
> Signed-off-by: Maxime Chevallier <maxime.chevallier@...tlin.com>
> Fixes: 6916e461e793 ("net: phy: Introduce ethernet link topology representation")
> Closes: https://lore.kernel.org/netdev/2e11b89d-100f-49e7-9c9a-834cc0b82f97@gmail.com/
> Closes: https://lore.kernel.org/netdev/20240409201553.GA4124869@dev-arch.thelio-3990X/
> ---
> drivers/net/phy/phy_device.c | 25 ++++++++-----------------
> drivers/net/phy/phy_link_topology.c | 13 ++++++++++---
> include/linux/phy_link_topology.h | 21 +++++++++++++--------
> net/ethtool/netlink.c | 2 +-
> 4 files changed, 32 insertions(+), 29 deletions(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index 616bd7ba46cb..111434201545 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -277,14 +277,6 @@ static void phy_mdio_device_remove(struct mdio_device *mdiodev)
>
> static struct phy_driver genphy_driver;
>
> -static struct phy_link_topology *phy_get_link_topology(struct phy_device *phydev)
> -{
> - if (phydev->attached_dev)
> - return phydev->attached_dev->link_topo;
> -
> - return NULL;
> -}
> -
> static LIST_HEAD(phy_fixup_list);
> static DEFINE_MUTEX(phy_fixup_lock);
>
> @@ -1389,10 +1381,10 @@ static DEVICE_ATTR_RO(phy_standalone);
> int phy_sfp_connect_phy(void *upstream, struct phy_device *phy)
> {
> struct phy_device *phydev = upstream;
> - struct phy_link_topology *topo = phy_get_link_topology(phydev);
> + struct net_device *dev = phydev->attached_dev;
>
> - if (topo)
> - return phy_link_topo_add_phy(topo, phy, PHY_UPSTREAM_PHY, phydev);
> + if (dev)
> + return phy_link_topo_add_phy(dev, phy, PHY_UPSTREAM_PHY, phydev);
>
> return 0;
> }
> @@ -1411,10 +1403,10 @@ EXPORT_SYMBOL(phy_sfp_connect_phy);
> void phy_sfp_disconnect_phy(void *upstream, struct phy_device *phy)
> {
> struct phy_device *phydev = upstream;
> - struct phy_link_topology *topo = phy_get_link_topology(phydev);
> + struct net_device *dev = phydev->attached_dev;
>
> - if (topo)
> - phy_link_topo_del_phy(topo, phy);
> + if (dev)
> + phy_link_topo_del_phy(dev, phy);
> }
> EXPORT_SYMBOL(phy_sfp_disconnect_phy);
>
> @@ -1561,8 +1553,7 @@ 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);
> + err = phy_link_topo_add_phy(dev, phydev, PHY_UPSTREAM_MAC, dev);
> if (err)
> goto error;
> }
> @@ -1992,7 +1983,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);
> + phy_link_topo_del_phy(dev, phydev);
> }
> phydev->phylink = NULL;
>
> diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
> index 985941c5c558..0e36bd7c15dc 100644
> --- a/drivers/net/phy/phy_link_topology.c
> +++ b/drivers/net/phy/phy_link_topology.c
> @@ -35,10 +35,11 @@ void phy_link_topo_destroy(struct phy_link_topology *topo)
> kfree(topo);
> }
>
> -int phy_link_topo_add_phy(struct phy_link_topology *topo,
> +int phy_link_topo_add_phy(struct net_device *dev,
> struct phy_device *phy,
> enum phy_upstream upt, void *upstream)
> {
> + struct phy_link_topology *topo = dev->link_topo;
> struct phy_device_node *pdn;
> int ret;
>
> @@ -90,10 +91,16 @@ int phy_link_topo_add_phy(struct phy_link_topology *topo,
> }
> EXPORT_SYMBOL_GPL(phy_link_topo_add_phy);
>
> -void phy_link_topo_del_phy(struct phy_link_topology *topo,
> +void phy_link_topo_del_phy(struct net_device *dev,
> struct phy_device *phy)
> {
> - struct phy_device_node *pdn = xa_erase(&topo->phys, phy->phyindex);
> + struct phy_link_topology *topo = dev->link_topo;
> + struct phy_device_node *pdn;
> +
> + if (!topo)
> + return;
> +
> + pdn = xa_erase(&topo->phys, phy->phyindex);
>
> /* We delete the PHY from the topology, however we don't re-set the
> * phy->phyindex field. If the PHY isn't gone, we can re-assign it the
> diff --git a/include/linux/phy_link_topology.h b/include/linux/phy_link_topology.h
> index 6b79feb607e7..166a01710aa2 100644
> --- a/include/linux/phy_link_topology.h
> +++ b/include/linux/phy_link_topology.h
> @@ -12,11 +12,11 @@
> #define __PHY_LINK_TOPOLOGY_H
>
> #include <linux/ethtool.h>
> +#include <linux/netdevice.h>
> #include <linux/phy_link_topology_core.h>
>
> struct xarray;
> struct phy_device;
> -struct net_device;
> struct sfp_bus;
>
> struct phy_device_node {
> @@ -37,11 +37,16 @@ struct phy_link_topology {
> u32 next_phy_index;
> };
>
> -static inline struct phy_device *
> -phy_link_topo_get_phy(struct phy_link_topology *topo, u32 phyindex)
> +static inline struct phy_device
> +*phy_link_topo_get_phy(struct net_device *dev, u32 phyindex)
> {
> - struct phy_device_node *pdn = xa_load(&topo->phys, phyindex);
> + struct phy_link_topology *topo = dev->link_topo;
> + struct phy_device_node *pdn;
>
> + if (!topo)
> + return NULL;
> +
> + pdn = xa_load(&topo->phys, phyindex);
> if (pdn)
> return pdn->phy;
>
> @@ -49,21 +54,21 @@ phy_link_topo_get_phy(struct phy_link_topology *topo, u32 phyindex)
> }
>
> #if IS_REACHABLE(CONFIG_PHYLIB)
> -int phy_link_topo_add_phy(struct phy_link_topology *topo,
> +int phy_link_topo_add_phy(struct net_device *dev,
> struct phy_device *phy,
> enum phy_upstream upt, void *upstream);
>
> -void phy_link_topo_del_phy(struct phy_link_topology *lt, struct phy_device *phy);
> +void phy_link_topo_del_phy(struct net_device *dev, struct phy_device *phy);
>
> #else
> -static inline int phy_link_topo_add_phy(struct phy_link_topology *topo,
> +static inline int phy_link_topo_add_phy(struct net_device *dev,
> struct phy_device *phy,
> enum phy_upstream upt, void *upstream)
> {
> return 0;
> }
>
> -static inline void phy_link_topo_del_phy(struct phy_link_topology *topo,
> +static inline void phy_link_topo_del_phy(struct net_device *dev,
> struct phy_device *phy)
> {
> }
> diff --git a/net/ethtool/netlink.c b/net/ethtool/netlink.c
> index 563e94e0cbd8..f5b4adf324bc 100644
> --- a/net/ethtool/netlink.c
> +++ b/net/ethtool/netlink.c
> @@ -170,7 +170,7 @@ int ethnl_parse_header_dev_get(struct ethnl_req_info *req_info,
> struct nlattr *phy_id;
>
> phy_id = tb[ETHTOOL_A_HEADER_PHY_INDEX];
> - phydev = phy_link_topo_get_phy(dev->link_topo,
> + phydev = phy_link_topo_get_phy(dev,
> nla_get_u32(phy_id));
> if (!phydev) {
> NL_SET_BAD_ATTR(extack, phy_id);
Powered by blists - more mailing lists