Re: [PATCH net-next v12 14/18] net: phy: at803x: Support SFP through phy_port interface

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

Le 09/09/2025 à 17:26, Maxime Chevallier a écrit :
> Convert the at803x driver to use the generic phylib SFP handling, via a
> dedicated .attach_port() callback, populating the supported interfaces.
> 
> As these devices are limited to 1000BaseX, a workaround is used to also
> support, in a very limited way, copper modules. This is done by
> supporting SGMII but limiting it to 1G full duplex (in which case it's
> somewhat compatible with 1000BaseX).
> 
> Signed-off-by: Maxime Chevallier <maxime.chevallier@bootlin.com>

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

> ---
>   drivers/net/phy/qcom/at803x.c | 78 ++++++++++++++---------------------
>   1 file changed, 32 insertions(+), 46 deletions(-)
> 
> diff --git a/drivers/net/phy/qcom/at803x.c b/drivers/net/phy/qcom/at803x.c
> index 51a132242462..2995b08bac96 100644
> --- a/drivers/net/phy/qcom/at803x.c
> +++ b/drivers/net/phy/qcom/at803x.c
> @@ -20,7 +20,7 @@
>   #include <linux/of.h>
>   #include <linux/phylink.h>
>   #include <linux/reset.h>
> -#include <linux/sfp.h>
> +#include <linux/phy_port.h>
>   #include <dt-bindings/net/qca-ar803x.h>
>   
>   #include "qcom.h"
> @@ -769,58 +769,44 @@ static int at8031_register_regulators(struct phy_device *phydev)
>   	return 0;
>   }
>   
> -static int at8031_sfp_insert(void *upstream, const struct sfp_eeprom_id *id)
> +static int at803x_configure_mii(struct phy_port *port, bool enable,
> +				phy_interface_t interface)
>   {
> -	struct phy_device *phydev = upstream;
> -	__ETHTOOL_DECLARE_LINK_MODE_MASK(phy_support);
> -	__ETHTOOL_DECLARE_LINK_MODE_MASK(sfp_support);
> -	DECLARE_PHY_INTERFACE_MASK(interfaces);
> -	phy_interface_t iface;
> -
> -	linkmode_zero(phy_support);
> -	phylink_set(phy_support, 1000baseX_Full);
> -	phylink_set(phy_support, 1000baseT_Full);
> -	phylink_set(phy_support, Autoneg);
> -	phylink_set(phy_support, Pause);
> -	phylink_set(phy_support, Asym_Pause);
> -
> -	linkmode_zero(sfp_support);
> -	sfp_parse_support(phydev->sfp_bus, id, sfp_support, interfaces);
> -	/* Some modules support 10G modes as well as others we support.
> -	 * Mask out non-supported modes so the correct interface is picked.
> -	 */
> -	linkmode_and(sfp_support, phy_support, sfp_support);
> +	struct phy_device *phydev = port_phydev(port);
>   
> -	if (linkmode_empty(sfp_support)) {
> -		dev_err(&phydev->mdio.dev, "incompatible SFP module inserted\n");
> -		return -EINVAL;
> -	}
> +	if (interface == PHY_INTERFACE_MODE_SGMII)
> +		dev_warn(&phydev->mdio.dev,
> +			 "module may not function if 1000Base-X not supported\n");
> +
> +	return 0;
> +}
>   
> -	iface = sfp_select_interface(phydev->sfp_bus, sfp_support);
> +static const struct phy_port_ops at803x_port_ops = {
> +	.configure_mii = at803x_configure_mii,
> +};
>   
> -	/* Only 1000Base-X is supported by AR8031/8033 as the downstream SerDes
> -	 * interface for use with SFP modules.
> -	 * However, some copper modules detected as having a preferred SGMII
> -	 * interface do default to and function in 1000Base-X mode, so just
> -	 * print a warning and allow such modules, as they may have some chance
> -	 * of working.
> +static int at8031_attach_mii_port(struct phy_device *phydev,
> +				  struct phy_port *port)
> +{
> +	linkmode_zero(port->supported);
> +	phylink_set(port->supported, 1000baseX_Full);
> +	phylink_set(port->supported, 1000baseT_Full);
> +	phylink_set(port->supported, Autoneg);
> +	phylink_set(port->supported, Pause);
> +	phylink_set(port->supported, Asym_Pause);
> +
> +	/* This device doesn't really support SGMII. However, do our best
> +	 * to be compatible with copper modules (that usually require SGMII),
> +	 * in a degraded mode as we only allow 1000BaseT Full
>   	 */
> -	if (iface == PHY_INTERFACE_MODE_SGMII)
> -		dev_warn(&phydev->mdio.dev, "module may not function if 1000Base-X not supported\n");
> -	else if (iface != PHY_INTERFACE_MODE_1000BASEX)
> -		return -EINVAL;
> +	__set_bit(PHY_INTERFACE_MODE_SGMII, port->interfaces);
> +	__set_bit(PHY_INTERFACE_MODE_1000BASEX, port->interfaces);
> +
> +	port->ops = &at803x_port_ops;
>   
>   	return 0;
>   }
>   
> -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)
>   {
>   	struct device_node *node = phydev->mdio.dev.of_node;
> @@ -841,8 +827,7 @@ static int at8031_parse_dt(struct phy_device *phydev)
>   		return ret;
>   	}
>   
> -	/* Only AR8031/8033 support 1000Base-X for SFP modules */
> -	return phy_sfp_probe(phydev, &at8031_sfp_ops);
> +	return 0;
>   }
>   
>   static int at8031_probe(struct phy_device *phydev)
> @@ -1173,6 +1158,7 @@ static struct phy_driver at803x_driver[] = {
>   	.set_tunable		= at803x_set_tunable,
>   	.cable_test_start	= at8031_cable_test_start,
>   	.cable_test_get_status	= at8031_cable_test_get_status,
> +	.attach_mii_port	= at8031_attach_mii_port,
>   }, {
>   	/* Qualcomm Atheros AR8032 */
>   	PHY_ID_MATCH_EXACT(ATH8032_PHY_ID),