[net-next PATCH 2/6] net: pcs: Implement OF support for PCS driver

Christian Marangi posted 6 patches 9 months ago
[net-next PATCH 2/6] net: pcs: Implement OF support for PCS driver
Posted by Christian Marangi 9 months ago
Implement the foundation of OF support for PCS driver.

To support this, implement a simple Provider API where a PCS driver can
expose multiple PCS with an xlate .get function.

PCS driver will have to call of_pcs_add_provider() and pass the device
node pointer and a xlate function to return the correct PCS for the
requested interface and the passed #pcs-cells.

This will register the PCS in a global list of providers so that
consumer can access it.

Consumer will then use of_pcs_get() to get the actual PCS by passing the
device_node pointer, the index for #pcs-cells and the requested
interface.

For simple implementation where #pcs-cells is 0 and the PCS driver
expose a single PCS, the xlate function of_pcs_simple_get() is
provided. In such case the passed interface is ignored and is expected
that the PCS supports any interface mode supported by the MAC.

For advanced implementation a custom xlate function is required. Such
function should return an error if the PCS is not supported for the
requested interface type.

This is needed for the correct function of of_phylink_mac_select_pcs()
later described.

PCS driver on removal should first call phylink_pcs_release() on every
PCS the driver provides and then correctly delete as a provider with
the usage of of_pcs_del_provider().

A generic function for .mac_select_pcs is provided for any MAC driver
that will declare PCS in DT, of_phylink_mac_select_pcs().
This function will parse "pcs-handle" property and will try every PCS
declared in DT until one that supports the requested interface type is
found. This works by leveraging the return value of the xlate function
returned by of_pcs_get() and checking if it's an ERROR or NULL, in such
case the next PCS in the phandle array is tested.

Some additional helper are provided for xlate functions,
pcs_supports_interface() as a simple function to check if the requested
interface is supported by the PCS and phylink_pcs_release() to release a
PCS from a phylink instance.

Co-developed-by: Daniel Golle <daniel@makrotopia.org>
Signed-off-by: Daniel Golle <daniel@makrotopia.org>
Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
---
 drivers/net/pcs/Kconfig          |   7 ++
 drivers/net/pcs/Makefile         |   1 +
 drivers/net/pcs/pcs.c            | 185 +++++++++++++++++++++++++++++++
 drivers/net/phy/phylink.c        |  21 ++++
 include/linux/pcs/pcs-provider.h |  46 ++++++++
 include/linux/pcs/pcs.h          |  62 +++++++++++
 include/linux/phylink.h          |   2 +
 7 files changed, 324 insertions(+)
 create mode 100644 drivers/net/pcs/pcs.c
 create mode 100644 include/linux/pcs/pcs-provider.h
 create mode 100644 include/linux/pcs/pcs.h

diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
index f6aa437473de..8c3b720de6fd 100644
--- a/drivers/net/pcs/Kconfig
+++ b/drivers/net/pcs/Kconfig
@@ -5,6 +5,13 @@
 
 menu "PCS device drivers"
 
+config OF_PCS
+	tristate
+	depends on OF
+	depends on PHYLINK
+	help
+		OpenFirmware PCS accessors
+
 config PCS_XPCS
 	tristate "Synopsys DesignWare Ethernet XPCS"
 	select PHYLINK
diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
index 4f7920618b90..29881f0f981f 100644
--- a/drivers/net/pcs/Makefile
+++ b/drivers/net/pcs/Makefile
@@ -1,6 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 # Makefile for Linux PCS drivers
 
+obj-$(CONFIG_OF_PCS)		+= pcs.o
 pcs_xpcs-$(CONFIG_PCS_XPCS)	:= pcs-xpcs.o pcs-xpcs-plat.o \
 				   pcs-xpcs-nxp.o pcs-xpcs-wx.o
 
diff --git a/drivers/net/pcs/pcs.c b/drivers/net/pcs/pcs.c
new file mode 100644
index 000000000000..af04a76ef825
--- /dev/null
+++ b/drivers/net/pcs/pcs.c
@@ -0,0 +1,185 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/mutex.h>
+#include <linux/of.h>
+#include <linux/phylink.h>
+#include <linux/pcs/pcs.h>
+#include <linux/pcs/pcs-provider.h>
+
+struct of_pcs_provider {
+	struct list_head link;
+
+	struct device_node *node;
+	struct phylink_pcs *(*get)(struct of_phandle_args *pcsspec,
+				   void *data,
+				   phy_interface_t interface);
+
+	void *data;
+};
+
+static LIST_HEAD(of_pcs_providers);
+static DEFINE_MUTEX(of_pcs_mutex);
+
+struct phylink_pcs *of_pcs_simple_get(struct of_phandle_args *pcsspec, void *data,
+				      phy_interface_t interface)
+{
+	struct phylink_pcs *pcs = data;
+
+	if (!pcs_supports_interface(pcs, interface))
+		return ERR_PTR(-EOPNOTSUPP);
+
+	return data;
+}
+EXPORT_SYMBOL_GPL(of_pcs_simple_get);
+
+int of_pcs_add_provider(struct device_node *np,
+			struct phylink_pcs *(*get)(struct of_phandle_args *pcsspec,
+						   void *data,
+						   phy_interface_t interface),
+			void *data)
+{
+	struct of_pcs_provider *pp;
+
+	if (!np)
+		return 0;
+
+	pp = kzalloc(sizeof(*pp), GFP_KERNEL);
+	if (!pp)
+		return -ENOMEM;
+
+	pp->node = of_node_get(np);
+	pp->data = data;
+	pp->get = get;
+
+	mutex_lock(&of_pcs_mutex);
+	list_add(&pp->link, &of_pcs_providers);
+	mutex_unlock(&of_pcs_mutex);
+	pr_debug("Added pcs provider from %pOF\n", np);
+
+	fwnode_dev_initialized(&np->fwnode, true);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(of_pcs_add_provider);
+
+void of_pcs_del_provider(struct device_node *np)
+{
+	struct of_pcs_provider *pp;
+
+	if (!np)
+		return;
+
+	mutex_lock(&of_pcs_mutex);
+	list_for_each_entry(pp, &of_pcs_providers, link) {
+		if (pp->node == np) {
+			list_del(&pp->link);
+			fwnode_dev_initialized(&np->fwnode, false);
+			of_node_put(pp->node);
+			kfree(pp);
+			break;
+		}
+	}
+	mutex_unlock(&of_pcs_mutex);
+}
+EXPORT_SYMBOL_GPL(of_pcs_del_provider);
+
+static int of_parse_pcsspec(const struct device_node *np, int index,
+			    const char *name, struct of_phandle_args *out_args)
+{
+	int ret = -ENOENT;
+
+	if (!np)
+		return -ENOENT;
+
+	if (name)
+		index = of_property_match_string(np, "pcs-names", name);
+
+	ret = of_parse_phandle_with_args(np, "pcs-handle", "#pcs-cells",
+					 index, out_args);
+	if (ret || (name && index < 0))
+		return ret;
+
+	return 0;
+}
+
+static struct phylink_pcs *
+of_pcs_get_from_pcsspec(struct of_phandle_args *pcsspec,
+			phy_interface_t interface)
+{
+	struct of_pcs_provider *provider;
+	struct phylink_pcs *pcs = ERR_PTR(-EPROBE_DEFER);
+
+	if (!pcsspec)
+		return ERR_PTR(-EINVAL);
+
+	mutex_lock(&of_pcs_mutex);
+	list_for_each_entry(provider, &of_pcs_providers, link) {
+		if (provider->node == pcsspec->np) {
+			pcs = provider->get(pcsspec, provider->data,
+					    interface);
+			if (!IS_ERR(pcs))
+				break;
+		}
+	}
+	mutex_unlock(&of_pcs_mutex);
+
+	return pcs;
+}
+
+static struct phylink_pcs *__of_pcs_get(struct device_node *np, int index,
+					const char *con_id,
+					phy_interface_t interface)
+{
+	struct of_phandle_args pcsspec;
+	struct phylink_pcs *pcs;
+	int ret;
+
+	ret = of_parse_pcsspec(np, index, con_id, &pcsspec);
+	if (ret)
+		return ERR_PTR(ret);
+
+	pcs = of_pcs_get_from_pcsspec(&pcsspec, interface);
+	of_node_put(pcsspec.np);
+
+	return pcs;
+}
+
+struct phylink_pcs *of_pcs_get(struct device_node *np, int index,
+			       phy_interface_t interface)
+{
+	return __of_pcs_get(np, index, NULL, interface);
+}
+EXPORT_SYMBOL_GPL(of_pcs_get);
+
+struct phylink_pcs *of_phylink_mac_select_pcs(struct phylink_config *config,
+					      phy_interface_t interface)
+{
+	int i, count;
+	struct device *dev = config->dev;
+	struct device_node *np = dev->of_node;
+	struct phylink_pcs *pcs = ERR_PTR(-ENODEV);
+
+	/* To enable using_mac_select_pcs on phylink_create */
+	if (interface == PHY_INTERFACE_MODE_NA)
+		return NULL;
+
+	/* Reject configuring PCS with Internal mode */
+	if (interface == PHY_INTERFACE_MODE_INTERNAL)
+		return ERR_PTR(-EINVAL);
+
+	if (!of_property_present(np, "pcs-handle"))
+		return pcs;
+
+	count = of_count_phandle_with_args(np, "pcs-handle", "#pcs-cells");
+	if (count < 0)
+		return ERR_PTR(count);
+
+	for (i = 0; i < count; i++) {
+		pcs = of_pcs_get(np, i, interface);
+		if (!IS_ERR_OR_NULL(pcs))
+			return pcs;
+	}
+
+	return pcs;
+}
+EXPORT_SYMBOL_GPL(of_phylink_mac_select_pcs);
diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
index eef1712ec22c..7f71547e89fe 100644
--- a/drivers/net/phy/phylink.c
+++ b/drivers/net/phy/phylink.c
@@ -1130,6 +1130,27 @@ int phylink_pcs_pre_init(struct phylink *pl, struct phylink_pcs *pcs)
 }
 EXPORT_SYMBOL_GPL(phylink_pcs_pre_init);
 
+/**
+ * phylink_pcs_release() - release a PCS
+ * @pl: a pointer to &struct phylink_pcs
+ *
+ * PCS provider can use this to release a PCS from a phylink
+ * instance by stopping the attached netdev. This is only done
+ * if the PCS is actually attached to a phylink, otherwise is
+ * ignored.
+ */
+void phylink_pcs_release(struct phylink_pcs *pcs)
+{
+	struct phylink *pl = pcs->phylink;
+
+	if (pl) {
+		rtnl_lock();
+		dev_close(pl->netdev);
+		rtnl_unlock();
+	}
+}
+EXPORT_SYMBOL_GPL(phylink_pcs_release);
+
 static void phylink_mac_config(struct phylink *pl,
 			       const struct phylink_link_state *state)
 {
diff --git a/include/linux/pcs/pcs-provider.h b/include/linux/pcs/pcs-provider.h
new file mode 100644
index 000000000000..0172d0286f07
--- /dev/null
+++ b/include/linux/pcs/pcs-provider.h
@@ -0,0 +1,46 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef __LINUX_PCS_PROVIDER_H
+#define __LINUX_PCS_PROVIDER_H
+
+#include <linux/phy.h>
+
+/**
+ * of_pcs_simple_get - Simple xlate function to retrieve PCS
+ * @pcsspec: Phandle arguments
+ * @data: Context data (assumed assigned to the single PCS)
+ * @interface: requested PHY interface type for PCS
+ *
+ * Returns the PCS (pointed by data) or an -EOPNOTSUPP pointer
+ * if the PCS doesn't support the requested interface.
+ */
+struct phylink_pcs *of_pcs_simple_get(struct of_phandle_args *pcsspec, void *data,
+				      phy_interface_t interface);
+
+/**
+ * of_pcs_add_provider - Registers a new PCS provider
+ * @np: Device node
+ * @get: xlate function to retrieve the PCS
+ * @data: Context data
+ *
+ * Register and add a new PCS to the global providers list
+ * for the device node. A function to get the PCS from
+ * device node with the use of phandle args.
+ * To the get function is also passed the interface type
+ * requested for the PHY. PCS driver will use the passed
+ * interface to understand if the PCS can support it or not.
+ *
+ * Returns 0 on success or -ENOMEM on allocation failure.
+ */
+int of_pcs_add_provider(struct device_node *np,
+			struct phylink_pcs *(*get)(struct of_phandle_args *pcsspec,
+						   void *data,
+						   phy_interface_t interface),
+			void *data);
+
+/**
+ * of_pcs_del_provider - Removes a PCS provider
+ * @np: Device node
+ */
+void of_pcs_del_provider(struct device_node *np);
+
+#endif /* __LINUX_PCS_PROVIDER_H */
diff --git a/include/linux/pcs/pcs.h b/include/linux/pcs/pcs.h
new file mode 100644
index 000000000000..b681bf05ac08
--- /dev/null
+++ b/include/linux/pcs/pcs.h
@@ -0,0 +1,62 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+#ifndef __LINUX_PCS_H
+#define __LINUX_PCS_H
+
+#include <linux/phy.h>
+#include <linux/phylink.h>
+
+static inline bool pcs_supports_interface(struct phylink_pcs *pcs,
+					  phy_interface_t interface)
+{
+	return test_bit(interface, pcs->supported_interfaces);
+}
+
+#ifdef CONFIG_OF_PCS
+/**
+ * of_pcs_get - Retrieves a PCS from a device node
+ * @np: Device node
+ * @index: Index of PCS handle in Device Node
+ * @interface: requested PHY interface type for PCS
+ *
+ * Get a PCS for the requested PHY interface type from the
+ * device node at index.
+ *
+ * Returns a pointer to the phylink_pcs or a negative
+ * error pointer. Can return -EPROBE_DEFER if the PCS is not
+ * present in global providers list (either due to driver
+ * still needs to be probed or it failed to probe/removed)
+ */
+struct phylink_pcs *of_pcs_get(struct device_node *np, int index,
+			       phy_interface_t interface);
+
+/**
+ * of_phylink_mac_select_pcs - Generic MAC select pcs for OF PCS provider
+ * @config: phylink config pointer
+ * @interface: requested PHY interface type for PCS
+ *
+ * Generic helper function to get a PCS from a "pcs-handle" OF property
+ * defined in device tree. Each phandle defined in "pcs-handle" will be
+ * tested until a PCS that supports the requested PHY interface is found.
+ *
+ * Returns a pointer to the selected PCS or an error pointer.
+ * Return NULL for PHY_INTERFACE_MODE_NA and a -EINVAL error pointer
+ * for PHY_INTERFACE_MODE_INTERNAL. It can also return -EPROBE_DEFER,
+ * refer to of_pcs_get for details about it.
+ */
+struct phylink_pcs *of_phylink_mac_select_pcs(struct phylink_config *config,
+					      phy_interface_t interface);
+#else
+static inline struct phylink_pcs *of_pcs_get(struct device_node *np, int index,
+					     phy_interface_t interface)
+{
+	return PTR_ERR(-ENOENT);
+}
+
+static inline struct phylink_pcs *of_phylink_mac_select_pcs(struct phylink_config *config,
+							    phy_interface_t interface)
+{
+	return PTR_ERR(-EOPNOTSUPP);
+}
+#endif
+
+#endif /* __LINUX_PCS_H */
diff --git a/include/linux/phylink.h b/include/linux/phylink.h
index c187267a15b6..80367d4fbad9 100644
--- a/include/linux/phylink.h
+++ b/include/linux/phylink.h
@@ -695,6 +695,8 @@ void phylink_pcs_change(struct phylink_pcs *, bool up);
 
 int phylink_pcs_pre_init(struct phylink *pl, struct phylink_pcs *pcs);
 
+void phylink_pcs_release(struct phylink_pcs *pcs);
+
 void phylink_start(struct phylink *);
 void phylink_stop(struct phylink *);
 
-- 
2.48.1
Re: [net-next PATCH 2/6] net: pcs: Implement OF support for PCS driver
Posted by Sean Anderson 8 months, 2 weeks ago
On 3/18/25 19:58, Christian Marangi wrote:
> Implement the foundation of OF support for PCS driver.
> 
> To support this, implement a simple Provider API where a PCS driver can
> expose multiple PCS with an xlate .get function.
> 
> PCS driver will have to call of_pcs_add_provider() and pass the device
> node pointer and a xlate function to return the correct PCS for the
> requested interface and the passed #pcs-cells.
> 
> This will register the PCS in a global list of providers so that
> consumer can access it.
> 
> Consumer will then use of_pcs_get() to get the actual PCS by passing the
> device_node pointer, the index for #pcs-cells and the requested
> interface.
> 
> For simple implementation where #pcs-cells is 0 and the PCS driver
> expose a single PCS, the xlate function of_pcs_simple_get() is
> provided. In such case the passed interface is ignored and is expected
> that the PCS supports any interface mode supported by the MAC.
> 
> For advanced implementation a custom xlate function is required. Such
> function should return an error if the PCS is not supported for the
> requested interface type.
> 
> This is needed for the correct function of of_phylink_mac_select_pcs()
> later described.
> 
> PCS driver on removal should first call phylink_pcs_release() on every
> PCS the driver provides and then correctly delete as a provider with
> the usage of of_pcs_del_provider().
> 
> A generic function for .mac_select_pcs is provided for any MAC driver
> that will declare PCS in DT, of_phylink_mac_select_pcs().
> This function will parse "pcs-handle" property and will try every PCS
> declared in DT until one that supports the requested interface type is
> found. This works by leveraging the return value of the xlate function
> returned by of_pcs_get() and checking if it's an ERROR or NULL, in such
> case the next PCS in the phandle array is tested.
> 
> Some additional helper are provided for xlate functions,
> pcs_supports_interface() as a simple function to check if the requested
> interface is supported by the PCS and phylink_pcs_release() to release a
> PCS from a phylink instance.
> 
> Co-developed-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  drivers/net/pcs/Kconfig          |   7 ++
>  drivers/net/pcs/Makefile         |   1 +
>  drivers/net/pcs/pcs.c            | 185 +++++++++++++++++++++++++++++++
>  drivers/net/phy/phylink.c        |  21 ++++
>  include/linux/pcs/pcs-provider.h |  46 ++++++++
>  include/linux/pcs/pcs.h          |  62 +++++++++++
>  include/linux/phylink.h          |   2 +
>  7 files changed, 324 insertions(+)
>  create mode 100644 drivers/net/pcs/pcs.c
>  create mode 100644 include/linux/pcs/pcs-provider.h
>  create mode 100644 include/linux/pcs/pcs.h
> 
> diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
> index f6aa437473de..8c3b720de6fd 100644
> --- a/drivers/net/pcs/Kconfig
> +++ b/drivers/net/pcs/Kconfig
> @@ -5,6 +5,13 @@
>  
>  menu "PCS device drivers"
>  
> +config OF_PCS
> +	tristate
> +	depends on OF
> +	depends on PHYLINK
> +	help
> +		OpenFirmware PCS accessors

More than this, please.

> +
>  config PCS_XPCS
>  	tristate "Synopsys DesignWare Ethernet XPCS"
>  	select PHYLINK
> diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
> index 4f7920618b90..29881f0f981f 100644
> --- a/drivers/net/pcs/Makefile
> +++ b/drivers/net/pcs/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # Makefile for Linux PCS drivers
>  
> +obj-$(CONFIG_OF_PCS)		+= pcs.o
>  pcs_xpcs-$(CONFIG_PCS_XPCS)	:= pcs-xpcs.o pcs-xpcs-plat.o \
>  				   pcs-xpcs-nxp.o pcs-xpcs-wx.o
>  
> diff --git a/drivers/net/pcs/pcs.c b/drivers/net/pcs/pcs.c
> new file mode 100644
> index 000000000000..af04a76ef825
> --- /dev/null
> +++ b/drivers/net/pcs/pcs.c
> @@ -0,0 +1,185 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/phylink.h>
> +#include <linux/pcs/pcs.h>
> +#include <linux/pcs/pcs-provider.h>
> +
> +struct of_pcs_provider {
> +	struct list_head link;
> +
> +	struct device_node *node;
> +	struct phylink_pcs *(*get)(struct of_phandle_args *pcsspec,
> +				   void *data,
> +				   phy_interface_t interface);

I think it is better to register each PCS explicitly, as no driver needs
cells (yet). It simplifies the lookup and registration code.

> +
> +	void *data;
> +};
> +
> +static LIST_HEAD(of_pcs_providers);
> +static DEFINE_MUTEX(of_pcs_mutex);
> +
> +struct phylink_pcs *of_pcs_simple_get(struct of_phandle_args *pcsspec, void *data,
> +				      phy_interface_t interface)
> +{
> +	struct phylink_pcs *pcs = data;
> +
> +	if (!pcs_supports_interface(pcs, interface))
> +		return ERR_PTR(-EOPNOTSUPP);
> +
> +	return data;
> +}
> +EXPORT_SYMBOL_GPL(of_pcs_simple_get);
> +
> +int of_pcs_add_provider(struct device_node *np,
> +			struct phylink_pcs *(*get)(struct of_phandle_args *pcsspec,
> +						   void *data,
> +						   phy_interface_t interface),
> +			void *data)
> +{
> +	struct of_pcs_provider *pp;
> +
> +	if (!np)
> +		return 0;
> +
> +	pp = kzalloc(sizeof(*pp), GFP_KERNEL);
> +	if (!pp)
> +		return -ENOMEM;
> +
> +	pp->node = of_node_get(np);
> +	pp->data = data;
> +	pp->get = get;
> +
> +	mutex_lock(&of_pcs_mutex);
> +	list_add(&pp->link, &of_pcs_providers);
> +	mutex_unlock(&of_pcs_mutex);
> +	pr_debug("Added pcs provider from %pOF\n", np);
> +
> +	fwnode_dev_initialized(&np->fwnode, true);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_pcs_add_provider);
> +
> +void of_pcs_del_provider(struct device_node *np)
> +{
> +	struct of_pcs_provider *pp;
> +
> +	if (!np)
> +		return;
> +
> +	mutex_lock(&of_pcs_mutex);
> +	list_for_each_entry(pp, &of_pcs_providers, link) {
> +		if (pp->node == np) {
> +			list_del(&pp->link);
> +			fwnode_dev_initialized(&np->fwnode, false);
> +			of_node_put(pp->node);
> +			kfree(pp);
> +			break;
> +		}
> +	}
> +	mutex_unlock(&of_pcs_mutex);
> +}
> +EXPORT_SYMBOL_GPL(of_pcs_del_provider);
> +
> +static int of_parse_pcsspec(const struct device_node *np, int index,
> +			    const char *name, struct of_phandle_args *out_args)
> +{
> +	int ret = -ENOENT;
> +
> +	if (!np)
> +		return -ENOENT;
> +
> +	if (name)
> +		index = of_property_match_string(np, "pcs-names", name);
> +
> +	ret = of_parse_phandle_with_args(np, "pcs-handle", "#pcs-cells",
> +					 index, out_args);
> +	if (ret || (name && index < 0))
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static struct phylink_pcs *
> +of_pcs_get_from_pcsspec(struct of_phandle_args *pcsspec,
> +			phy_interface_t interface)
> +{
> +	struct of_pcs_provider *provider;
> +	struct phylink_pcs *pcs = ERR_PTR(-EPROBE_DEFER);
> +
> +	if (!pcsspec)
> +		return ERR_PTR(-EINVAL);
> +
> +	mutex_lock(&of_pcs_mutex);
> +	list_for_each_entry(provider, &of_pcs_providers, link) {
> +		if (provider->node == pcsspec->np) {
> +			pcs = provider->get(pcsspec, provider->data,
> +					    interface);
> +			if (!IS_ERR(pcs))
> +				break;
> +		}
> +	}
> +	mutex_unlock(&of_pcs_mutex);
> +
> +	return pcs;
> +}
> +
> +static struct phylink_pcs *__of_pcs_get(struct device_node *np, int index,
> +					const char *con_id,
> +					phy_interface_t interface)
> +{
> +	struct of_phandle_args pcsspec;
> +	struct phylink_pcs *pcs;
> +	int ret;
> +
> +	ret = of_parse_pcsspec(np, index, con_id, &pcsspec);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	pcs = of_pcs_get_from_pcsspec(&pcsspec, interface);
> +	of_node_put(pcsspec.np);
> +
> +	return pcs;
> +}
> +
> +struct phylink_pcs *of_pcs_get(struct device_node *np, int index,
> +			       phy_interface_t interface)
> +{
> +	return __of_pcs_get(np, index, NULL, interface);
> +}
> +EXPORT_SYMBOL_GPL(of_pcs_get);
> +
> +struct phylink_pcs *of_phylink_mac_select_pcs(struct phylink_config *config,
> +					      phy_interface_t interface)
> +{
> +	int i, count;
> +	struct device *dev = config->dev;
> +	struct device_node *np = dev->of_node;
> +	struct phylink_pcs *pcs = ERR_PTR(-ENODEV);
> +
> +	/* To enable using_mac_select_pcs on phylink_create */
> +	if (interface == PHY_INTERFACE_MODE_NA)
> +		return NULL;
> +
> +	/* Reject configuring PCS with Internal mode */
> +	if (interface == PHY_INTERFACE_MODE_INTERNAL)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (!of_property_present(np, "pcs-handle"))
> +		return pcs;
> +
> +	count = of_count_phandle_with_args(np, "pcs-handle", "#pcs-cells");

You need to have a way for the MAC to specify a different phandle for
backwards-compatibility. There are several MACs that have different
names in existing devicetrees.

> +	if (count < 0)
> +		return ERR_PTR(count);
> +
> +	for (i = 0; i < count; i++) {
> +		pcs = of_pcs_get(np, i, interface);
> +		if (!IS_ERR_OR_NULL(pcs))

As commented by others, this is really a bit late to get the PCS
and it complicates the error-handling. It is easier for drivers to get
the pcs in probe() (or some other setup), and  

> +			return pcs;
> +	}
> +
> +	return pcs;
> +}
> +EXPORT_SYMBOL_GPL(of_phylink_mac_select_pcs);
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index eef1712ec22c..7f71547e89fe 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -1130,6 +1130,27 @@ int phylink_pcs_pre_init(struct phylink *pl, struct phylink_pcs *pcs)
>  }
>  EXPORT_SYMBOL_GPL(phylink_pcs_pre_init);
>  
> +/**
> + * phylink_pcs_release() - release a PCS
> + * @pl: a pointer to &struct phylink_pcs
> + *
> + * PCS provider can use this to release a PCS from a phylink
> + * instance by stopping the attached netdev. This is only done
> + * if the PCS is actually attached to a phylink, otherwise is
> + * ignored.
> + */
> +void phylink_pcs_release(struct phylink_pcs *pcs)
> +{
> +	struct phylink *pl = pcs->phylink;
> +
> +	if (pl) {
> +		rtnl_lock();
> +		dev_close(pl->netdev);
> +		rtnl_unlock();

What about pl->type == PHYLINK_DEV?

And if you race with mac_select_pcs?

This approach is untenable IMO.

--Sean

> +	}
> +}
> +EXPORT_SYMBOL_GPL(phylink_pcs_release);
> +
>  static void phylink_mac_config(struct phylink *pl,
>  			       const struct phylink_link_state *state)
>  {
> diff --git a/include/linux/pcs/pcs-provider.h b/include/linux/pcs/pcs-provider.h
> new file mode 100644
> index 000000000000..0172d0286f07
> --- /dev/null
> +++ b/include/linux/pcs/pcs-provider.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef __LINUX_PCS_PROVIDER_H
> +#define __LINUX_PCS_PROVIDER_H
> +
> +#include <linux/phy.h>
> +
> +/**
> + * of_pcs_simple_get - Simple xlate function to retrieve PCS
> + * @pcsspec: Phandle arguments
> + * @data: Context data (assumed assigned to the single PCS)
> + * @interface: requested PHY interface type for PCS
> + *
> + * Returns the PCS (pointed by data) or an -EOPNOTSUPP pointer
> + * if the PCS doesn't support the requested interface.
> + */
> +struct phylink_pcs *of_pcs_simple_get(struct of_phandle_args *pcsspec, void *data,
> +				      phy_interface_t interface);
> +
> +/**
> + * of_pcs_add_provider - Registers a new PCS provider
> + * @np: Device node
> + * @get: xlate function to retrieve the PCS
> + * @data: Context data
> + *
> + * Register and add a new PCS to the global providers list
> + * for the device node. A function to get the PCS from
> + * device node with the use of phandle args.
> + * To the get function is also passed the interface type
> + * requested for the PHY. PCS driver will use the passed
> + * interface to understand if the PCS can support it or not.
> + *
> + * Returns 0 on success or -ENOMEM on allocation failure.
> + */
> +int of_pcs_add_provider(struct device_node *np,
> +			struct phylink_pcs *(*get)(struct of_phandle_args *pcsspec,
> +						   void *data,
> +						   phy_interface_t interface),
> +			void *data);
> +
> +/**
> + * of_pcs_del_provider - Removes a PCS provider
> + * @np: Device node
> + */
> +void of_pcs_del_provider(struct device_node *np);
> +
> +#endif /* __LINUX_PCS_PROVIDER_H */
> diff --git a/include/linux/pcs/pcs.h b/include/linux/pcs/pcs.h
> new file mode 100644
> index 000000000000..b681bf05ac08
> --- /dev/null
> +++ b/include/linux/pcs/pcs.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef __LINUX_PCS_H
> +#define __LINUX_PCS_H
> +
> +#include <linux/phy.h>
> +#include <linux/phylink.h>
> +
> +static inline bool pcs_supports_interface(struct phylink_pcs *pcs,
> +					  phy_interface_t interface)
> +{
> +	return test_bit(interface, pcs->supported_interfaces);
> +}
> +
> +#ifdef CONFIG_OF_PCS
> +/**
> + * of_pcs_get - Retrieves a PCS from a device node
> + * @np: Device node
> + * @index: Index of PCS handle in Device Node
> + * @interface: requested PHY interface type for PCS
> + *
> + * Get a PCS for the requested PHY interface type from the
> + * device node at index.
> + *
> + * Returns a pointer to the phylink_pcs or a negative
> + * error pointer. Can return -EPROBE_DEFER if the PCS is not
> + * present in global providers list (either due to driver
> + * still needs to be probed or it failed to probe/removed)
> + */
> +struct phylink_pcs *of_pcs_get(struct device_node *np, int index,
> +			       phy_interface_t interface);
> +
> +/**
> + * of_phylink_mac_select_pcs - Generic MAC select pcs for OF PCS provider
> + * @config: phylink config pointer
> + * @interface: requested PHY interface type for PCS
> + *
> + * Generic helper function to get a PCS from a "pcs-handle" OF property
> + * defined in device tree. Each phandle defined in "pcs-handle" will be
> + * tested until a PCS that supports the requested PHY interface is found.
> + *
> + * Returns a pointer to the selected PCS or an error pointer.
> + * Return NULL for PHY_INTERFACE_MODE_NA and a -EINVAL error pointer
> + * for PHY_INTERFACE_MODE_INTERNAL. It can also return -EPROBE_DEFER,
> + * refer to of_pcs_get for details about it.
> + */
> +struct phylink_pcs *of_phylink_mac_select_pcs(struct phylink_config *config,
> +					      phy_interface_t interface);
> +#else
> +static inline struct phylink_pcs *of_pcs_get(struct device_node *np, int index,
> +					     phy_interface_t interface)
> +{
> +	return PTR_ERR(-ENOENT);
> +}
> +
> +static inline struct phylink_pcs *of_phylink_mac_select_pcs(struct phylink_config *config,
> +							    phy_interface_t interface)
> +{
> +	return PTR_ERR(-EOPNOTSUPP);
> +}
> +#endif
> +
> +#endif /* __LINUX_PCS_H */
> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> index c187267a15b6..80367d4fbad9 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -695,6 +695,8 @@ void phylink_pcs_change(struct phylink_pcs *, bool up);
>  
>  int phylink_pcs_pre_init(struct phylink *pl, struct phylink_pcs *pcs);
>  
> +void phylink_pcs_release(struct phylink_pcs *pcs);
> +
>  void phylink_start(struct phylink *);
>  void phylink_stop(struct phylink *);
>
Re: [net-next PATCH 2/6] net: pcs: Implement OF support for PCS driver
Posted by kernel test robot 9 months ago
Hi Christian,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Christian-Marangi/net-phylink-reset-PCS-Phylink-double-reference-on-phylink_stop/20250319-080303
base:   net-next/main
patch link:    https://lore.kernel.org/r/20250318235850.6411-3-ansuelsmth%40gmail.com
patch subject: [net-next PATCH 2/6] net: pcs: Implement OF support for PCS driver
config: i386-buildonly-randconfig-002-20250319 (https://download.01.org/0day-ci/archive/20250320/202503200045.AFf6buBJ-lkp@intel.com/config)
compiler: gcc-12 (Debian 12.2.0-14) 12.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250320/202503200045.AFf6buBJ-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202503200045.AFf6buBJ-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/phy/phylink.c:989: warning: Function parameter or struct member 'pcs' not described in 'phylink_pcs_release'
>> drivers/net/phy/phylink.c:989: warning: Excess function parameter 'pl' description in 'phylink_pcs_release'


vim +989 drivers/net/phy/phylink.c

   978	
   979	/**
   980	 * phylink_pcs_release() - release a PCS
   981	 * @pl: a pointer to &struct phylink_pcs
   982	 *
   983	 * PCS provider can use this to release a PCS from a phylink
   984	 * instance by stopping the attached netdev. This is only done
   985	 * if the PCS is actually attached to a phylink, otherwise is
   986	 * ignored.
   987	 */
   988	void phylink_pcs_release(struct phylink_pcs *pcs)
 > 989	{
   990		struct phylink *pl = pcs->phylink;
   991	
   992		if (pl) {
   993			rtnl_lock();
   994			dev_close(pl->netdev);
   995			rtnl_unlock();
   996		}
   997	}
   998	EXPORT_SYMBOL_GPL(phylink_pcs_release);
   999	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Re: [net-next PATCH 2/6] net: pcs: Implement OF support for PCS driver
Posted by Russell King (Oracle) 9 months ago
On Wed, Mar 19, 2025 at 12:58:38AM +0100, Christian Marangi wrote:
> Implement the foundation of OF support for PCS driver.
> 
> To support this, implement a simple Provider API where a PCS driver can
> expose multiple PCS with an xlate .get function.
> 
> PCS driver will have to call of_pcs_add_provider() and pass the device
> node pointer and a xlate function to return the correct PCS for the
> requested interface and the passed #pcs-cells.
> 
> This will register the PCS in a global list of providers so that
> consumer can access it.
> 
> Consumer will then use of_pcs_get() to get the actual PCS by passing the
> device_node pointer, the index for #pcs-cells and the requested
> interface.
> 
> For simple implementation where #pcs-cells is 0 and the PCS driver
> expose a single PCS, the xlate function of_pcs_simple_get() is
> provided. In such case the passed interface is ignored and is expected
> that the PCS supports any interface mode supported by the MAC.
> 
> For advanced implementation a custom xlate function is required. Such
> function should return an error if the PCS is not supported for the
> requested interface type.
> 
> This is needed for the correct function of of_phylink_mac_select_pcs()
> later described.
> 
> PCS driver on removal should first call phylink_pcs_release() on every
> PCS the driver provides and then correctly delete as a provider with
> the usage of of_pcs_del_provider().
> 
> A generic function for .mac_select_pcs is provided for any MAC driver
> that will declare PCS in DT, of_phylink_mac_select_pcs().
> This function will parse "pcs-handle" property and will try every PCS
> declared in DT until one that supports the requested interface type is
> found. This works by leveraging the return value of the xlate function
> returned by of_pcs_get() and checking if it's an ERROR or NULL, in such
> case the next PCS in the phandle array is tested.
> 
> Some additional helper are provided for xlate functions,
> pcs_supports_interface() as a simple function to check if the requested
> interface is supported by the PCS and phylink_pcs_release() to release a
> PCS from a phylink instance.
> 
> Co-developed-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>

As a general comment, should we be developing stuff that is DT-centric
or fwnode-centric. We already have users of phylink using swnodes, and
it seems bad to design something today that is centred around just one
method of describing something.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [net-next PATCH 2/6] net: pcs: Implement OF support for PCS driver
Posted by Christian Marangi 9 months ago
On Wed, Mar 19, 2025 at 03:17:27PM +0000, Russell King (Oracle) wrote:
> On Wed, Mar 19, 2025 at 12:58:38AM +0100, Christian Marangi wrote:
> > Implement the foundation of OF support for PCS driver.
> > 
> > To support this, implement a simple Provider API where a PCS driver can
> > expose multiple PCS with an xlate .get function.
> > 
> > PCS driver will have to call of_pcs_add_provider() and pass the device
> > node pointer and a xlate function to return the correct PCS for the
> > requested interface and the passed #pcs-cells.
> > 
> > This will register the PCS in a global list of providers so that
> > consumer can access it.
> > 
> > Consumer will then use of_pcs_get() to get the actual PCS by passing the
> > device_node pointer, the index for #pcs-cells and the requested
> > interface.
> > 
> > For simple implementation where #pcs-cells is 0 and the PCS driver
> > expose a single PCS, the xlate function of_pcs_simple_get() is
> > provided. In such case the passed interface is ignored and is expected
> > that the PCS supports any interface mode supported by the MAC.
> > 
> > For advanced implementation a custom xlate function is required. Such
> > function should return an error if the PCS is not supported for the
> > requested interface type.
> > 
> > This is needed for the correct function of of_phylink_mac_select_pcs()
> > later described.
> > 
> > PCS driver on removal should first call phylink_pcs_release() on every
> > PCS the driver provides and then correctly delete as a provider with
> > the usage of of_pcs_del_provider().
> > 
> > A generic function for .mac_select_pcs is provided for any MAC driver
> > that will declare PCS in DT, of_phylink_mac_select_pcs().
> > This function will parse "pcs-handle" property and will try every PCS
> > declared in DT until one that supports the requested interface type is
> > found. This works by leveraging the return value of the xlate function
> > returned by of_pcs_get() and checking if it's an ERROR or NULL, in such
> > case the next PCS in the phandle array is tested.
> > 
> > Some additional helper are provided for xlate functions,
> > pcs_supports_interface() as a simple function to check if the requested
> > interface is supported by the PCS and phylink_pcs_release() to release a
> > PCS from a phylink instance.
> > 
> > Co-developed-by: Daniel Golle <daniel@makrotopia.org>
> > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> 
> As a general comment, should we be developing stuff that is DT-centric
> or fwnode-centric. We already have users of phylink using swnodes, and
> it seems bad to design something today that is centred around just one
> method of describing something.
>

Honestly, at least for me testing scenario different than DT is quite
difficult, we can make changes to also support swnodes but we won't have
anything to test them. Given the bad situation PCS is currently I feel
we should first focus on DT or at least model something workable first
than put even more complex stuff on the table.

-- 
	Ansuel
Re: [net-next PATCH 2/6] net: pcs: Implement OF support for PCS driver
Posted by Russell King (Oracle) 9 months ago
On Wed, Mar 19, 2025 at 05:03:15PM +0100, Christian Marangi wrote:
> On Wed, Mar 19, 2025 at 03:17:27PM +0000, Russell King (Oracle) wrote:
> > On Wed, Mar 19, 2025 at 12:58:38AM +0100, Christian Marangi wrote:
> > > Implement the foundation of OF support for PCS driver.
> > > 
> > > To support this, implement a simple Provider API where a PCS driver can
> > > expose multiple PCS with an xlate .get function.
> > > 
> > > PCS driver will have to call of_pcs_add_provider() and pass the device
> > > node pointer and a xlate function to return the correct PCS for the
> > > requested interface and the passed #pcs-cells.
> > > 
> > > This will register the PCS in a global list of providers so that
> > > consumer can access it.
> > > 
> > > Consumer will then use of_pcs_get() to get the actual PCS by passing the
> > > device_node pointer, the index for #pcs-cells and the requested
> > > interface.
> > > 
> > > For simple implementation where #pcs-cells is 0 and the PCS driver
> > > expose a single PCS, the xlate function of_pcs_simple_get() is
> > > provided. In such case the passed interface is ignored and is expected
> > > that the PCS supports any interface mode supported by the MAC.
> > > 
> > > For advanced implementation a custom xlate function is required. Such
> > > function should return an error if the PCS is not supported for the
> > > requested interface type.
> > > 
> > > This is needed for the correct function of of_phylink_mac_select_pcs()
> > > later described.
> > > 
> > > PCS driver on removal should first call phylink_pcs_release() on every
> > > PCS the driver provides and then correctly delete as a provider with
> > > the usage of of_pcs_del_provider().
> > > 
> > > A generic function for .mac_select_pcs is provided for any MAC driver
> > > that will declare PCS in DT, of_phylink_mac_select_pcs().
> > > This function will parse "pcs-handle" property and will try every PCS
> > > declared in DT until one that supports the requested interface type is
> > > found. This works by leveraging the return value of the xlate function
> > > returned by of_pcs_get() and checking if it's an ERROR or NULL, in such
> > > case the next PCS in the phandle array is tested.
> > > 
> > > Some additional helper are provided for xlate functions,
> > > pcs_supports_interface() as a simple function to check if the requested
> > > interface is supported by the PCS and phylink_pcs_release() to release a
> > > PCS from a phylink instance.
> > > 
> > > Co-developed-by: Daniel Golle <daniel@makrotopia.org>
> > > Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> > > Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> > 
> > As a general comment, should we be developing stuff that is DT-centric
> > or fwnode-centric. We already have users of phylink using swnodes, and
> > it seems bad to design something today that is centred around just one
> > method of describing something.
> >
> 
> Honestly, at least for me testing scenario different than DT is quite
> difficult, we can make changes to also support swnodes but we won't have
> anything to test them. Given the bad situation PCS is currently I feel
> we should first focus on DT or at least model something workable first
> than put even more complex stuff on the table.

The problem I have is that once we invent DT specific interfaces, we
seem to endlessly persist with them even when we have corresponding
fwnode interfaces, even when it's easy to convert.

So, my opinion today is that we should avoid single-firmware specific
interfaces.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [net-next PATCH 2/6] net: pcs: Implement OF support for PCS driver
Posted by Christian Marangi 9 months ago
On Wed, Mar 19, 2025 at 12:58:38AM +0100, Christian Marangi wrote:
> Implement the foundation of OF support for PCS driver.
> 
> To support this, implement a simple Provider API where a PCS driver can
> expose multiple PCS with an xlate .get function.
> 
> PCS driver will have to call of_pcs_add_provider() and pass the device
> node pointer and a xlate function to return the correct PCS for the
> requested interface and the passed #pcs-cells.
> 
> This will register the PCS in a global list of providers so that
> consumer can access it.
> 
> Consumer will then use of_pcs_get() to get the actual PCS by passing the
> device_node pointer, the index for #pcs-cells and the requested
> interface.
> 
> For simple implementation where #pcs-cells is 0 and the PCS driver
> expose a single PCS, the xlate function of_pcs_simple_get() is
> provided. In such case the passed interface is ignored and is expected
> that the PCS supports any interface mode supported by the MAC.
> 
> For advanced implementation a custom xlate function is required. Such
> function should return an error if the PCS is not supported for the
> requested interface type.
> 
> This is needed for the correct function of of_phylink_mac_select_pcs()
> later described.
> 
> PCS driver on removal should first call phylink_pcs_release() on every
> PCS the driver provides and then correctly delete as a provider with
> the usage of of_pcs_del_provider().
> 
> A generic function for .mac_select_pcs is provided for any MAC driver
> that will declare PCS in DT, of_phylink_mac_select_pcs().
> This function will parse "pcs-handle" property and will try every PCS
> declared in DT until one that supports the requested interface type is
> found. This works by leveraging the return value of the xlate function
> returned by of_pcs_get() and checking if it's an ERROR or NULL, in such
> case the next PCS in the phandle array is tested.
> 
> Some additional helper are provided for xlate functions,
> pcs_supports_interface() as a simple function to check if the requested
> interface is supported by the PCS and phylink_pcs_release() to release a
> PCS from a phylink instance.
> 
> Co-developed-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  drivers/net/pcs/Kconfig          |   7 ++
>  drivers/net/pcs/Makefile         |   1 +
>  drivers/net/pcs/pcs.c            | 185 +++++++++++++++++++++++++++++++
>  drivers/net/phy/phylink.c        |  21 ++++
>  include/linux/pcs/pcs-provider.h |  46 ++++++++
>  include/linux/pcs/pcs.h          |  62 +++++++++++
>  include/linux/phylink.h          |   2 +
>  7 files changed, 324 insertions(+)
>  create mode 100644 drivers/net/pcs/pcs.c
>  create mode 100644 include/linux/pcs/pcs-provider.h
>  create mode 100644 include/linux/pcs/pcs.h
> 
> diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
> index f6aa437473de..8c3b720de6fd 100644
> --- a/drivers/net/pcs/Kconfig
> +++ b/drivers/net/pcs/Kconfig
> @@ -5,6 +5,13 @@
>  
>  menu "PCS device drivers"
>  
> +config OF_PCS
> +	tristate
> +	depends on OF
> +	depends on PHYLINK
> +	help
> +		OpenFirmware PCS accessors
> +
>  config PCS_XPCS
>  	tristate "Synopsys DesignWare Ethernet XPCS"
>  	select PHYLINK
> diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
> index 4f7920618b90..29881f0f981f 100644
> --- a/drivers/net/pcs/Makefile
> +++ b/drivers/net/pcs/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # Makefile for Linux PCS drivers
>  
> +obj-$(CONFIG_OF_PCS)		+= pcs.o
>  pcs_xpcs-$(CONFIG_PCS_XPCS)	:= pcs-xpcs.o pcs-xpcs-plat.o \
>  				   pcs-xpcs-nxp.o pcs-xpcs-wx.o
>  
> diff --git a/drivers/net/pcs/pcs.c b/drivers/net/pcs/pcs.c
> new file mode 100644
> index 000000000000..af04a76ef825
> --- /dev/null
> +++ b/drivers/net/pcs/pcs.c
> @@ -0,0 +1,185 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/phylink.h>
> +#include <linux/pcs/pcs.h>
> +#include <linux/pcs/pcs-provider.h>
> +
> +struct of_pcs_provider {
> +	struct list_head link;
> +
> +	struct device_node *node;
> +	struct phylink_pcs *(*get)(struct of_phandle_args *pcsspec,
> +				   void *data,
> +				   phy_interface_t interface);
> +
> +	void *data;
> +};
> +
> +static LIST_HEAD(of_pcs_providers);
> +static DEFINE_MUTEX(of_pcs_mutex);
> +
> +struct phylink_pcs *of_pcs_simple_get(struct of_phandle_args *pcsspec, void *data,
> +				      phy_interface_t interface)
> +{
> +	struct phylink_pcs *pcs = data;
> +
> +	if (!pcs_supports_interface(pcs, interface))
> +		return ERR_PTR(-EOPNOTSUPP);
> +
> +	return data;
> +}
> +EXPORT_SYMBOL_GPL(of_pcs_simple_get);
> +
> +int of_pcs_add_provider(struct device_node *np,
> +			struct phylink_pcs *(*get)(struct of_phandle_args *pcsspec,
> +						   void *data,
> +						   phy_interface_t interface),
> +			void *data)
> +{
> +	struct of_pcs_provider *pp;
> +
> +	if (!np)
> +		return 0;
> +
> +	pp = kzalloc(sizeof(*pp), GFP_KERNEL);
> +	if (!pp)
> +		return -ENOMEM;
> +
> +	pp->node = of_node_get(np);
> +	pp->data = data;
> +	pp->get = get;
> +
> +	mutex_lock(&of_pcs_mutex);
> +	list_add(&pp->link, &of_pcs_providers);
> +	mutex_unlock(&of_pcs_mutex);
> +	pr_debug("Added pcs provider from %pOF\n", np);
> +
> +	fwnode_dev_initialized(&np->fwnode, true);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_pcs_add_provider);
> +
> +void of_pcs_del_provider(struct device_node *np)
> +{
> +	struct of_pcs_provider *pp;
> +
> +	if (!np)
> +		return;
> +
> +	mutex_lock(&of_pcs_mutex);
> +	list_for_each_entry(pp, &of_pcs_providers, link) {
> +		if (pp->node == np) {
> +			list_del(&pp->link);
> +			fwnode_dev_initialized(&np->fwnode, false);
> +			of_node_put(pp->node);
> +			kfree(pp);
> +			break;
> +		}
> +	}
> +	mutex_unlock(&of_pcs_mutex);
> +}
> +EXPORT_SYMBOL_GPL(of_pcs_del_provider);
> +
> +static int of_parse_pcsspec(const struct device_node *np, int index,
> +			    const char *name, struct of_phandle_args *out_args)
> +{
> +	int ret = -ENOENT;
> +
> +	if (!np)
> +		return -ENOENT;
> +
> +	if (name)
> +		index = of_property_match_string(np, "pcs-names", name);
> +
> +	ret = of_parse_phandle_with_args(np, "pcs-handle", "#pcs-cells",
> +					 index, out_args);
> +	if (ret || (name && index < 0))
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static struct phylink_pcs *
> +of_pcs_get_from_pcsspec(struct of_phandle_args *pcsspec,
> +			phy_interface_t interface)
> +{
> +	struct of_pcs_provider *provider;
> +	struct phylink_pcs *pcs = ERR_PTR(-EPROBE_DEFER);
> +
> +	if (!pcsspec)
> +		return ERR_PTR(-EINVAL);
> +
> +	mutex_lock(&of_pcs_mutex);
> +	list_for_each_entry(provider, &of_pcs_providers, link) {
> +		if (provider->node == pcsspec->np) {
> +			pcs = provider->get(pcsspec, provider->data,
> +					    interface);
> +			if (!IS_ERR(pcs))
> +				break;
> +		}
> +	}
> +	mutex_unlock(&of_pcs_mutex);
> +
> +	return pcs;
> +}
> +
> +static struct phylink_pcs *__of_pcs_get(struct device_node *np, int index,
> +					const char *con_id,
> +					phy_interface_t interface)
> +{
> +	struct of_phandle_args pcsspec;
> +	struct phylink_pcs *pcs;
> +	int ret;
> +
> +	ret = of_parse_pcsspec(np, index, con_id, &pcsspec);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	pcs = of_pcs_get_from_pcsspec(&pcsspec, interface);
> +	of_node_put(pcsspec.np);
> +
> +	return pcs;
> +}
> +
> +struct phylink_pcs *of_pcs_get(struct device_node *np, int index,
> +			       phy_interface_t interface)
> +{
> +	return __of_pcs_get(np, index, NULL, interface);
> +}
> +EXPORT_SYMBOL_GPL(of_pcs_get);
> +
> +struct phylink_pcs *of_phylink_mac_select_pcs(struct phylink_config *config,
> +					      phy_interface_t interface)
> +{
> +	int i, count;
> +	struct device *dev = config->dev;
> +	struct device_node *np = dev->of_node;
> +	struct phylink_pcs *pcs = ERR_PTR(-ENODEV);
> +
> +	/* To enable using_mac_select_pcs on phylink_create */
> +	if (interface == PHY_INTERFACE_MODE_NA)
> +		return NULL;
> +
> +	/* Reject configuring PCS with Internal mode */
> +	if (interface == PHY_INTERFACE_MODE_INTERNAL)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (!of_property_present(np, "pcs-handle"))
> +		return pcs;
> +
> +	count = of_count_phandle_with_args(np, "pcs-handle", "#pcs-cells");
> +	if (count < 0)
> +		return ERR_PTR(count);
> +
> +	for (i = 0; i < count; i++) {
> +		pcs = of_pcs_get(np, i, interface);
> +		if (!IS_ERR_OR_NULL(pcs))
> +			return pcs;
> +	}
> +
> +	return pcs;
> +}
> +EXPORT_SYMBOL_GPL(of_phylink_mac_select_pcs);
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index eef1712ec22c..7f71547e89fe 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -1130,6 +1130,27 @@ int phylink_pcs_pre_init(struct phylink *pl, struct phylink_pcs *pcs)
>  }
>  EXPORT_SYMBOL_GPL(phylink_pcs_pre_init);
>  
> +/**
> + * phylink_pcs_release() - release a PCS
> + * @pl: a pointer to &struct phylink_pcs
> + *
> + * PCS provider can use this to release a PCS from a phylink
> + * instance by stopping the attached netdev. This is only done
> + * if the PCS is actually attached to a phylink, otherwise is
> + * ignored.
> + */
> +void phylink_pcs_release(struct phylink_pcs *pcs)
> +{
> +	struct phylink *pl = pcs->phylink;
> +
> +	if (pl) {
> +		rtnl_lock();
> +		dev_close(pl->netdev);
> +		rtnl_unlock();
> +	}
> +}
> +EXPORT_SYMBOL_GPL(phylink_pcs_release);
> +
>  static void phylink_mac_config(struct phylink *pl,
>  			       const struct phylink_link_state *state)
>  {
> diff --git a/include/linux/pcs/pcs-provider.h b/include/linux/pcs/pcs-provider.h
> new file mode 100644
> index 000000000000..0172d0286f07
> --- /dev/null
> +++ b/include/linux/pcs/pcs-provider.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef __LINUX_PCS_PROVIDER_H
> +#define __LINUX_PCS_PROVIDER_H
> +
> +#include <linux/phy.h>
> +
> +/**
> + * of_pcs_simple_get - Simple xlate function to retrieve PCS
> + * @pcsspec: Phandle arguments
> + * @data: Context data (assumed assigned to the single PCS)
> + * @interface: requested PHY interface type for PCS
> + *
> + * Returns the PCS (pointed by data) or an -EOPNOTSUPP pointer
> + * if the PCS doesn't support the requested interface.
> + */
> +struct phylink_pcs *of_pcs_simple_get(struct of_phandle_args *pcsspec, void *data,
> +				      phy_interface_t interface);
> +
> +/**
> + * of_pcs_add_provider - Registers a new PCS provider
> + * @np: Device node
> + * @get: xlate function to retrieve the PCS
> + * @data: Context data
> + *
> + * Register and add a new PCS to the global providers list
> + * for the device node. A function to get the PCS from
> + * device node with the use of phandle args.
> + * To the get function is also passed the interface type
> + * requested for the PHY. PCS driver will use the passed
> + * interface to understand if the PCS can support it or not.
> + *
> + * Returns 0 on success or -ENOMEM on allocation failure.
> + */
> +int of_pcs_add_provider(struct device_node *np,
> +			struct phylink_pcs *(*get)(struct of_phandle_args *pcsspec,
> +						   void *data,
> +						   phy_interface_t interface),
> +			void *data);
> +
> +/**
> + * of_pcs_del_provider - Removes a PCS provider
> + * @np: Device node
> + */
> +void of_pcs_del_provider(struct device_node *np);
> +
> +#endif /* __LINUX_PCS_PROVIDER_H */
> diff --git a/include/linux/pcs/pcs.h b/include/linux/pcs/pcs.h
> new file mode 100644
> index 000000000000..b681bf05ac08
> --- /dev/null
> +++ b/include/linux/pcs/pcs.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef __LINUX_PCS_H
> +#define __LINUX_PCS_H
> +
> +#include <linux/phy.h>
> +#include <linux/phylink.h>
> +
> +static inline bool pcs_supports_interface(struct phylink_pcs *pcs,
> +					  phy_interface_t interface)
> +{
> +	return test_bit(interface, pcs->supported_interfaces);
> +}
> +
> +#ifdef CONFIG_OF_PCS

Will be changed to IS_ENABLED to account for module config.

> +/**
> + * of_pcs_get - Retrieves a PCS from a device node
> + * @np: Device node
> + * @index: Index of PCS handle in Device Node
> + * @interface: requested PHY interface type for PCS
> + *
> + * Get a PCS for the requested PHY interface type from the
> + * device node at index.
> + *
> + * Returns a pointer to the phylink_pcs or a negative
> + * error pointer. Can return -EPROBE_DEFER if the PCS is not
> + * present in global providers list (either due to driver
> + * still needs to be probed or it failed to probe/removed)
> + */
> +struct phylink_pcs *of_pcs_get(struct device_node *np, int index,
> +			       phy_interface_t interface);
> +
> +/**
> + * of_phylink_mac_select_pcs - Generic MAC select pcs for OF PCS provider
> + * @config: phylink config pointer
> + * @interface: requested PHY interface type for PCS
> + *
> + * Generic helper function to get a PCS from a "pcs-handle" OF property
> + * defined in device tree. Each phandle defined in "pcs-handle" will be
> + * tested until a PCS that supports the requested PHY interface is found.
> + *
> + * Returns a pointer to the selected PCS or an error pointer.
> + * Return NULL for PHY_INTERFACE_MODE_NA and a -EINVAL error pointer
> + * for PHY_INTERFACE_MODE_INTERNAL. It can also return -EPROBE_DEFER,
> + * refer to of_pcs_get for details about it.
> + */
> +struct phylink_pcs *of_phylink_mac_select_pcs(struct phylink_config *config,
> +					      phy_interface_t interface);
> +#else
> +static inline struct phylink_pcs *of_pcs_get(struct device_node *np, int index,
> +					     phy_interface_t interface)
> +{
> +	return PTR_ERR(-ENOENT);

Wrong macro for error, will be fixed in v2.

> +}
> +
> +static inline struct phylink_pcs *of_phylink_mac_select_pcs(struct phylink_config *config,
> +							    phy_interface_t interface)
> +{
> +	return PTR_ERR(-EOPNOTSUPP);

Ditto.

> +}
> +#endif
> +
> +#endif /* __LINUX_PCS_H */
> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> index c187267a15b6..80367d4fbad9 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -695,6 +695,8 @@ void phylink_pcs_change(struct phylink_pcs *, bool up);
>  
>  int phylink_pcs_pre_init(struct phylink *pl, struct phylink_pcs *pcs);
>  
> +void phylink_pcs_release(struct phylink_pcs *pcs);
> +
>  void phylink_start(struct phylink *);
>  void phylink_stop(struct phylink *);
>  
> -- 
> 2.48.1
> 

-- 
	Ansuel
Re: [net-next PATCH 2/6] net: pcs: Implement OF support for PCS driver
Posted by Christian Marangi 9 months ago
On Wed, Mar 19, 2025 at 12:58:38AM +0100, Christian Marangi wrote:
> Implement the foundation of OF support for PCS driver.
> 
> To support this, implement a simple Provider API where a PCS driver can
> expose multiple PCS with an xlate .get function.
> 
> PCS driver will have to call of_pcs_add_provider() and pass the device
> node pointer and a xlate function to return the correct PCS for the
> requested interface and the passed #pcs-cells.
> 
> This will register the PCS in a global list of providers so that
> consumer can access it.
> 
> Consumer will then use of_pcs_get() to get the actual PCS by passing the
> device_node pointer, the index for #pcs-cells and the requested
> interface.
> 
> For simple implementation where #pcs-cells is 0 and the PCS driver
> expose a single PCS, the xlate function of_pcs_simple_get() is
> provided. In such case the passed interface is ignored and is expected
> that the PCS supports any interface mode supported by the MAC.
> 
> For advanced implementation a custom xlate function is required. Such
> function should return an error if the PCS is not supported for the
> requested interface type.
> 
> This is needed for the correct function of of_phylink_mac_select_pcs()
> later described.
> 
> PCS driver on removal should first call phylink_pcs_release() on every
> PCS the driver provides and then correctly delete as a provider with
> the usage of of_pcs_del_provider().
> 
> A generic function for .mac_select_pcs is provided for any MAC driver
> that will declare PCS in DT, of_phylink_mac_select_pcs().
> This function will parse "pcs-handle" property and will try every PCS
> declared in DT until one that supports the requested interface type is
> found. This works by leveraging the return value of the xlate function
> returned by of_pcs_get() and checking if it's an ERROR or NULL, in such
> case the next PCS in the phandle array is tested.
> 
> Some additional helper are provided for xlate functions,
> pcs_supports_interface() as a simple function to check if the requested
> interface is supported by the PCS and phylink_pcs_release() to release a
> PCS from a phylink instance.
> 
> Co-developed-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Daniel Golle <daniel@makrotopia.org>
> Signed-off-by: Christian Marangi <ansuelsmth@gmail.com>
> ---
>  drivers/net/pcs/Kconfig          |   7 ++
>  drivers/net/pcs/Makefile         |   1 +
>  drivers/net/pcs/pcs.c            | 185 +++++++++++++++++++++++++++++++
>  drivers/net/phy/phylink.c        |  21 ++++
>  include/linux/pcs/pcs-provider.h |  46 ++++++++
>  include/linux/pcs/pcs.h          |  62 +++++++++++
>  include/linux/phylink.h          |   2 +
>  7 files changed, 324 insertions(+)
>  create mode 100644 drivers/net/pcs/pcs.c
>  create mode 100644 include/linux/pcs/pcs-provider.h
>  create mode 100644 include/linux/pcs/pcs.h
> 
> diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
> index f6aa437473de..8c3b720de6fd 100644
> --- a/drivers/net/pcs/Kconfig
> +++ b/drivers/net/pcs/Kconfig
> @@ -5,6 +5,13 @@
>  
>  menu "PCS device drivers"
>  
> +config OF_PCS
> +	tristate
> +	depends on OF
> +	depends on PHYLINK
> +	help
> +		OpenFirmware PCS accessors
> +
>  config PCS_XPCS
>  	tristate "Synopsys DesignWare Ethernet XPCS"
>  	select PHYLINK
> diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
> index 4f7920618b90..29881f0f981f 100644
> --- a/drivers/net/pcs/Makefile
> +++ b/drivers/net/pcs/Makefile
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: GPL-2.0
>  # Makefile for Linux PCS drivers
>  
> +obj-$(CONFIG_OF_PCS)		+= pcs.o
>  pcs_xpcs-$(CONFIG_PCS_XPCS)	:= pcs-xpcs.o pcs-xpcs-plat.o \
>  				   pcs-xpcs-nxp.o pcs-xpcs-wx.o
>  
> diff --git a/drivers/net/pcs/pcs.c b/drivers/net/pcs/pcs.c
> new file mode 100644
> index 000000000000..af04a76ef825
> --- /dev/null
> +++ b/drivers/net/pcs/pcs.c
> @@ -0,0 +1,185 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/phylink.h>
> +#include <linux/pcs/pcs.h>
> +#include <linux/pcs/pcs-provider.h>
> +
> +struct of_pcs_provider {
> +	struct list_head link;
> +
> +	struct device_node *node;
> +	struct phylink_pcs *(*get)(struct of_phandle_args *pcsspec,
> +				   void *data,
> +				   phy_interface_t interface);
> +
> +	void *data;
> +};
> +
> +static LIST_HEAD(of_pcs_providers);
> +static DEFINE_MUTEX(of_pcs_mutex);
> +
> +struct phylink_pcs *of_pcs_simple_get(struct of_phandle_args *pcsspec, void *data,
> +				      phy_interface_t interface)
> +{
> +	struct phylink_pcs *pcs = data;
> +
> +	if (!pcs_supports_interface(pcs, interface))
> +		return ERR_PTR(-EOPNOTSUPP);
> +
> +	return data;
> +}
> +EXPORT_SYMBOL_GPL(of_pcs_simple_get);
> +
> +int of_pcs_add_provider(struct device_node *np,
> +			struct phylink_pcs *(*get)(struct of_phandle_args *pcsspec,
> +						   void *data,
> +						   phy_interface_t interface),
> +			void *data)
> +{
> +	struct of_pcs_provider *pp;
> +
> +	if (!np)
> +		return 0;
> +
> +	pp = kzalloc(sizeof(*pp), GFP_KERNEL);
> +	if (!pp)
> +		return -ENOMEM;
> +
> +	pp->node = of_node_get(np);
> +	pp->data = data;
> +	pp->get = get;
> +
> +	mutex_lock(&of_pcs_mutex);
> +	list_add(&pp->link, &of_pcs_providers);
> +	mutex_unlock(&of_pcs_mutex);
> +	pr_debug("Added pcs provider from %pOF\n", np);
> +
> +	fwnode_dev_initialized(&np->fwnode, true);
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(of_pcs_add_provider);
> +
> +void of_pcs_del_provider(struct device_node *np)
> +{
> +	struct of_pcs_provider *pp;
> +
> +	if (!np)
> +		return;
> +
> +	mutex_lock(&of_pcs_mutex);
> +	list_for_each_entry(pp, &of_pcs_providers, link) {
> +		if (pp->node == np) {
> +			list_del(&pp->link);
> +			fwnode_dev_initialized(&np->fwnode, false);
> +			of_node_put(pp->node);
> +			kfree(pp);
> +			break;
> +		}
> +	}
> +	mutex_unlock(&of_pcs_mutex);
> +}
> +EXPORT_SYMBOL_GPL(of_pcs_del_provider);
> +
> +static int of_parse_pcsspec(const struct device_node *np, int index,
> +			    const char *name, struct of_phandle_args *out_args)
> +{
> +	int ret = -ENOENT;
> +
> +	if (!np)
> +		return -ENOENT;
> +
> +	if (name)
> +		index = of_property_match_string(np, "pcs-names", name);
> +
> +	ret = of_parse_phandle_with_args(np, "pcs-handle", "#pcs-cells",
> +					 index, out_args);
> +	if (ret || (name && index < 0))
> +		return ret;
> +
> +	return 0;
> +}
> +
> +static struct phylink_pcs *
> +of_pcs_get_from_pcsspec(struct of_phandle_args *pcsspec,
> +			phy_interface_t interface)
> +{
> +	struct of_pcs_provider *provider;
> +	struct phylink_pcs *pcs = ERR_PTR(-EPROBE_DEFER);
> +
> +	if (!pcsspec)
> +		return ERR_PTR(-EINVAL);
> +
> +	mutex_lock(&of_pcs_mutex);
> +	list_for_each_entry(provider, &of_pcs_providers, link) {
> +		if (provider->node == pcsspec->np) {
> +			pcs = provider->get(pcsspec, provider->data,
> +					    interface);
> +			if (!IS_ERR(pcs))
> +				break;
> +		}
> +	}
> +	mutex_unlock(&of_pcs_mutex);
> +
> +	return pcs;
> +}
> +
> +static struct phylink_pcs *__of_pcs_get(struct device_node *np, int index,
> +					const char *con_id,
> +					phy_interface_t interface)
> +{
> +	struct of_phandle_args pcsspec;
> +	struct phylink_pcs *pcs;
> +	int ret;
> +
> +	ret = of_parse_pcsspec(np, index, con_id, &pcsspec);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	pcs = of_pcs_get_from_pcsspec(&pcsspec, interface);
> +	of_node_put(pcsspec.np);
> +
> +	return pcs;
> +}
> +
> +struct phylink_pcs *of_pcs_get(struct device_node *np, int index,
> +			       phy_interface_t interface)
> +{
> +	return __of_pcs_get(np, index, NULL, interface);
> +}
> +EXPORT_SYMBOL_GPL(of_pcs_get);
> +
> +struct phylink_pcs *of_phylink_mac_select_pcs(struct phylink_config *config,
> +					      phy_interface_t interface)
> +{
> +	int i, count;
> +	struct device *dev = config->dev;
> +	struct device_node *np = dev->of_node;
> +	struct phylink_pcs *pcs = ERR_PTR(-ENODEV);
> +
> +	/* To enable using_mac_select_pcs on phylink_create */
> +	if (interface == PHY_INTERFACE_MODE_NA)
> +		return NULL;
> +
> +	/* Reject configuring PCS with Internal mode */
> +	if (interface == PHY_INTERFACE_MODE_INTERNAL)
> +		return ERR_PTR(-EINVAL);
> +
> +	if (!of_property_present(np, "pcs-handle"))
> +		return pcs;
> +
> +	count = of_count_phandle_with_args(np, "pcs-handle", "#pcs-cells");
> +	if (count < 0)
> +		return ERR_PTR(count);
> +
> +	for (i = 0; i < count; i++) {
> +		pcs = of_pcs_get(np, i, interface);
> +		if (!IS_ERR_OR_NULL(pcs))
> +			return pcs;
> +	}
> +
> +	return pcs;
> +}
> +EXPORT_SYMBOL_GPL(of_phylink_mac_select_pcs);
> diff --git a/drivers/net/phy/phylink.c b/drivers/net/phy/phylink.c
> index eef1712ec22c..7f71547e89fe 100644
> --- a/drivers/net/phy/phylink.c
> +++ b/drivers/net/phy/phylink.c
> @@ -1130,6 +1130,27 @@ int phylink_pcs_pre_init(struct phylink *pl, struct phylink_pcs *pcs)
>  }
>  EXPORT_SYMBOL_GPL(phylink_pcs_pre_init);
>  
> +/**
> + * phylink_pcs_release() - release a PCS
> + * @pl: a pointer to &struct phylink_pcs

This is a typo and will be fixed in v2.

> + *
> + * PCS provider can use this to release a PCS from a phylink
> + * instance by stopping the attached netdev. This is only done
> + * if the PCS is actually attached to a phylink, otherwise is
> + * ignored.
> + */
> +void phylink_pcs_release(struct phylink_pcs *pcs)
> +{
> +	struct phylink *pl = pcs->phylink;
> +
> +	if (pl) {
> +		rtnl_lock();
> +		dev_close(pl->netdev);
> +		rtnl_unlock();
> +	}
> +}
> +EXPORT_SYMBOL_GPL(phylink_pcs_release);
> +
>  static void phylink_mac_config(struct phylink *pl,
>  			       const struct phylink_link_state *state)
>  {
> diff --git a/include/linux/pcs/pcs-provider.h b/include/linux/pcs/pcs-provider.h
> new file mode 100644
> index 000000000000..0172d0286f07
> --- /dev/null
> +++ b/include/linux/pcs/pcs-provider.h
> @@ -0,0 +1,46 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef __LINUX_PCS_PROVIDER_H
> +#define __LINUX_PCS_PROVIDER_H
> +
> +#include <linux/phy.h>
> +
> +/**
> + * of_pcs_simple_get - Simple xlate function to retrieve PCS
> + * @pcsspec: Phandle arguments
> + * @data: Context data (assumed assigned to the single PCS)
> + * @interface: requested PHY interface type for PCS
> + *
> + * Returns the PCS (pointed by data) or an -EOPNOTSUPP pointer
> + * if the PCS doesn't support the requested interface.
> + */
> +struct phylink_pcs *of_pcs_simple_get(struct of_phandle_args *pcsspec, void *data,
> +				      phy_interface_t interface);
> +
> +/**
> + * of_pcs_add_provider - Registers a new PCS provider
> + * @np: Device node
> + * @get: xlate function to retrieve the PCS
> + * @data: Context data
> + *
> + * Register and add a new PCS to the global providers list
> + * for the device node. A function to get the PCS from
> + * device node with the use of phandle args.
> + * To the get function is also passed the interface type
> + * requested for the PHY. PCS driver will use the passed
> + * interface to understand if the PCS can support it or not.
> + *
> + * Returns 0 on success or -ENOMEM on allocation failure.
> + */
> +int of_pcs_add_provider(struct device_node *np,
> +			struct phylink_pcs *(*get)(struct of_phandle_args *pcsspec,
> +						   void *data,
> +						   phy_interface_t interface),
> +			void *data);
> +
> +/**
> + * of_pcs_del_provider - Removes a PCS provider
> + * @np: Device node
> + */
> +void of_pcs_del_provider(struct device_node *np);
> +
> +#endif /* __LINUX_PCS_PROVIDER_H */
> diff --git a/include/linux/pcs/pcs.h b/include/linux/pcs/pcs.h
> new file mode 100644
> index 000000000000..b681bf05ac08
> --- /dev/null
> +++ b/include/linux/pcs/pcs.h
> @@ -0,0 +1,62 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +#ifndef __LINUX_PCS_H
> +#define __LINUX_PCS_H
> +
> +#include <linux/phy.h>
> +#include <linux/phylink.h>
> +
> +static inline bool pcs_supports_interface(struct phylink_pcs *pcs,
> +					  phy_interface_t interface)
> +{
> +	return test_bit(interface, pcs->supported_interfaces);
> +}
> +
> +#ifdef CONFIG_OF_PCS
> +/**
> + * of_pcs_get - Retrieves a PCS from a device node
> + * @np: Device node
> + * @index: Index of PCS handle in Device Node
> + * @interface: requested PHY interface type for PCS
> + *
> + * Get a PCS for the requested PHY interface type from the
> + * device node at index.
> + *
> + * Returns a pointer to the phylink_pcs or a negative
> + * error pointer. Can return -EPROBE_DEFER if the PCS is not
> + * present in global providers list (either due to driver
> + * still needs to be probed or it failed to probe/removed)
> + */
> +struct phylink_pcs *of_pcs_get(struct device_node *np, int index,
> +			       phy_interface_t interface);
> +
> +/**
> + * of_phylink_mac_select_pcs - Generic MAC select pcs for OF PCS provider
> + * @config: phylink config pointer
> + * @interface: requested PHY interface type for PCS
> + *
> + * Generic helper function to get a PCS from a "pcs-handle" OF property
> + * defined in device tree. Each phandle defined in "pcs-handle" will be
> + * tested until a PCS that supports the requested PHY interface is found.
> + *
> + * Returns a pointer to the selected PCS or an error pointer.
> + * Return NULL for PHY_INTERFACE_MODE_NA and a -EINVAL error pointer
> + * for PHY_INTERFACE_MODE_INTERNAL. It can also return -EPROBE_DEFER,
> + * refer to of_pcs_get for details about it.
> + */
> +struct phylink_pcs *of_phylink_mac_select_pcs(struct phylink_config *config,
> +					      phy_interface_t interface);
> +#else
> +static inline struct phylink_pcs *of_pcs_get(struct device_node *np, int index,
> +					     phy_interface_t interface)
> +{
> +	return PTR_ERR(-ENOENT);
> +}
> +
> +static inline struct phylink_pcs *of_phylink_mac_select_pcs(struct phylink_config *config,
> +							    phy_interface_t interface)
> +{
> +	return PTR_ERR(-EOPNOTSUPP);
> +}
> +#endif
> +
> +#endif /* __LINUX_PCS_H */
> diff --git a/include/linux/phylink.h b/include/linux/phylink.h
> index c187267a15b6..80367d4fbad9 100644
> --- a/include/linux/phylink.h
> +++ b/include/linux/phylink.h
> @@ -695,6 +695,8 @@ void phylink_pcs_change(struct phylink_pcs *, bool up);
>  
>  int phylink_pcs_pre_init(struct phylink *pl, struct phylink_pcs *pcs);
>  
> +void phylink_pcs_release(struct phylink_pcs *pcs);
> +
>  void phylink_start(struct phylink *);
>  void phylink_stop(struct phylink *);
>  
> -- 
> 2.48.1
> 

-- 
	Ansuel