Knowing the bus name is helpful when we want to expose the link topology
to userspace, add a helper to return the SFP bus name.
Signed-off-by: Maxime Chevallier <[email protected]>
---
drivers/net/phy/sfp-bus.c | 9 +++++++++
include/linux/sfp.h | 6 ++++++
2 files changed, 15 insertions(+)
diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
index f42e9a082935..835fb2271b12 100644
--- a/drivers/net/phy/sfp-bus.c
+++ b/drivers/net/phy/sfp-bus.c
@@ -859,3 +859,12 @@ void sfp_unregister_socket(struct sfp_bus *bus)
sfp_bus_put(bus);
}
EXPORT_SYMBOL_GPL(sfp_unregister_socket);
+
+const char *sfp_get_name(struct sfp_bus *bus)
+{
+ if (bus->sfp_dev)
+ return dev_name(bus->sfp_dev);
+
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(sfp_get_name);
diff --git a/include/linux/sfp.h b/include/linux/sfp.h
index 0573e53b0c11..96f7644908bd 100644
--- a/include/linux/sfp.h
+++ b/include/linux/sfp.h
@@ -570,6 +570,7 @@ struct sfp_bus *sfp_bus_find_fwnode(const struct fwnode_handle *fwnode);
int sfp_bus_add_upstream(struct sfp_bus *bus, void *upstream,
const struct sfp_upstream_ops *ops);
void sfp_bus_del_upstream(struct sfp_bus *bus);
+const char *sfp_get_name(struct sfp_bus *bus);
#else
static inline int sfp_parse_port(struct sfp_bus *bus,
const struct sfp_eeprom_id *id,
@@ -648,6 +649,11 @@ static inline int sfp_bus_add_upstream(struct sfp_bus *bus, void *upstream,
static inline void sfp_bus_del_upstream(struct sfp_bus *bus)
{
}
+
+static const char *sfp_get_name(struct sfp_bus *bus)
+{
+ return NULL;
+}
#endif
#endif
--
2.41.0
> +const char *sfp_get_name(struct sfp_bus *bus)
> +{
> + if (bus->sfp_dev)
> + return dev_name(bus->sfp_dev);
> +
> + return NULL;
> +}
Locking? Do you assume rtnl? Does this function need to take rtnl?
Andrew
On Tue, Nov 21, 2023 at 02:00:58AM +0100, Andrew Lunn wrote:
> > +const char *sfp_get_name(struct sfp_bus *bus)
> > +{
> > + if (bus->sfp_dev)
> > + return dev_name(bus->sfp_dev);
> > +
> > + return NULL;
> > +}
>
> Locking? Do you assume rtnl? Does this function need to take rtnl?
Yes, rtnl needs to be held to safely access bus->sfp_dev, and that
either needs to happen in this function, or be documented as being
requried (and ASSERT_RTNL() added here.)
The reason is that sfp_dev is the SFP socket device which can be
unbound via sfp_unregister_socket(), which will set bus->sfp_dev to
NULL. This could race with the above.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi Andrew, Russell,
On Tue, 21 Nov 2023 10:20:43 +0000
"Russell King (Oracle)" <[email protected]> wrote:
> On Tue, Nov 21, 2023 at 02:00:58AM +0100, Andrew Lunn wrote:
> > > +const char *sfp_get_name(struct sfp_bus *bus)
> > > +{
> > > + if (bus->sfp_dev)
> > > + return dev_name(bus->sfp_dev);
> > > +
> > > + return NULL;
> > > +}
> >
> > Locking? Do you assume rtnl? Does this function need to take rtnl?
>
> Yes, rtnl needs to be held to safely access bus->sfp_dev, and that
> either needs to happen in this function, or be documented as being
> requried (and ASSERT_RTNL() added here.)
>
> The reason is that sfp_dev is the SFP socket device which can be
> unbound via sfp_unregister_socket(), which will set bus->sfp_dev to
> NULL. This could race with the above.
>
That's right, I'll add an assert and document it, thanks for spotting
this.
Maxime