| Message ID | 20231117162323.626979-4-maxime.chevallier@bootlin.com (mailing list archive) |
|---|---|
| State | RFC |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Introduce PHY listing and link_topology tracking | expand |
> +/** > + * 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 > + */ > +int phy_sfp_connect_phy(void *upstream, struct phy_device *phy) > +{ > + struct phy_device *phydev = upstream; Will this function only ever be called from a PHY driver? If so, we know upstream is PHY. So we can avoid using void * and make it a struct phy_device *. Andrew
On Tue, Nov 21, 2023 at 01:57:24AM +0100, Andrew Lunn wrote: > > +/** > > + * 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 > > + */ > > +int phy_sfp_connect_phy(void *upstream, struct phy_device *phy) > > +{ > > + struct phy_device *phydev = upstream; > > Will this function only ever be called from a PHY driver? If so, we > know upstream is PHY. So we can avoid using void * and make it a > struct phy_device *. No. This function is hooked into the .connect_phy method of sfp_upstream_ops, and the SFP bus layer has no idea what the "upstream" is. In this case, it's a PHY. In the case of phylink, it's the phylink struct. So no, "struct phy_device *" here will cause build errors.
On Tue, Nov 21, 2023 at 10:08:00AM +0000, Russell King (Oracle) wrote: > On Tue, Nov 21, 2023 at 01:57:24AM +0100, Andrew Lunn wrote: > > > +/** > > > + * 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 > > > + */ > > > +int phy_sfp_connect_phy(void *upstream, struct phy_device *phy) > > > +{ > > > + struct phy_device *phydev = upstream; > > > > Will this function only ever be called from a PHY driver? If so, we > > know upstream is PHY. So we can avoid using void * and make it a > > struct phy_device *. > > No. This function is hooked into the .connect_phy method of > sfp_upstream_ops, and the SFP bus layer has no idea what the > "upstream" is. In this case, it's a PHY. In the case of phylink, > it's the phylink struct. So no, "struct phy_device *" here will > cause build errors. O.K, thanks for checking this. It would of been nice to have some compile time checking what is passed is what we expect in terms of type, but C does not allow that in this case. Andrew
diff --git a/drivers/net/phy/at803x.c b/drivers/net/phy/at803x.c index 37fb033e1c29..9b1659b03aa5 100644 --- a/drivers/net/phy/at803x.c +++ b/drivers/net/phy/at803x.c @@ -730,6 +730,8 @@ static const struct sfp_upstream_ops at803x_sfp_ops = { .attach = phy_sfp_attach, .detach = phy_sfp_detach, .module_insert = at803x_sfp_insert, + .connect_phy = phy_sfp_connect_phy, + .disconnect_phy = phy_sfp_disconnect_phy, }; static int at803x_parse_dt(struct phy_device *phydev) diff --git a/drivers/net/phy/marvell-88x2222.c b/drivers/net/phy/marvell-88x2222.c index e3aa30dad2e6..3f77bbc7e04f 100644 --- a/drivers/net/phy/marvell-88x2222.c +++ b/drivers/net/phy/marvell-88x2222.c @@ -555,6 +555,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 eba652a4c1d8..674e29bce2cc 100644 --- a/drivers/net/phy/marvell.c +++ b/drivers/net/phy/marvell.c @@ -3254,6 +3254,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 d4bb90d76881..a88ebc0a6be5 100644 --- a/drivers/net/phy/marvell10g.c +++ b/drivers/net/phy/marvell10g.c @@ -496,6 +496,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 b38b0dc00ef7..3d68e1f8d785 100644 --- a/drivers/net/phy/phy_device.c +++ b/drivers/net/phy/phy_device.c @@ -1361,6 +1361,46 @@ 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 + */ +int phy_sfp_connect_phy(void *upstream, struct phy_device *phy) +{ + struct phy_device *phydev = upstream; + struct link_topology *lt = phy_get_link_topology(phydev); + + if (lt) + return link_topo_add_phy(lt, 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 link_topology *lt = phy_get_link_topology(phydev); + + if (lt) + link_topo_del_phy(lt, 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/include/linux/phy.h b/include/linux/phy.h index d698180b1df0..2d3ae93250e6 100644 --- a/include/linux/phy.h +++ b/include/linux/phy.h @@ -1719,6 +1719,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,
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> --- drivers/net/phy/at803x.c | 2 ++ 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 | 40 +++++++++++++++++++++++++++++++ include/linux/phy.h | 2 ++ 6 files changed, 50 insertions(+)