From: Clément Léger <clement.leger@bootlin.com>
Add support for the Renesas RZ/N1 GMAC. This support can make use of a
custom RZ/N1 PCS which is fetched by parsing the pcs-handle device tree
property.
Signed-off-by: "Clément Léger" <clement.leger@bootlin.com>
Co-developed-by: Romain Gantois <romain.gantois@bootlin.com>
Signed-off-by: Romain Gantois <romain.gantois@bootlin.com>
---
MAINTAINERS | 6 ++
drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++++
drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
drivers/net/ethernet/stmicro/stmmac/dwmac-rzn1.c | 87 ++++++++++++++++++++++++
4 files changed, 106 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index 6a233e1a3cf2..9735c7d2ee38 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18833,6 +18833,12 @@ F: include/dt-bindings/net/pcs-rzn1-miic.h
F: include/linux/pcs-rzn1-miic.h
F: net/dsa/tag_rzn1_a5psw.c
+RENESAS RZ/N1 DWMAC GLUE LAYER
+M: Romain Gantois <romain.gantois@bootlin.com>
+S: Maintained
+F: Documentation/devicetree/bindings/net/renesas,rzn1-gmac.yaml
+F: drivers/net/ethernet/stmicro/stmmac/dwmac-rzn1.c
+
RENESAS RZ/N1 RTC CONTROLLER DRIVER
M: Miquel Raynal <miquel.raynal@bootlin.com>
L: linux-rtc@vger.kernel.org
diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
index 4ec61f1ee71a..05cc07b8f48c 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
+++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
@@ -142,6 +142,18 @@ config DWMAC_ROCKCHIP
This selects the Rockchip RK3288 SoC glue layer support for
the stmmac device driver.
+config DWMAC_RZN1
+ tristate "Renesas RZ/N1 dwmac support"
+ default ARCH_RZN1
+ depends on OF && (ARCH_RZN1 || COMPILE_TEST)
+ select PCS_RZN1_MIIC
+ help
+ Support for Ethernet controller on Renesas RZ/N1 SoC family.
+
+ This selects the Renesas RZ/N1 SoC glue layer support for
+ the stmmac device driver. This support can make use of a custom MII
+ converter PCS device.
+
config DWMAC_SOCFPGA
tristate "SOCFPGA dwmac support"
default ARCH_INTEL_SOCFPGA
diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
index 26cad4344701..c2f0e91f6bf8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/Makefile
+++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
@@ -21,6 +21,7 @@ obj-$(CONFIG_DWMAC_MEDIATEK) += dwmac-mediatek.o
obj-$(CONFIG_DWMAC_MESON) += dwmac-meson.o dwmac-meson8b.o
obj-$(CONFIG_DWMAC_QCOM_ETHQOS) += dwmac-qcom-ethqos.o
obj-$(CONFIG_DWMAC_ROCKCHIP) += dwmac-rk.o
+obj-$(CONFIG_DWMAC_RZN1) += dwmac-rzn1.o
obj-$(CONFIG_DWMAC_SOCFPGA) += dwmac-altr-socfpga.o
obj-$(CONFIG_DWMAC_STARFIVE) += dwmac-starfive.o
obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-rzn1.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-rzn1.c
new file mode 100644
index 000000000000..5216d7890992
--- /dev/null
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-rzn1.c
@@ -0,0 +1,87 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 Schneider-Electric
+ *
+ * Clément Léger <clement.leger@bootlin.com>
+ */
+
+#include <linux/of.h>
+#include <linux/pcs-rzn1-miic.h>
+#include <linux/phylink.h>
+#include <linux/platform_device.h>
+
+#include "stmmac_platform.h"
+#include "stmmac.h"
+
+static int rzn1_dwmac_probe(struct platform_device *pdev)
+{
+ struct device_node *np = pdev->dev.of_node;
+ struct plat_stmmacenet_data *plat_dat;
+ struct stmmac_resources stmmac_res;
+ struct device *dev = &pdev->dev;
+ struct device_node *pcs_node;
+ struct stmmac_priv *priv;
+ struct phylink_pcs *pcs;
+ struct net_device *ndev;
+ int ret;
+
+ ret = stmmac_get_platform_resources(pdev, &stmmac_res);
+ if (ret)
+ return ret;
+
+ plat_dat = devm_stmmac_probe_config_dt(pdev, stmmac_res.mac);
+ if (IS_ERR(plat_dat))
+ return PTR_ERR(plat_dat);
+
+ plat_dat->bsp_priv = plat_dat;
+
+ ret = stmmac_dvr_probe(dev, plat_dat, &stmmac_res);
+ if (ret)
+ return ret;
+
+ ndev = platform_get_drvdata(pdev);
+ priv = netdev_priv(ndev);
+
+ pcs_node = of_parse_phandle(np, "pcs-handle", 0);
+ if (pcs_node) {
+ pcs = miic_create(dev, pcs_node);
+ of_node_put(pcs_node);
+ if (IS_ERR(pcs))
+ return PTR_ERR(pcs);
+
+ priv->hw->phylink_pcs = pcs;
+ }
+
+ return 0;
+}
+
+static void rzn1_dwmac_remove(struct platform_device *pdev)
+{
+ struct net_device *ndev = platform_get_drvdata(pdev);
+ struct stmmac_priv *priv = netdev_priv(ndev);
+
+ stmmac_pltfr_remove(pdev);
+
+ if (priv->hw->phylink_pcs)
+ miic_destroy(priv->hw->phylink_pcs);
+}
+
+static const struct of_device_id rzn1_dwmac_match[] = {
+ { .compatible = "renesas,rzn1-gmac" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, rzn1_dwmac_match);
+
+static struct platform_driver rzn1_dwmac_driver = {
+ .probe = rzn1_dwmac_probe,
+ .remove_new = rzn1_dwmac_remove,
+ .driver = {
+ .name = "rzn1-dwmac",
+ .of_match_table = rzn1_dwmac_match,
+ },
+};
+module_platform_driver(rzn1_dwmac_driver);
+
+MODULE_AUTHOR("Clément Léger <clement.leger@bootlin.com>");
+MODULE_DESCRIPTION("Renesas RZN1 DWMAC specific glue layer");
+MODULE_LICENSE("GPL");
--
2.44.0
On Tue, Apr 02, 2024 at 02:37:01PM +0200, Romain Gantois wrote:
> + ret = stmmac_dvr_probe(dev, plat_dat, &stmmac_res);
> + if (ret)
> + return ret;
> +
> + ndev = platform_get_drvdata(pdev);
> + priv = netdev_priv(ndev);
> +
> + pcs_node = of_parse_phandle(np, "pcs-handle", 0);
> + if (pcs_node) {
> + pcs = miic_create(dev, pcs_node);
> + of_node_put(pcs_node);
> + if (IS_ERR(pcs))
> + return PTR_ERR(pcs);
> +
> + priv->hw->phylink_pcs = pcs;
> + }
I'm afraid that this fails at one of the most basic principles of kernel
multi-threaded programming. stmmac_dvr_probe() as part of its work calls
register_netdev() which publishes to userspace the network device.
Everything that is required must be setup _prior_ to publication to
userspace to avoid races, because as soon as the network device is
published, userspace can decide to bring that interface up. If one
hasn't finished the initialisation, the interface can be brought up
before that initialisation is complete.
I don't see anything obvious in the stmmac data structures that would
allow you to hook in at an appropriate point before the
register_netdev() but after the netdev has been created. The
priv->hw data structure is created by stmmac_hwif_init()
I see that drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c is also
guilty of this as well, and should be fixed. It's even worse because it
does a truck load of stuff after stmmac_dvr_probe() which it most
definitely should not be doing.
I definitely get the feeling that the structure of the stmmac driver
is really getting out of hand, and is making stuff harder for people,
and it's not improving over time - in fact, it's getting worse. It
needs a *lot* of work to bring it back to a sane model.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Tue, Apr 02, 2024 at 02:49:48PM +0100, Russell King (Oracle) wrote:
> On Tue, Apr 02, 2024 at 02:37:01PM +0200, Romain Gantois wrote:
> > + ret = stmmac_dvr_probe(dev, plat_dat, &stmmac_res);
> > + if (ret)
> > + return ret;
> > +
> > + ndev = platform_get_drvdata(pdev);
> > + priv = netdev_priv(ndev);
> > +
> > + pcs_node = of_parse_phandle(np, "pcs-handle", 0);
> > + if (pcs_node) {
> > + pcs = miic_create(dev, pcs_node);
> > + of_node_put(pcs_node);
> > + if (IS_ERR(pcs))
> > + return PTR_ERR(pcs);
> > +
> > + priv->hw->phylink_pcs = pcs;
> > + }
>
> I'm afraid that this fails at one of the most basic principles of kernel
> multi-threaded programming. stmmac_dvr_probe() as part of its work calls
> register_netdev() which publishes to userspace the network device.
>
> Everything that is required must be setup _prior_ to publication to
> userspace to avoid races, because as soon as the network device is
> published, userspace can decide to bring that interface up. If one
> hasn't finished the initialisation, the interface can be brought up
> before that initialisation is complete.
>
> I don't see anything obvious in the stmmac data structures that would
> allow you to hook in at an appropriate point before the
> register_netdev() but after the netdev has been created. The
> priv->hw data structure is created by stmmac_hwif_init()
>
> I see that drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c is also
> guilty of this as well, and should be fixed. It's even worse because it
> does a truck load of stuff after stmmac_dvr_probe() which it most
> definitely should not be doing.
>
> I definitely get the feeling that the structure of the stmmac driver
> is really getting out of hand, and is making stuff harder for people,
> and it's not improving over time - in fact, it's getting worse. It
> needs a *lot* of work to bring it back to a sane model.
I'm not going to say that the two patches threaded to this email are
"sane" but at least it avoids the problem. socfpga still has issues
with initialisation done after register_netdev() though.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hello Russell, On Tue, 2 Apr 2024, Russell King (Oracle) wrote: > > I'm afraid that this fails at one of the most basic principles of kernel > > multi-threaded programming. stmmac_dvr_probe() as part of its work calls > > register_netdev() which publishes to userspace the network device. > > > > Everything that is required must be setup _prior_ to publication to > > userspace to avoid races, because as soon as the network device is > > published, userspace can decide to bring that interface up. If one > > hasn't finished the initialisation, the interface can be brought up > > before that initialisation is complete. ... > > I'm not going to say that the two patches threaded to this email are > "sane" but at least it avoids the problem. socfpga still has issues > with initialisation done after register_netdev() though. Thanks a lot for providing a fix to this issue, introducing new pcs_init/exit() hooks seems like the best solution at this time, I'll make sure to integrate those patches in the v2 for this series. Thanks, -- Romain Gantois, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Introduce a mechanism whereby platforms can create their PCS instances
prior to the network device being published to userspace, but after
some of the core stmmac initialisation has been completed. This means
that the data structures that platforms need will be available.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 14 ++++++++++++++
include/linux/stmmac.h | 2 ++
2 files changed, 16 insertions(+)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index fe3498e86de9..25fa33ae7017 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -7208,6 +7208,12 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
if (ret)
return ret;
+ if (priv->plat->pcs_init) {
+ ret = priv->plat->pcs_init(priv, priv->hw);
+ if (ret)
+ return ret;
+ }
+
/* Get the HW capability (new GMAC newer than 3.50a) */
priv->hw_cap_support = stmmac_get_hw_features(priv);
if (priv->hw_cap_support) {
@@ -7290,6 +7296,12 @@ static int stmmac_hw_init(struct stmmac_priv *priv)
return 0;
}
+static void stmmac_hw_exit(struct stmmac_priv *priv)
+{
+ if (priv->plat->pcs_exit)
+ priv->plat->pcs_exit(priv, priv->hw);
+}
+
static void stmmac_napi_add(struct net_device *dev)
{
struct stmmac_priv *priv = netdev_priv(dev);
@@ -7804,6 +7816,7 @@ int stmmac_dvr_probe(struct device *device,
priv->hw->pcs != STMMAC_PCS_RTBI)
stmmac_mdio_unregister(ndev);
error_mdio_register:
+ stmmac_hw_exit(priv);
stmmac_napi_del(ndev);
error_hw_init:
destroy_workqueue(priv->wq);
@@ -7844,6 +7857,7 @@ void stmmac_dvr_remove(struct device *dev)
if (priv->hw->pcs != STMMAC_PCS_TBI &&
priv->hw->pcs != STMMAC_PCS_RTBI)
stmmac_mdio_unregister(ndev);
+ stmmac_hw_exit(priv);
destroy_workqueue(priv->wq);
mutex_destroy(&priv->lock);
bitmap_free(priv->af_xdp_zc_qps);
diff --git a/include/linux/stmmac.h b/include/linux/stmmac.h
index dfa1828cd756..941fde506e51 100644
--- a/include/linux/stmmac.h
+++ b/include/linux/stmmac.h
@@ -285,6 +285,8 @@ struct plat_stmmacenet_data {
int (*crosststamp)(ktime_t *device, struct system_counterval_t *system,
void *ctx);
void (*dump_debug_regs)(void *priv);
+ int (*pcs_init)(struct stmmac_priv *priv, struct mac_device_info *hw);
+ void (*pcs_exit)(struct stmmac_priv *priv, struct mac_device_info *hw);
void *bsp_priv;
struct clk *stmmac_clk;
struct clk *pclk;
--
2.30.2
Hello Russell, On Tue, 02 Apr 2024 16:51:43 +0100 "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk> wrote: > Introduce a mechanism whereby platforms can create their PCS instances > prior to the network device being published to userspace, but after > some of the core stmmac initialisation has been completed. This means > that the data structures that platforms need will be available. > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Use the newly introduced pcs_init() and pcs_exit() operations to
create and destroy the PCS instance at a more appropriate moment during
the driver lifecycle, thereby avoiding publishing a network device to
userspace that has not yet finished its PCS initialisation.
There are other similar issues with this driver which remain
unaddressed, but these are out of scope for this patch.
Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk>
---
.../ethernet/stmicro/stmmac/dwmac-socfpga.c | 109 +++++++++---------
1 file changed, 55 insertions(+), 54 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
index 12b4a80ea3aa..67ca163936c8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c
@@ -379,6 +379,58 @@ static int socfpga_gen10_set_phy_mode(struct socfpga_dwmac *dwmac)
return 0;
}
+static int socfpga_dwmac_pcs_init(struct stmmac_priv *priv,
+ struct mac_device_info *hw)
+{
+ struct socfpga_dwmac *dwmac = priv->plat->bsp_priv;
+ struct regmap_config pcs_regmap_cfg = {
+ .reg_bits = 16,
+ .val_bits = 16,
+ .reg_shift = regmap_upshift(1),
+ };
+ struct mdio_regmap_config mrc;
+ struct regmap *pcs_regmap;
+ struct phylink_pcs *pcs;
+ struct mii_bus *pcs_bus;
+
+ if (!dwmac->tse_pcs_base)
+ return 0;
+
+ pcs_regmap = devm_regmap_init_mmio(priv->device, dwmac->tse_pcs_base,
+ &pcs_regmap_cfg);
+ if (IS_ERR(pcs_regmap))
+ return PTR_ERR(pcs_regmap);
+
+ memset(&mrc, 0, sizeof(mrc));
+ mrc.regmap = pcs_regmap;
+ mrc.parent = priv->device;
+ mrc.valid_addr = 0x0;
+ mrc.autoscan = false;
+
+ /* Can't use ndev->name here because it will not have been initialised,
+ * and in any case, the user can rename network interfaces at runtime.
+ */
+ snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii",
+ dev_name(priv->device));
+ pcs_bus = devm_mdio_regmap_register(priv->device, &mrc);
+ if (IS_ERR(pcs_bus))
+ return PTR_ERR(pcs_bus);
+
+ pcs = lynx_pcs_create_mdiodev(pcs_bus, 0);
+ if (IS_ERR(pcs))
+ return PTR_ERR(pcs);
+
+ hw->phylink_pcs = pcs;
+ return 0;
+}
+
+static void socfpga_dwmac_pcs_exit(struct stmmac_priv *priv,
+ struct mac_device_info *hw)
+{
+ if (hw->phylink_pcs)
+ lynx_pcs_destroy(hw->phylink_pcs);
+}
+
static int socfpga_dwmac_probe(struct platform_device *pdev)
{
struct plat_stmmacenet_data *plat_dat;
@@ -426,6 +478,8 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
dwmac->ops = ops;
plat_dat->bsp_priv = dwmac;
plat_dat->fix_mac_speed = socfpga_dwmac_fix_mac_speed;
+ plat_dat->pcs_init = socfpga_dwmac_pcs_init;
+ plat_dat->pcs_exit = socfpga_dwmac_pcs_exit;
ret = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
if (ret)
@@ -444,48 +498,6 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
if (ret)
goto err_dvr_remove;
- /* Create a regmap for the PCS so that it can be used by the PCS driver,
- * if we have such a PCS
- */
- if (dwmac->tse_pcs_base) {
- struct regmap_config pcs_regmap_cfg;
- struct mdio_regmap_config mrc;
- struct regmap *pcs_regmap;
- struct mii_bus *pcs_bus;
-
- memset(&pcs_regmap_cfg, 0, sizeof(pcs_regmap_cfg));
- memset(&mrc, 0, sizeof(mrc));
-
- pcs_regmap_cfg.reg_bits = 16;
- pcs_regmap_cfg.val_bits = 16;
- pcs_regmap_cfg.reg_shift = REGMAP_UPSHIFT(1);
-
- pcs_regmap = devm_regmap_init_mmio(&pdev->dev, dwmac->tse_pcs_base,
- &pcs_regmap_cfg);
- if (IS_ERR(pcs_regmap)) {
- ret = PTR_ERR(pcs_regmap);
- goto err_dvr_remove;
- }
-
- mrc.regmap = pcs_regmap;
- mrc.parent = &pdev->dev;
- mrc.valid_addr = 0x0;
- mrc.autoscan = false;
-
- snprintf(mrc.name, MII_BUS_ID_SIZE, "%s-pcs-mii", ndev->name);
- pcs_bus = devm_mdio_regmap_register(&pdev->dev, &mrc);
- if (IS_ERR(pcs_bus)) {
- ret = PTR_ERR(pcs_bus);
- goto err_dvr_remove;
- }
-
- stpriv->hw->phylink_pcs = lynx_pcs_create_mdiodev(pcs_bus, 0);
- if (IS_ERR(stpriv->hw->phylink_pcs)) {
- ret = PTR_ERR(stpriv->hw->phylink_pcs);
- goto err_dvr_remove;
- }
- }
-
return 0;
err_dvr_remove:
@@ -494,17 +506,6 @@ static int socfpga_dwmac_probe(struct platform_device *pdev)
return ret;
}
-static void socfpga_dwmac_remove(struct platform_device *pdev)
-{
- struct net_device *ndev = platform_get_drvdata(pdev);
- struct stmmac_priv *priv = netdev_priv(ndev);
- struct phylink_pcs *pcs = priv->hw->phylink_pcs;
-
- stmmac_pltfr_remove(pdev);
-
- lynx_pcs_destroy(pcs);
-}
-
#ifdef CONFIG_PM_SLEEP
static int socfpga_dwmac_resume(struct device *dev)
{
@@ -576,7 +577,7 @@ MODULE_DEVICE_TABLE(of, socfpga_dwmac_match);
static struct platform_driver socfpga_dwmac_driver = {
.probe = socfpga_dwmac_probe,
- .remove_new = socfpga_dwmac_remove,
+ .remove_new = stmmac_pltfr_remove,
.driver = {
.name = "socfpga-dwmac",
.pm = &socfpga_dwmac_pm_ops,
--
2.30.2
On Tue, Apr 02, 2024 at 04:51:48PM +0100, Russell King (Oracle) wrote:
> Use the newly introduced pcs_init() and pcs_exit() operations to
> create and destroy the PCS instance at a more appropriate moment during
> the driver lifecycle, thereby avoiding publishing a network device to
> userspace that has not yet finished its PCS initialisation.
>
> There are other similar issues with this driver which remain
> unaddressed, but these are out of scope for this patch.
Just for the record...
Digging into the history of this driver, the init-after-publish issue
was introduced by commit 3c201b5a84ed ("net: stmmac: socfpga: Remove
re-registration of reset controller") which gives information on why
calling the PHY configuration before stmmac_dvr_probe() didn't work.
This was further modified by 56868deece92 ("stmmac: dwmac-socfpga: add
PM ops and resume function").
I haven't decided what can be done about that yet - and I'm tempted to
leave it as-is for the time being until more of stmmac gets cleaned up.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hello Russell, On Tue, 02 Apr 2024 16:51:48 +0100 "Russell King (Oracle)" <rmk+kernel@armlinux.org.uk> wrote: > Use the newly introduced pcs_init() and pcs_exit() operations to > create and destroy the PCS instance at a more appropriate moment during > the driver lifecycle, thereby avoiding publishing a network device to > userspace that has not yet finished its PCS initialisation. > > There are other similar issues with this driver which remain > unaddressed, but these are out of scope for this patch. > > Signed-off-by: Russell King (Oracle) <rmk+kernel@armlinux.org.uk> Thanks for addressing this, Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
© 2016 - 2026 Red Hat, Inc.