[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zqd8z+/TL22OJ1iu@shell.armlinux.org.uk>
Date: Mon, 29 Jul 2024 12:28:15 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Daniel Golle <daniel@...rotopia.org>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
"David S. Miller" <davem@...emloft.net>,
Eric Dumazet <edumazet@...gle.com>,
Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
John Crispin <john@...ozen.org>, Felix Fietkau <nbd@....name>,
Lorenzo Bianconi <lorenzo@...nel.org>,
Sean Wang <sean.wang@...iatek.com>,
Mark Lee <Mark-MC.Lee@...iatek.com>,
Bc-bocun Chen <bc-bocun.chen@...iatek.com>,
Sam Shih <Sam.Shih@...iatek.com>,
Weijie Gao <Weijie.Gao@...iatek.com>,
Steven Liu <steven.liu@...iatek.com>,
Matthias Brugger <matthias.bgg@...il.com>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
linux-kernel@...r.kernel.org, netdev@...r.kernel.org
Subject: Re: [PATCH RFC net-next] net: pcs: add helper module for standalone
drivers
On Thu, Jul 25, 2024 at 01:44:49PM +0100, Daniel Golle wrote:
> Implement helper module for standalone PCS drivers which allows
> standaline PCS drivers to register and users to get instances of
> 'struct phylink_pcs' using device tree nodes.
>
> At this point only a single instance for each device tree node is
> supported, once we got devices providing more than one PCS we can
> extend it and introduce an xlate function as well as '#pcs-cells',
> similar to how this is done by the PHY framework.
>
> Signed-off-by: Daniel Golle <daniel@...rotopia.org>
> ---
> This is meant to provide the infrastructure suggested by
> Russell King in an earlier review. It just took me a long while to
> find the time to implement this.
> Users are going to be the standalone PCS drivers for 8/10 LynxI as
> well as 64/66 USXGMII PCS found on MediaTek MT7988 SoC.
> See also https://patchwork.kernel.org/comment/25636726/
>
> The full tree where this is being used can be found at
>
> https://github.com/dangowrt/linux/commits/mt7988-for-next/
>
> drivers/net/pcs/Kconfig | 4 ++
> drivers/net/pcs/Makefile | 1 +
> drivers/net/pcs/pcs-standalone.c | 95 +++++++++++++++++++++++++++++
> include/linux/pcs/pcs-standalone.h | 25 ++++++++
> 4 files changed, 129 insertions(+)
> create mode 100644 drivers/net/pcs/pcs-standalone.c
> create mode 100644 include/linux/pcs/pcs-standalone.h
>
> diff --git a/drivers/net/pcs/Kconfig b/drivers/net/pcs/Kconfig
> index f6aa437473de..2b02b9351fa4 100644
> --- a/drivers/net/pcs/Kconfig
> +++ b/drivers/net/pcs/Kconfig
> @@ -5,6 +5,10 @@
>
> menu "PCS device drivers"
>
> +config PCS_STANDALONE
> + tristate
> + select PHYLINK
> +
> config PCS_XPCS
> tristate "Synopsys DesignWare Ethernet XPCS"
> select PHYLINK
> diff --git a/drivers/net/pcs/Makefile b/drivers/net/pcs/Makefile
> index 4f7920618b90..0cb0057f2b8e 100644
> --- a/drivers/net/pcs/Makefile
> +++ b/drivers/net/pcs/Makefile
> @@ -4,6 +4,7 @@
> pcs_xpcs-$(CONFIG_PCS_XPCS) := pcs-xpcs.o pcs-xpcs-plat.o \
> pcs-xpcs-nxp.o pcs-xpcs-wx.o
>
> +obj-$(CONFIG_PCS_STANDALONE) += pcs-standalone.o
> obj-$(CONFIG_PCS_XPCS) += pcs_xpcs.o
> obj-$(CONFIG_PCS_LYNX) += pcs-lynx.o
> obj-$(CONFIG_PCS_MTK_LYNXI) += pcs-mtk-lynxi.o
> diff --git a/drivers/net/pcs/pcs-standalone.c b/drivers/net/pcs/pcs-standalone.c
> new file mode 100644
> index 000000000000..1569793328a1
> --- /dev/null
> +++ b/drivers/net/pcs/pcs-standalone.c
> @@ -0,0 +1,95 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Helpers for standalone PCS drivers
> + *
> + * Copyright (C) 2024 Daniel Golle <daniel@...rotopia.org>
> + */
> +
> +#include <linux/pcs/pcs-standalone.h>
> +#include <linux/phylink.h>
> +
> +static LIST_HEAD(pcs_list);
> +static DEFINE_MUTEX(pcs_mutex);
> +
> +struct pcs_standalone {
> + struct device *dev;
> + struct phylink_pcs *pcs;
> + struct list_head list;
> +};
> +
> +static void devm_pcs_provider_release(struct device *dev, void *res)
> +{
> + struct pcs_standalone *pcssa = (struct pcs_standalone *)res;
> +
> + mutex_lock(&pcs_mutex);
> + list_del(&pcssa->list);
> + mutex_unlock(&pcs_mutex);
This needs to do notify phylink if the PCS has gone away, but the
locking for this would be somewhat difficult (because pcs->phylink
could change if the PCS changes.) That would need to be solved
somehow.
> +}
> +
> +int devm_pcs_register(struct device *dev, struct phylink_pcs *pcs)
> +{
> + struct pcs_standalone *pcssa;
> +
> + pcssa = devres_alloc(devm_pcs_provider_release, sizeof(*pcssa),
> + GFP_KERNEL);
> + if (!pcssa)
> + return -ENOMEM;
> +
> + devres_add(dev, pcssa);
> + pcssa->pcs = pcs;
> + pcssa->dev = dev;
> +
> + mutex_lock(&pcs_mutex);
> + list_add_tail(&pcssa->list, &pcs_list);
> + mutex_unlock(&pcs_mutex);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(devm_pcs_register);
> +
> +static struct pcs_standalone *of_pcs_locate(const struct device_node *_np, u32 index)
> +{
> + struct device_node *np;
> + struct pcs_standalone *iter, *pcssa = NULL;
> +
> + if (!_np)
> + return NULL;
> +
> + np = of_parse_phandle(_np, "pcs-handle", index);
> + if (!np)
> + return NULL;
> +
> + mutex_lock(&pcs_mutex);
> + list_for_each_entry(iter, &pcs_list, list) {
> + if (iter->dev->of_node != np)
> + continue;
> +
> + pcssa = iter;
> + break;
> + }
> + mutex_unlock(&pcs_mutex);
> +
> + of_node_put(np);
> +
> + return pcssa ?: ERR_PTR(-ENODEV);
> +}
> +
> +struct phylink_pcs *devm_of_pcs_get(struct device *dev,
> + const struct device_node *np,
> + unsigned int index)
> +{
> + struct pcs_standalone *pcssa;
> +
> + pcssa = of_pcs_locate(np ?: dev->of_node, index);
> + if (IS_ERR_OR_NULL(pcssa))
> + return ERR_PTR(PTR_ERR(pcssa));
> +
> + device_link_add(dev, pcssa->dev, DL_FLAG_AUTOREMOVE_CONSUMER);
This is really not a nice solution when one has a network device that
has multiple interfaces. This will cause all interfaces on that device
to be purged from the system when a PCS for one of the interfaces
goes away. If the system is using NFS-root, that could result in the
rootfs being lost. We should handle this more gracefully.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Powered by blists - more mailing lists