* [PATCH] net: sfp: rework upstream interface
@ 2019-11-08 17:39 Russell King
2019-11-08 17:45 ` Russell King - ARM Linux admin
2019-11-10 3:36 ` David Miller
0 siblings, 2 replies; 3+ messages in thread
From: Russell King @ 2019-11-08 17:39 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, Heiner Kallweit; +Cc: David S. Miller, netdev
The current upstream interface is an all-or-nothing, which is
sub-optimal for future changes, as it doesn't allow the upstream driver
to prepare for the SFP module becoming available, as it is at boot.
Switch to a find-sfp-bus, add-upstream, del-upstream, put-sfp-bus
interface structure instead, which allows the upstream driver to
prepare for a module being available as soon as add-upstream is called.
Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
---
drivers/net/phy/phylink.c | 10 +++--
drivers/net/phy/sfp-bus.c | 92 +++++++++++++++++++++++++++------------
include/linux/sfp.h | 25 +++++++----
3 files changed, 88 insertions(+), 39 deletions(-)
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index 4c5e8b4f8d80..2b70b4d50573 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -551,7 +551,7 @@ static int phylink_register_sfp(struct phylink *pl,
struct sfp_bus *bus;
int ret;
- bus = sfp_register_upstream_node(fwnode, pl, &sfp_phylink_ops);
+ bus = sfp_bus_find_fwnode(fwnode);
if (IS_ERR(bus)) {
ret = PTR_ERR(bus);
phylink_err(pl, "unable to attach SFP bus: %d\n", ret);
@@ -560,7 +560,10 @@ static int phylink_register_sfp(struct phylink *pl,
pl->sfp_bus = bus;
- return 0;
+ ret = sfp_bus_add_upstream(bus, pl, &sfp_phylink_ops);
+ sfp_bus_put(bus);
+
+ return ret;
}
/**
@@ -654,8 +657,7 @@ EXPORT_SYMBOL_GPL(phylink_create);
*/
void phylink_destroy(struct phylink *pl)
{
- if (pl->sfp_bus)
- sfp_unregister_upstream(pl->sfp_bus);
+ sfp_bus_del_upstream(pl->sfp_bus);
if (pl->link_gpio)
gpiod_put(pl->link_gpio);
diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
index d037aab6a71d..715d45214e18 100644
--- a/drivers/net/phy/sfp-bus.c
+++ b/drivers/net/phy/sfp-bus.c
@@ -329,10 +329,19 @@ static void sfp_bus_release(struct kref *kref)
kfree(bus);
}
-static void sfp_bus_put(struct sfp_bus *bus)
+/**
+ * sfp_bus_put() - put a reference on the &struct sfp_bus
+ * bus: the &struct sfp_bus found via sfp_bus_find_fwnode()
+ *
+ * Put a reference on the &struct sfp_bus and free the underlying structure
+ * if this was the last reference.
+ */
+void sfp_bus_put(struct sfp_bus *bus)
{
- kref_put_mutex(&bus->kref, sfp_bus_release, &sfp_mutex);
+ if (bus)
+ kref_put_mutex(&bus->kref, sfp_bus_release, &sfp_mutex);
}
+EXPORT_SYMBOL_GPL(sfp_bus_put);
static int sfp_register_bus(struct sfp_bus *bus)
{
@@ -348,11 +357,11 @@ static int sfp_register_bus(struct sfp_bus *bus)
return ret;
}
}
+ bus->registered = true;
bus->socket_ops->attach(bus->sfp);
if (bus->started)
bus->socket_ops->start(bus->sfp);
bus->upstream_ops->attach(bus->upstream, bus);
- bus->registered = true;
return 0;
}
@@ -446,13 +455,12 @@ static void sfp_upstream_clear(struct sfp_bus *bus)
}
/**
- * sfp_register_upstream_node() - parse and register the neighbouring device
+ * sfp_bus_find_fwnode() - parse and locate the SFP bus from fwnode
* @fwnode: firmware node for the parent device (MAC or PHY)
- * @upstream: the upstream private data
- * @ops: the upstream's &struct sfp_upstream_ops
*
- * Parse the parent device's firmware node for a SFP bus, and register the
- * SFP bus using sfp_register_upstream().
+ * Parse the parent device's firmware node for a SFP bus, and locate
+ * the sfp_bus structure, incrementing its reference count. This must
+ * be put via sfp_bus_put() when done.
*
* Returns: on success, a pointer to the sfp_bus structure,
* %NULL if no SFP is specified,
@@ -462,9 +470,7 @@ static void sfp_upstream_clear(struct sfp_bus *bus)
* %-ENOMEM if we failed to allocate the bus.
* an error from the upstream's connect_phy() method.
*/
-struct sfp_bus *sfp_register_upstream_node(struct fwnode_handle *fwnode,
- void *upstream,
- const struct sfp_upstream_ops *ops)
+struct sfp_bus *sfp_bus_find_fwnode(struct fwnode_handle *fwnode)
{
struct fwnode_reference_args ref;
struct sfp_bus *bus;
@@ -482,7 +488,39 @@ struct sfp_bus *sfp_register_upstream_node(struct fwnode_handle *fwnode,
if (!bus)
return ERR_PTR(-ENOMEM);
+ return bus;
+}
+EXPORT_SYMBOL_GPL(sfp_bus_find_fwnode);
+
+/**
+ * sfp_bus_add_upstream() - parse and register the neighbouring device
+ * @bus: the &struct sfp_bus found via sfp_bus_find_fwnode()
+ * @upstream: the upstream private data
+ * @ops: the upstream's &struct sfp_upstream_ops
+ *
+ * Add upstream driver for the SFP bus, and if the bus is complete, register
+ * the SFP bus using sfp_register_upstream(). This takes a reference on the
+ * bus, so it is safe to put the bus after this call.
+ *
+ * Returns: on success, a pointer to the sfp_bus structure,
+ * %NULL if no SFP is specified,
+ * on failure, an error pointer value:
+ * corresponding to the errors detailed for
+ * fwnode_property_get_reference_args().
+ * %-ENOMEM if we failed to allocate the bus.
+ * an error from the upstream's connect_phy() method.
+ */
+int sfp_bus_add_upstream(struct sfp_bus *bus, void *upstream,
+ const struct sfp_upstream_ops *ops)
+{
+ int ret;
+
+ /* If no bus, return success */
+ if (!bus)
+ return 0;
+
rtnl_lock();
+ kref_get(&bus->kref);
bus->upstream_ops = ops;
bus->upstream = upstream;
@@ -495,33 +533,33 @@ struct sfp_bus *sfp_register_upstream_node(struct fwnode_handle *fwnode,
}
rtnl_unlock();
- if (ret) {
+ if (ret)
sfp_bus_put(bus);
- bus = ERR_PTR(ret);
- }
- return bus;
+ return ret;
}
-EXPORT_SYMBOL_GPL(sfp_register_upstream_node);
+EXPORT_SYMBOL_GPL(sfp_bus_add_upstream);
/**
- * sfp_unregister_upstream() - Unregister sfp bus
+ * sfp_bus_del_upstream() - Delete a sfp bus
* @bus: a pointer to the &struct sfp_bus structure for the sfp module
*
- * Unregister a previously registered upstream connection for the SFP
- * module. @bus is returned from sfp_register_upstream().
+ * Delete a previously registered upstream connection for the SFP
+ * module. @bus should have been added by sfp_bus_add_upstream().
*/
-void sfp_unregister_upstream(struct sfp_bus *bus)
+void sfp_bus_del_upstream(struct sfp_bus *bus)
{
- rtnl_lock();
- if (bus->sfp)
- sfp_unregister_bus(bus);
- sfp_upstream_clear(bus);
- rtnl_unlock();
+ if (bus) {
+ rtnl_lock();
+ if (bus->sfp)
+ sfp_unregister_bus(bus);
+ sfp_upstream_clear(bus);
+ rtnl_unlock();
- sfp_bus_put(bus);
+ sfp_bus_put(bus);
+ }
}
-EXPORT_SYMBOL_GPL(sfp_unregister_upstream);
+EXPORT_SYMBOL_GPL(sfp_bus_del_upstream);
/* Socket driver entry points */
int sfp_add_phy(struct sfp_bus *bus, struct phy_device *phydev)
diff --git a/include/linux/sfp.h b/include/linux/sfp.h
index 355a08a76fd4..c8464de7cff5 100644
--- a/include/linux/sfp.h
+++ b/include/linux/sfp.h
@@ -508,10 +508,11 @@ int sfp_get_module_eeprom(struct sfp_bus *bus, struct ethtool_eeprom *ee,
u8 *data);
void sfp_upstream_start(struct sfp_bus *bus);
void sfp_upstream_stop(struct sfp_bus *bus);
-struct sfp_bus *sfp_register_upstream_node(struct fwnode_handle *fwnode,
- void *upstream,
- const struct sfp_upstream_ops *ops);
-void sfp_unregister_upstream(struct sfp_bus *bus);
+void sfp_bus_put(struct sfp_bus *bus);
+struct sfp_bus *sfp_bus_find_fwnode(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);
#else
static inline int sfp_parse_port(struct sfp_bus *bus,
const struct sfp_eeprom_id *id,
@@ -553,14 +554,22 @@ static inline void sfp_upstream_stop(struct sfp_bus *bus)
{
}
-static inline struct sfp_bus *sfp_register_upstream_node(
- struct fwnode_handle *fwnode, void *upstream,
- const struct sfp_upstream_ops *ops)
+static inline void sfp_bus_put(struct sfp_bus *bus)
+{
+}
+
+static inline struct sfp_bus *sfp_bus_find_fwnode(struct fwnode_handle *fwnode)
{
return NULL;
}
-static inline void sfp_unregister_upstream(struct sfp_bus *bus)
+static int sfp_bus_add_upstream(struct sfp_bus *bus, void *upstream,
+ const struct sfp_upstream_ops *ops)
+{
+ return 0;
+}
+
+static inline void sfp_bus_del_upstream(struct sfp_bus *bus)
{
}
#endif
--
2.20.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] net: sfp: rework upstream interface
2019-11-08 17:39 [PATCH] net: sfp: rework upstream interface Russell King
@ 2019-11-08 17:45 ` Russell King - ARM Linux admin
2019-11-10 3:36 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: Russell King - ARM Linux admin @ 2019-11-08 17:45 UTC (permalink / raw)
To: Andrew Lunn, Florian Fainelli, Heiner Kallweit; +Cc: David S. Miller, netdev
Sorry, this should've been for net-next.
On Fri, Nov 08, 2019 at 05:39:29PM +0000, Russell King wrote:
> The current upstream interface is an all-or-nothing, which is
> sub-optimal for future changes, as it doesn't allow the upstream driver
> to prepare for the SFP module becoming available, as it is at boot.
>
> Switch to a find-sfp-bus, add-upstream, del-upstream, put-sfp-bus
> interface structure instead, which allows the upstream driver to
> prepare for a module being available as soon as add-upstream is called.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
> ---
> drivers/net/phy/phylink.c | 10 +++--
> drivers/net/phy/sfp-bus.c | 92 +++++++++++++++++++++++++++------------
> include/linux/sfp.h | 25 +++++++----
> 3 files changed, 88 insertions(+), 39 deletions(-)
>
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index 4c5e8b4f8d80..2b70b4d50573 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -551,7 +551,7 @@ static int phylink_register_sfp(struct phylink *pl,
> struct sfp_bus *bus;
> int ret;
>
> - bus = sfp_register_upstream_node(fwnode, pl, &sfp_phylink_ops);
> + bus = sfp_bus_find_fwnode(fwnode);
> if (IS_ERR(bus)) {
> ret = PTR_ERR(bus);
> phylink_err(pl, "unable to attach SFP bus: %d\n", ret);
> @@ -560,7 +560,10 @@ static int phylink_register_sfp(struct phylink *pl,
>
> pl->sfp_bus = bus;
>
> - return 0;
> + ret = sfp_bus_add_upstream(bus, pl, &sfp_phylink_ops);
> + sfp_bus_put(bus);
> +
> + return ret;
> }
>
> /**
> @@ -654,8 +657,7 @@ EXPORT_SYMBOL_GPL(phylink_create);
> */
> void phylink_destroy(struct phylink *pl)
> {
> - if (pl->sfp_bus)
> - sfp_unregister_upstream(pl->sfp_bus);
> + sfp_bus_del_upstream(pl->sfp_bus);
> if (pl->link_gpio)
> gpiod_put(pl->link_gpio);
>
> diff --git a/drivers/net/phy/sfp-bus.c b/drivers/net/phy/sfp-bus.c
> index d037aab6a71d..715d45214e18 100644
> --- a/drivers/net/phy/sfp-bus.c
> +++ b/drivers/net/phy/sfp-bus.c
> @@ -329,10 +329,19 @@ static void sfp_bus_release(struct kref *kref)
> kfree(bus);
> }
>
> -static void sfp_bus_put(struct sfp_bus *bus)
> +/**
> + * sfp_bus_put() - put a reference on the &struct sfp_bus
> + * bus: the &struct sfp_bus found via sfp_bus_find_fwnode()
> + *
> + * Put a reference on the &struct sfp_bus and free the underlying structure
> + * if this was the last reference.
> + */
> +void sfp_bus_put(struct sfp_bus *bus)
> {
> - kref_put_mutex(&bus->kref, sfp_bus_release, &sfp_mutex);
> + if (bus)
> + kref_put_mutex(&bus->kref, sfp_bus_release, &sfp_mutex);
> }
> +EXPORT_SYMBOL_GPL(sfp_bus_put);
>
> static int sfp_register_bus(struct sfp_bus *bus)
> {
> @@ -348,11 +357,11 @@ static int sfp_register_bus(struct sfp_bus *bus)
> return ret;
> }
> }
> + bus->registered = true;
> bus->socket_ops->attach(bus->sfp);
> if (bus->started)
> bus->socket_ops->start(bus->sfp);
> bus->upstream_ops->attach(bus->upstream, bus);
> - bus->registered = true;
> return 0;
> }
>
> @@ -446,13 +455,12 @@ static void sfp_upstream_clear(struct sfp_bus *bus)
> }
>
> /**
> - * sfp_register_upstream_node() - parse and register the neighbouring device
> + * sfp_bus_find_fwnode() - parse and locate the SFP bus from fwnode
> * @fwnode: firmware node for the parent device (MAC or PHY)
> - * @upstream: the upstream private data
> - * @ops: the upstream's &struct sfp_upstream_ops
> *
> - * Parse the parent device's firmware node for a SFP bus, and register the
> - * SFP bus using sfp_register_upstream().
> + * Parse the parent device's firmware node for a SFP bus, and locate
> + * the sfp_bus structure, incrementing its reference count. This must
> + * be put via sfp_bus_put() when done.
> *
> * Returns: on success, a pointer to the sfp_bus structure,
> * %NULL if no SFP is specified,
> @@ -462,9 +470,7 @@ static void sfp_upstream_clear(struct sfp_bus *bus)
> * %-ENOMEM if we failed to allocate the bus.
> * an error from the upstream's connect_phy() method.
> */
> -struct sfp_bus *sfp_register_upstream_node(struct fwnode_handle *fwnode,
> - void *upstream,
> - const struct sfp_upstream_ops *ops)
> +struct sfp_bus *sfp_bus_find_fwnode(struct fwnode_handle *fwnode)
> {
> struct fwnode_reference_args ref;
> struct sfp_bus *bus;
> @@ -482,7 +488,39 @@ struct sfp_bus *sfp_register_upstream_node(struct fwnode_handle *fwnode,
> if (!bus)
> return ERR_PTR(-ENOMEM);
>
> + return bus;
> +}
> +EXPORT_SYMBOL_GPL(sfp_bus_find_fwnode);
> +
> +/**
> + * sfp_bus_add_upstream() - parse and register the neighbouring device
> + * @bus: the &struct sfp_bus found via sfp_bus_find_fwnode()
> + * @upstream: the upstream private data
> + * @ops: the upstream's &struct sfp_upstream_ops
> + *
> + * Add upstream driver for the SFP bus, and if the bus is complete, register
> + * the SFP bus using sfp_register_upstream(). This takes a reference on the
> + * bus, so it is safe to put the bus after this call.
> + *
> + * Returns: on success, a pointer to the sfp_bus structure,
> + * %NULL if no SFP is specified,
> + * on failure, an error pointer value:
> + * corresponding to the errors detailed for
> + * fwnode_property_get_reference_args().
> + * %-ENOMEM if we failed to allocate the bus.
> + * an error from the upstream's connect_phy() method.
> + */
> +int sfp_bus_add_upstream(struct sfp_bus *bus, void *upstream,
> + const struct sfp_upstream_ops *ops)
> +{
> + int ret;
> +
> + /* If no bus, return success */
> + if (!bus)
> + return 0;
> +
> rtnl_lock();
> + kref_get(&bus->kref);
> bus->upstream_ops = ops;
> bus->upstream = upstream;
>
> @@ -495,33 +533,33 @@ struct sfp_bus *sfp_register_upstream_node(struct fwnode_handle *fwnode,
> }
> rtnl_unlock();
>
> - if (ret) {
> + if (ret)
> sfp_bus_put(bus);
> - bus = ERR_PTR(ret);
> - }
>
> - return bus;
> + return ret;
> }
> -EXPORT_SYMBOL_GPL(sfp_register_upstream_node);
> +EXPORT_SYMBOL_GPL(sfp_bus_add_upstream);
>
> /**
> - * sfp_unregister_upstream() - Unregister sfp bus
> + * sfp_bus_del_upstream() - Delete a sfp bus
> * @bus: a pointer to the &struct sfp_bus structure for the sfp module
> *
> - * Unregister a previously registered upstream connection for the SFP
> - * module. @bus is returned from sfp_register_upstream().
> + * Delete a previously registered upstream connection for the SFP
> + * module. @bus should have been added by sfp_bus_add_upstream().
> */
> -void sfp_unregister_upstream(struct sfp_bus *bus)
> +void sfp_bus_del_upstream(struct sfp_bus *bus)
> {
> - rtnl_lock();
> - if (bus->sfp)
> - sfp_unregister_bus(bus);
> - sfp_upstream_clear(bus);
> - rtnl_unlock();
> + if (bus) {
> + rtnl_lock();
> + if (bus->sfp)
> + sfp_unregister_bus(bus);
> + sfp_upstream_clear(bus);
> + rtnl_unlock();
>
> - sfp_bus_put(bus);
> + sfp_bus_put(bus);
> + }
> }
> -EXPORT_SYMBOL_GPL(sfp_unregister_upstream);
> +EXPORT_SYMBOL_GPL(sfp_bus_del_upstream);
>
> /* Socket driver entry points */
> int sfp_add_phy(struct sfp_bus *bus, struct phy_device *phydev)
> diff --git a/include/linux/sfp.h b/include/linux/sfp.h
> index 355a08a76fd4..c8464de7cff5 100644
> --- a/include/linux/sfp.h
> +++ b/include/linux/sfp.h
> @@ -508,10 +508,11 @@ int sfp_get_module_eeprom(struct sfp_bus *bus, struct ethtool_eeprom *ee,
> u8 *data);
> void sfp_upstream_start(struct sfp_bus *bus);
> void sfp_upstream_stop(struct sfp_bus *bus);
> -struct sfp_bus *sfp_register_upstream_node(struct fwnode_handle *fwnode,
> - void *upstream,
> - const struct sfp_upstream_ops *ops);
> -void sfp_unregister_upstream(struct sfp_bus *bus);
> +void sfp_bus_put(struct sfp_bus *bus);
> +struct sfp_bus *sfp_bus_find_fwnode(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);
> #else
> static inline int sfp_parse_port(struct sfp_bus *bus,
> const struct sfp_eeprom_id *id,
> @@ -553,14 +554,22 @@ static inline void sfp_upstream_stop(struct sfp_bus *bus)
> {
> }
>
> -static inline struct sfp_bus *sfp_register_upstream_node(
> - struct fwnode_handle *fwnode, void *upstream,
> - const struct sfp_upstream_ops *ops)
> +static inline void sfp_bus_put(struct sfp_bus *bus)
> +{
> +}
> +
> +static inline struct sfp_bus *sfp_bus_find_fwnode(struct fwnode_handle *fwnode)
> {
> return NULL;
> }
>
> -static inline void sfp_unregister_upstream(struct sfp_bus *bus)
> +static int sfp_bus_add_upstream(struct sfp_bus *bus, void *upstream,
> + const struct sfp_upstream_ops *ops)
> +{
> + return 0;
> +}
> +
> +static inline void sfp_bus_del_upstream(struct sfp_bus *bus)
> {
> }
> #endif
> --
> 2.20.1
>
>
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] net: sfp: rework upstream interface
2019-11-08 17:39 [PATCH] net: sfp: rework upstream interface Russell King
2019-11-08 17:45 ` Russell King - ARM Linux admin
@ 2019-11-10 3:36 ` David Miller
1 sibling, 0 replies; 3+ messages in thread
From: David Miller @ 2019-11-10 3:36 UTC (permalink / raw)
To: rmk+kernel; +Cc: andrew, f.fainelli, hkallweit1, netdev
From: Russell King <rmk+kernel@armlinux.org.uk>
Date: Fri, 08 Nov 2019 17:39:29 +0000
> The current upstream interface is an all-or-nothing, which is
> sub-optimal for future changes, as it doesn't allow the upstream driver
> to prepare for the SFP module becoming available, as it is at boot.
>
> Switch to a find-sfp-bus, add-upstream, del-upstream, put-sfp-bus
> interface structure instead, which allows the upstream driver to
> prepare for a module being available as soon as add-upstream is called.
>
> Signed-off-by: Russell King <rmk+kernel@armlinux.org.uk>
Applied to net-next, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-11-10 3:37 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-08 17:39 [PATCH] net: sfp: rework upstream interface Russell King
2019-11-08 17:45 ` Russell King - ARM Linux admin
2019-11-10 3:36 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).