Re: [PATCH net-next v12 11/18] net: phy: marvell-88x2222: Support SFP through phy_port interface

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

Le 09/09/2025 à 17:26, Maxime Chevallier a écrit :
> The 88x2222 PHY from Marvell only supports serialised modes as its
> line-facing interfaces. Convert that driver to the generic phylib SFP
> handling.
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

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

> ---
>   drivers/net/phy/marvell-88x2222.c | 95 +++++++++++++------------------
>   1 file changed, 38 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c
> index fad2f54c1eac..ba1bbb6c63d6 100644
> --- a/drivers/net/phy/marvell-88x2222.c
> +++ b/drivers/net/phy/marvell-88x2222.c
> @@ -13,7 +13,7 @@
>   #include <linux/mdio.h>
>   #include <linux/marvell_phy.h>
>   #include <linux/of.h>
> -#include <linux/sfp.h>
> +#include <linux/phy_port.h>
>   #include <linux/netdevice.h>
>   
>   /* Port PCS Configuration */
> @@ -473,90 +473,70 @@ static int mv2222_config_init(struct phy_device *phydev)
>   	return 0;
>   }
>   
> -static int mv2222_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
> +static int mv2222_configure_serdes(struct phy_port *port, bool enable,
> +				   phy_interface_t interface)
>   {
> -	DECLARE_PHY_INTERFACE_MASK(interfaces);
> -	struct phy_device *phydev = upstream;
> -	phy_interface_t sfp_interface;
> +	struct phy_device *phydev = port_phydev(port);
>   	struct mv2222_data *priv;
> -	struct device *dev;
> -	int ret;
> -
> -	__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_supported) = { 0, };
> +	int ret = 0;
>   
>   	priv = phydev->priv;
> -	dev = &phydev->mdio.dev;
> -
> -	sfp_parse_support(phydev->sfp_bus, id, sfp_supported, interfaces);
> -	phydev->port = sfp_parse_port(phydev->sfp_bus, id, sfp_supported);
> -	sfp_interface = sfp_select_interface(phydev->sfp_bus, sfp_supported);
> -
> -	dev_info(dev, "%s SFP module inserted\n", phy_modes(sfp_interface));
> +	priv->line_interface = interface;
>   
> -	if (sfp_interface != PHY_INTERFACE_MODE_10GBASER &&
> -	    sfp_interface != PHY_INTERFACE_MODE_1000BASEX &&
> -	    sfp_interface != PHY_INTERFACE_MODE_SGMII) {
> -		dev_err(dev, "Incompatible SFP module inserted\n");
> +	if (enable) {
> +		linkmode_and(priv->supported, phydev->supported, port->supported);
>   
> -		return -EINVAL;
> -	}
> -
> -	priv->line_interface = sfp_interface;
> -	linkmode_and(priv->supported, phydev->supported, sfp_supported);
> +		ret = mv2222_config_line(phydev);
> +		if (ret < 0)
> +			return ret;
>   
> -	ret = mv2222_config_line(phydev);
> -	if (ret < 0)
> -		return ret;
> +		if (mutex_trylock(&phydev->lock)) {
> +			ret = mv2222_config_aneg(phydev);
> +			mutex_unlock(&phydev->lock);
> +		}
>   
> -	if (mutex_trylock(&phydev->lock)) {
> -		ret = mv2222_config_aneg(phydev);
> -		mutex_unlock(&phydev->lock);
> +	} else {
> +		linkmode_zero(priv->supported);
>   	}
>   
>   	return ret;
>   }
>   
> -static void mv2222_sfp_remove(void *upstream)
> +static void mv2222_port_link_up(struct phy_port *port)
>   {
> -	struct phy_device *phydev = upstream;
> -	struct mv2222_data *priv;
> -
> -	priv = phydev->priv;
> -
> -	priv->line_interface = PHY_INTERFACE_MODE_NA;
> -	linkmode_zero(priv->supported);
> -	phydev->port = PORT_NONE;
> -}
> -
> -static void mv2222_sfp_link_up(void *upstream)
> -{
> -	struct phy_device *phydev = upstream;
> +	struct phy_device *phydev = port_phydev(port);
>   	struct mv2222_data *priv;
>   
>   	priv = phydev->priv;
>   	priv->sfp_link = true;
>   }
>   
> -static void mv2222_sfp_link_down(void *upstream)
> +static void mv2222_port_link_down(struct phy_port *port)
>   {
> -	struct phy_device *phydev = upstream;
> +	struct phy_device *phydev = port_phydev(port);
>   	struct mv2222_data *priv;
>   
>   	priv = phydev->priv;
>   	priv->sfp_link = false;
>   }
>   
> -static const struct sfp_upstream_ops sfp_phy_ops = {
> -	.module_insert = mv2222_sfp_insert,
> -	.module_remove = mv2222_sfp_remove,
> -	.link_up = mv2222_sfp_link_up,
> -	.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 const struct phy_port_ops mv2222_port_ops = {
> +	.link_up = mv2222_port_link_up,
> +	.link_down = mv2222_port_link_down,
> +	.configure_mii = mv2222_configure_serdes,
>   };
>   
> +static int mv2222_attach_mii_port(struct phy_device *phydev, struct phy_port *port)
> +{
> +	port->ops = &mv2222_port_ops;
> +
> +	__set_bit(PHY_INTERFACE_MODE_10GBASER, port->interfaces);
> +	__set_bit(PHY_INTERFACE_MODE_1000BASEX, port->interfaces);
> +	__set_bit(PHY_INTERFACE_MODE_SGMII, port->interfaces);
> +
> +	return 0;
> +}
> +
>   static int mv2222_probe(struct phy_device *phydev)
>   {
>   	struct device *dev = &phydev->mdio.dev;
> @@ -592,7 +572,7 @@ static int mv2222_probe(struct phy_device *phydev)
>   	priv->line_interface = PHY_INTERFACE_MODE_NA;
>   	phydev->priv = priv;
>   
> -	return phy_sfp_probe(phydev, &sfp_phy_ops);
> +	return 0;
>   }
>   
>   static struct phy_driver mv2222_drivers[] = {
> @@ -609,6 +589,7 @@ static struct phy_driver mv2222_drivers[] = {
>   		.suspend = mv2222_suspend,
>   		.resume = mv2222_resume,
>   		.read_status = mv2222_read_status,
> +		.attach_mii_port = mv2222_attach_mii_port,
>   	},
>   };
>   module_phy_driver(mv2222_drivers);