[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <3690479.R56niFO833@fw-rgant>
Date: Mon, 12 May 2025 10:52:55 +0200
From: Romain Gantois <romain.gantois@...tlin.com>
To: davem@...emloft.net, Maxime Chevallier <maxime.chevallier@...tlin.com>
Cc: Maxime Chevallier <maxime.chevallier@...tlin.com>, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-arm-msm@...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>,
Heiner Kallweit <hkallweit1@...il.com>,
Vladimir Oltean <vladimir.oltean@....com>,
Köry Maincent <kory.maincent@...tlin.com>,
Marek Behún <kabel@...nel.org>,
Oleksij Rempel <o.rempel@...gutronix.de>,
Nicolò Veronese <nicveronese@...il.com>,
Simon Horman <horms@...nel.org>, mwojtas@...omium.org,
Antoine Tenart <atenart@...nel.org>, devicetree@...r.kernel.org,
Conor Dooley <conor+dt@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Rob Herring <robh@...nel.org>, Daniel Golle <daniel@...rotopia.org>,
Dimitri Fedrau <dimitri.fedrau@...bherr.com>
Subject:
Re: [PATCH net-next v6 12/14] net: phy: Only rely on phy_port for PHY-driven
SFP
On Wednesday, 7 May 2025 15:53:28 CEST Maxime Chevallier wrote:
> Now that all PHY drivers that support downstream SFP have been converted
> to phy_port serdes handling, we can make the generic PHY SFP handling
> mandatory, thus making all phylib sfp helpers static.
>
> Signed-off-by: Maxime Chevallier <maxime.chevallier@...tlin.com>
> ---
> drivers/net/phy/phy_device.c | 28 +++++++++-------------------
> include/linux/phy.h | 6 ------
> 2 files changed, 9 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index aca3a47cbb66..7f319526a7fe 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -1384,7 +1384,7 @@ static DEVICE_ATTR_RO(phy_standalone);
> *
> * Return: 0 on success, otherwise a negative error code.
> */
> -int phy_sfp_connect_phy(void *upstream, struct phy_device *phy)
> +static int phy_sfp_connect_phy(void *upstream, struct phy_device *phy)
> {
> struct phy_device *phydev = upstream;
> struct net_device *dev = phydev->attached_dev;
> @@ -1394,7 +1394,6 @@ int phy_sfp_connect_phy(void *upstream, struct
> phy_device *phy)
>
> return 0;
> }
> -EXPORT_SYMBOL(phy_sfp_connect_phy);
>
> /**
> * phy_sfp_disconnect_phy - Disconnect the SFP module's PHY from the
> upstream PHY @@ -1406,7 +1405,7 @@ EXPORT_SYMBOL(phy_sfp_connect_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)
> +static void phy_sfp_disconnect_phy(void *upstream, struct phy_device *phy)
> {
> struct phy_device *phydev = upstream;
> struct net_device *dev = phydev->attached_dev;
> @@ -1414,7 +1413,6 @@ void phy_sfp_disconnect_phy(void *upstream, struct
> phy_device *phy) 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
> @@ -1423,7 +1421,7 @@ EXPORT_SYMBOL(phy_sfp_disconnect_phy);
> *
> * This is used to fill in the sfp_upstream_ops .attach member.
> */
> -void phy_sfp_attach(void *upstream, struct sfp_bus *bus)
> +static void phy_sfp_attach(void *upstream, struct sfp_bus *bus)
> {
> struct phy_device *phydev = upstream;
>
> @@ -1431,7 +1429,6 @@ void phy_sfp_attach(void *upstream, struct sfp_bus
> *bus) phydev->attached_dev->sfp_bus = bus;
> phydev->sfp_bus_attached = true;
> }
> -EXPORT_SYMBOL(phy_sfp_attach);
>
> /**
> * phy_sfp_detach - detach the SFP bus from the PHY upstream network device
> @@ -1440,7 +1437,7 @@ EXPORT_SYMBOL(phy_sfp_attach);
> *
> * This is used to fill in the sfp_upstream_ops .detach member.
> */
> -void phy_sfp_detach(void *upstream, struct sfp_bus *bus)
> +static void phy_sfp_detach(void *upstream, struct sfp_bus *bus)
> {
> struct phy_device *phydev = upstream;
>
> @@ -1448,7 +1445,6 @@ void phy_sfp_detach(void *upstream, struct sfp_bus
> *bus) phydev->attached_dev->sfp_bus = NULL;
> phydev->sfp_bus_attached = false;
> }
> -EXPORT_SYMBOL(phy_sfp_detach);
>
> static int phy_sfp_module_insert(void *upstream, const struct sfp_eeprom_id
> *id) {
> @@ -1591,10 +1587,8 @@ static int phy_setup_sfp_port(struct phy_device
> *phydev) /**
> * phy_sfp_probe - probe for a SFP cage attached to this PHY device
> * @phydev: Pointer to phy_device
> - * @ops: SFP's upstream operations
> */
> -int phy_sfp_probe(struct phy_device *phydev,
> - const struct sfp_upstream_ops *ops)
> +static int phy_sfp_probe(struct phy_device *phydev)
> {
> struct sfp_bus *bus;
> int ret = 0;
> @@ -1606,7 +1600,7 @@ int phy_sfp_probe(struct phy_device *phydev,
>
> phydev->sfp_bus = bus;
>
> - ret = sfp_bus_add_upstream(bus, phydev, ops);
> + ret = sfp_bus_add_upstream(bus, phydev, &sfp_phydev_ops);
> sfp_bus_put(bus);
> }
>
> @@ -1615,7 +1609,6 @@ int phy_sfp_probe(struct phy_device *phydev,
>
> return ret;
> }
> -EXPORT_SYMBOL(phy_sfp_probe);
>
> static bool phy_drv_supports_irq(const struct phy_driver *phydrv)
> {
> @@ -3432,12 +3425,9 @@ static int phy_setup_ports(struct phy_device *phydev)
> if (ret)
> return ret;
>
> - /* Use generic SFP probing only if the driver didn't do so already */
> - if (!phydev->sfp_bus) {
Alright, since you removed this, my earlier review comment about potentially
making phy_sfp_probe() legacy doesn't apply.
> - ret = phy_sfp_probe(phydev, &sfp_phydev_ops);
> - if (ret)
> - goto out;
> - }
> + ret = phy_sfp_probe(phydev);
> + if (ret)
> + goto out;
>
> if (phydev->n_ports < phydev->max_n_ports) {
> ret = phy_default_setup_single_port(phydev);
> diff --git a/include/linux/phy.h b/include/linux/phy.h
> index aef13fab8882..4df1c951dcf2 100644
> --- a/include/linux/phy.h
> +++ b/include/linux/phy.h
> @@ -1796,12 +1796,6 @@ 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 speed);
> -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,
> - const struct sfp_upstream_ops *ops);
> struct phy_device *phy_attach(struct net_device *dev, const char *bus_id,
> phy_interface_t interface);
> struct phy_device *phy_find_first(struct mii_bus *bus);
Thanks!
--
Romain Gantois, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)
Powered by blists - more mailing lists