Re: [PATCH net-next v18 01/10] net: phy: phy_link_topology: Add a helper for opportunistic alloc

["Christophe Leroy (CS GROUP)" <chleroy@kernel.org>]

Le 27/09/2026 à 15:36, Maxime Chevallier a écrit :
> The phy_link_topology structure stores information about the PHY-related
> components connected to a net_device. It is opportunistically allocated,
> when we add the first item to the topology, as this is not relevant for
> all kinds of net_devices.
> 
> In preparation for the addition of phy_port tracking in the topology,
> let's make a dedicated helper for that allocation sequence.
> 
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>
> Tested-by: Aleksei Sviridkin <f@lex.la>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

Reviewed-by: Christophe Leroy (CS GROUP) <chleroy@kernel.org>

> ---
>   drivers/net/phy/phy_link_topology.c | 40 +++++++++++++++++++++++------
>   1 file changed, 32 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/phy/phy_link_topology.c b/drivers/net/phy/phy_link_topology.c
> index 4134de7ae313..0462283c8020 100644
> --- a/drivers/net/phy/phy_link_topology.c
> +++ b/drivers/net/phy/phy_link_topology.c
> @@ -28,11 +28,39 @@ static int netdev_alloc_phy_link_topology(struct net_device *dev)
>   	return 0;
>   }
>   
> +static struct phy_link_topology *phy_link_topo_get_or_alloc(struct net_device *dev)
> +{
> +	int ret;
> +
> +	if (dev->link_topo)
> +		return dev->link_topo;
> +
> +	/* The topology is allocated the first time we add an object to it.
> +	 * It is freed alongside the netdev. It can be called on multiple
> +	 * contexts:
> +	 *  - It can be called from .probe() : No rtnl, no netdev_lock
> +	 *  - .ndo_open() : rtnl and possibly netdev_lock
> +	 *  - SFP state machine : rtnl held or not
> +	 *
> +	 *  However, we can't really have races :
> +	 *  - If we have a PHY, phy_link_topo_add_phy() will always run first
> +	 *    and trigger the alloc. Only then the ports can be added through
> +	 *    phylib or sfp.
> +	 *  - If we don't, the SFP port for the cage is registered first, and
> +	 *    only then other ports/PHYs can be registered.
> +	 */
> +	ret = netdev_alloc_phy_link_topology(dev);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	return dev->link_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_link_topology *topo;
>   	struct phy_device_node *pdn;
>   	int ret;
>   
> @@ -45,13 +73,9 @@ int phy_link_topo_add_phy(struct net_device *dev,
>   	if (WARN_ON_ONCE(netdev_need_ops_lock(dev)))
>   		return -EOPNOTSUPP;
>   
> -	if (!topo) {
> -		ret = netdev_alloc_phy_link_topology(dev);
> -		if (ret)
> -			return ret;
> -
> -		topo = dev->link_topo;
> -	}
> +	topo = phy_link_topo_get_or_alloc(dev);
> +	if (IS_ERR(topo))
> +		return PTR_ERR(topo);
>   
>   	pdn = kzalloc_obj(*pdn);
>   	if (!pdn)