Re: [PATCH net-next v17 03/14] net: phy: add helpers to handle sfp phy connect/disconnect

[Christophe Leroy <christophe.leroy@csgroup.eu>]

Le 09/07/2024 à 08:30, Maxime Chevallier a écrit :
> There are a few PHY drivers that can handle SFP modules through their
> sfp_upstream_ops. Introduce Phylib helpers to keep track of connected
> SFP PHYs in a netdevice's namespace, by adding the SFP PHY to the
> upstream PHY's netdev's namespace.
> 
> By doing so, these SFP PHYs can be enumerated and exposed to users,
> which will be able to use their capabilities.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
> Reviewed-by: Andrew Lunn <andrew@lunn.ch>

Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Tested-by: Christophe Leroy <christophe.leroy@csgroup.eu>

> ---
>   drivers/net/phy/marvell-88x2222.c |  2 ++
>   drivers/net/phy/marvell.c         |  2 ++
>   drivers/net/phy/marvell10g.c      |  2 ++
>   drivers/net/phy/phy_device.c      | 42 +++++++++++++++++++++++++++++++
>   drivers/net/phy/qcom/at803x.c     |  2 ++
>   drivers/net/phy/qcom/qca807x.c    |  2 ++
>   include/linux/phy.h               |  2 ++
>   7 files changed, 54 insertions(+)
> 
> diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
> index b88398e6872b..0b777cdd7078 100644
> --- a/drivers/net/phy/marvell-88x2222.c
> +++ b/drivers/net/phy/marvell-88x2222.c
> @@ -553,6 +553,8 @@ static const struct sfp_upstream_ops sfp_phy_ops = {
>   	.link_down = mv2222_sfp_link_down,
>   	.attach = phy_sfp_attach,
>   	.detach = phy_sfp_detach,
> +	.connect_phy = phy_sfp_connect_phy,
> +	.disconnect_phy = phy_sfp_disconnect_phy,
>   };
>   
>   static int mv2222_probe(struct phy_device *phydev)
> diff --git a/drivers/net/phy/marvell.c b/drivers/net/phy/marvell.c
> index b89fbffa6a93..9964bf3dea2f 100644
> --- a/drivers/net/phy/marvell.c
> +++ b/drivers/net/phy/marvell.c
> @@ -3613,6 +3613,8 @@ static const struct sfp_upstream_ops m88e1510_sfp_ops = {
>   	.module_remove = m88e1510_sfp_remove,
>   	.attach = phy_sfp_attach,
>   	.detach = phy_sfp_detach,
> +	.connect_phy = phy_sfp_connect_phy,
> +	.disconnect_phy = phy_sfp_disconnect_phy,
>   };
>   
>   static int m88e1510_probe(struct phy_device *phydev)
> diff --git a/drivers/net/phy/marvell10g.c b/drivers/net/phy/marvell10g.c
> index ad43e280930c..6642eb642d4b 100644
> --- a/drivers/net/phy/marvell10g.c
> +++ b/drivers/net/phy/marvell10g.c
> @@ -503,6 +503,8 @@ static int mv3310_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
>   static const struct sfp_upstream_ops mv3310_sfp_ops = {
>   	.attach = phy_sfp_attach,
>   	.detach = phy_sfp_detach,
> +	.connect_phy = phy_sfp_connect_phy,
> +	.disconnect_phy = phy_sfp_disconnect_phy,
>   	.module_insert = mv3310_sfp_insert,
>   };
>   
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index e68acaba1b4f..a3309782220c 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1370,6 +1370,48 @@ phy_standalone_show(struct device *dev, struct device_attribute *attr,
>   }
>   static DEVICE_ATTR_RO(phy_standalone);
>   
> +/**
> + * phy_sfp_connect_phy - Connect the SFP module's PHY to the upstream PHY
> + * @upstream: pointer to the upstream phy device
> + * @phy: pointer to the SFP module's phy device
> + *
> + * This helper allows keeping track of PHY devices on the link. It adds the
> + * SFP module's phy to the phy namespace of the upstream phy
> + *
> + * Return: 0 on success, otherwise a negative error code.
> + */
> +int phy_sfp_connect_phy(void *upstream, struct phy_device *phy)
> +{
> +	struct phy_device *phydev = upstream;
> +	struct net_device *dev = phydev->attached_dev;
> +
> +	if (dev)
> +		return phy_link_topo_add_phy(dev, phy, PHY_UPSTREAM_PHY, phydev);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL(phy_sfp_connect_phy);
> +
> +/**
> + * phy_sfp_disconnect_phy - Disconnect the SFP module's PHY from the upstream PHY
> + * @upstream: pointer to the upstream phy device
> + * @phy: pointer to the SFP module's phy device
> + *
> + * This helper allows keeping track of PHY devices on the link. It removes the
> + * SFP module's phy to the phy namespace of the upstream phy. As the module phy
> + * will be destroyed, re-inserting the same module will add a new phy with a
> + * new index.
> + */
> +void phy_sfp_disconnect_phy(void *upstream, struct phy_device *phy)
> +{
> +	struct phy_device *phydev = upstream;
> +	struct net_device *dev = phydev->attached_dev;
> +
> +	if (dev)
> +		phy_link_topo_del_phy(dev, phy);
> +}
> +EXPORT_SYMBOL(phy_sfp_disconnect_phy);
> +
>   /**
>    * phy_sfp_attach - attach the SFP bus to the PHY upstream network device
>    * @upstream: pointer to the phy device
> diff --git a/drivers/net/phy/qcom/at803x.c b/drivers/net/phy/qcom/at803x.c
> index c8f83e5f78ab..105602581a03 100644
> --- a/drivers/net/phy/qcom/at803x.c
> +++ b/drivers/net/phy/qcom/at803x.c
> @@ -770,6 +770,8 @@ static const struct sfp_upstream_ops at8031_sfp_ops = {
>   	.attach = phy_sfp_attach,
>   	.detach = phy_sfp_detach,
>   	.module_insert = at8031_sfp_insert,
> +	.connect_phy = phy_sfp_connect_phy,
> +	.disconnect_phy = phy_sfp_disconnect_phy,
>   };
>   
>   static int at8031_parse_dt(struct phy_device *phydev)
> diff --git a/drivers/net/phy/qcom/qca807x.c b/drivers/net/phy/qcom/qca807x.c
> index 672c6929119a..5eb0ab1cb70e 100644
> --- a/drivers/net/phy/qcom/qca807x.c
> +++ b/drivers/net/phy/qcom/qca807x.c
> @@ -699,6 +699,8 @@ static const struct sfp_upstream_ops qca807x_sfp_ops = {
>   	.detach = phy_sfp_detach,
>   	.module_insert = qca807x_sfp_insert,
>   	.module_remove = qca807x_sfp_remove,
> +	.connect_phy = phy_sfp_connect_phy,
> +	.disconnect_phy = phy_sfp_disconnect_phy,
>   };
>   
>   static int qca807x_probe(struct phy_device *phydev)
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index 2d477eb2809a..f7ef7ed6d5ce 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -1762,6 +1762,8 @@ int phy_suspend(struct phy_device *phydev);
>   int phy_resume(struct phy_device *phydev);
>   int __phy_resume(struct phy_device *phydev);
>   int phy_loopback(struct phy_device *phydev, bool enable);
> +int phy_sfp_connect_phy(void *upstream, struct phy_device *phy);
> +void phy_sfp_disconnect_phy(void *upstream, struct phy_device *phy);
>   void phy_sfp_attach(void *upstream, struct sfp_bus *bus);
>   void phy_sfp_detach(void *upstream, struct sfp_bus *bus);
>   int phy_sfp_probe(struct phy_device *phydev,