Re: [PATCH v5 06/12] net: stmmac: Add glue layer for StarFive JH7110 SoC

Emil Renner Berthing
2023-03-10T00:40:16Z

On Fri, 10 Mar 2023 at 01:02, Emil Renner Berthing
<emil.renner.berthing@canonical.com> wrote:
> On Fri, 3 Mar 2023 at 10:01, Samin Guo <samin.guo@starfivetech.com> wrote:
> >
> > This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
> >
> > Co-developed-by: Emil Renner Berthing <kernel@esmil.dk>
> > Signed-off-by: Emil Renner Berthing <kernel@esmil.dk>
> > Signed-off-by: Samin Guo <samin.guo@starfivetech.com>
> > ---
> > MAINTAINERS | 1 +
> > drivers/net/ethernet/stmicro/stmmac/Kconfig | 12 ++
> > drivers/net/ethernet/stmicro/stmmac/Makefile | 1 +
> > .../ethernet/stmicro/stmmac/dwmac-starfive.c | 125 ++++++++++++++++++
> > 4 files changed, 139 insertions(+)
> > create mode 100644 drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 4e236b7c7fd2..91a4f190c827 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -19916,6 +19916,7 @@ STARFIVE DWMAC GLUE LAYER
> > M: Emil Renner Berthing <kernel@esmil.dk>
> > M: Samin Guo <samin.guo@starfivetech.com>
> > S: Maintained
> > +F: Documentation/devicetree/bindings/net/dwmac-starfive.c
> > F: Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> >
> > STARFIVE JH71X0 CLOCK DRIVERS
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/Kconfig b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > index f77511fe4e87..47fbccef9d04 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > +++ b/drivers/net/ethernet/stmicro/stmmac/Kconfig
> > @@ -165,6 +165,18 @@ config DWMAC_SOCFPGA
> > for the stmmac device driver. This driver is used for
> > arria5 and cyclone5 FPGA SoCs.
> >
> > +config DWMAC_STARFIVE
> > + tristate "StarFive dwmac support"
> > + depends on OF && (ARCH_STARFIVE || COMPILE_TEST)
> > + depends on STMMAC_ETH
> > + default ARCH_STARFIVE
> > + help
> > + Support for ethernet controllers on StarFive RISC-V SoCs
> > +
> > + This selects the StarFive platform specific glue layer support for
> > + the stmmac device driver. This driver is used for StarFive JH7110
> > + ethernet controller.
> > +
> > config DWMAC_STI
> > tristate "STi GMAC support"
> > default ARCH_STI
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/Makefile b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > index 057e4bab5c08..8738fdbb4b2d 100644
> > --- a/drivers/net/ethernet/stmicro/stmmac/Makefile
> > +++ b/drivers/net/ethernet/stmicro/stmmac/Makefile
> > @@ -23,6 +23,7 @@ obj-$(CONFIG_DWMAC_OXNAS) += dwmac-oxnas.o
> > obj-$(CONFIG_DWMAC_QCOM_ETHQOS) += dwmac-qcom-ethqos.o
> > obj-$(CONFIG_DWMAC_ROCKCHIP) += dwmac-rk.o
> > obj-$(CONFIG_DWMAC_SOCFPGA) += dwmac-altr-socfpga.o
> > +obj-$(CONFIG_DWMAC_STARFIVE) += dwmac-starfive.o
> > obj-$(CONFIG_DWMAC_STI) += dwmac-sti.o
> > obj-$(CONFIG_DWMAC_STM32) += dwmac-stm32.o
> > obj-$(CONFIG_DWMAC_SUNXI) += dwmac-sunxi.o
> > diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> > new file mode 100644
> > index 000000000000..566378306f67
> > --- /dev/null
> > +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-starfive.c
> > @@ -0,0 +1,125 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * StarFive DWMAC platform driver
> > + *
> > + * Copyright (C) 2022 StarFive Technology Co., Ltd.
> > + * Copyright (C) 2022 Emil Renner Berthing <kernel@esmil.dk>
> > + *
> > + */
> > +
> > +#include <linux/of_device.h>
> > +
> > +#include "stmmac_platform.h"
> > +
> > +struct starfive_dwmac {
> > + struct device *dev;
> > + struct clk *clk_tx;
> > + struct clk *clk_gtx;
> > + bool tx_use_rgmii_rxin_clk;
> > +};
> > +
> > +static void starfive_eth_fix_mac_speed(void *priv, unsigned int speed)
> > +{
> > + struct starfive_dwmac *dwmac = priv;
> > + unsigned long rate;
> > + int err;
> > +
> > + /* Generally, the rgmii_tx clock is provided by the internal clock,
> > + * which needs to match the corresponding clock frequency according
> > + * to different speeds. If the rgmii_tx clock is provided by the
> > + * external rgmii_rxin, there is no need to configure the clock
> > + * internally, because rgmii_rxin will be adaptively adjusted.
> > + */
> > + if (dwmac->tx_use_rgmii_rxin_clk)
> > + return;
> > +
> > + switch (speed) {
> > + case SPEED_1000:
> > + rate = 125000000;
> > + break;
> > + case SPEED_100:
> > + rate = 25000000;
> > + break;
> > + case SPEED_10:
> > + rate = 2500000;
> > + break;
> > + default:
> > + dev_err(dwmac->dev, "invalid speed %u\n", speed);
> > + break;
> > + }
> > +
> > + err = clk_set_rate(dwmac->clk_tx, rate);
>
> Hi Samin,
>
> I tried exercising this code by forcing the interface to downgrade
> from 1000Mbps to 100Mbps (ethtool -s end0 speed 100), and it doesn't
> seem to work. The reason is that clk_tx is a mux, and when you call
> clk_set_rate it will try to find the parent with the closest clock
> rate instead of adjusting the current parent as is needed here.
> However that is easily fixed by calling clk_set_rate on clk_gtx which
> is just a gate that *will* propagate the rate change to the parent.
>
> With this change, this piece of code and downgrading from 1000Mbps to
> 100Mbps works on the JH7100. However on the JH7110 there is a second
> problem. The parent of clk_gtx, confusingly called
> clk_gmac{0,1}_gtxclk is a divider (and gate) that takes the 1GHz PLL0
> clock and divides it by some integer. But according to [1] it can at
> most divide by 15 which is not enough to generate the 25MHz clock
> needed for 100Mbps. So now I wonder how this is supposed to work on
> the JH7110.
>
> [1]: https://doc-en.rvspace.org/JH7110/TRM/JH7110_TRM/sys_crg.html#sys_crg__section_skz_fxm_wsb

Ah, I see now that gmac0_gtxclk is only used by gmac0 on the
VisionFive 2 v1.2A, where I think it's a known problem that only
1000Mbps works.
On the 1.3B this function is not used at all, and I guess it also
ought to be skipped for gmac1 of the 1.2A using the rmii interface so
it doesn't risk changing the parent of the tx clock.

> > + if (err)
> > + dev_err(dwmac->dev, "failed to set tx rate %lu\n", rate);
> > +}
> > +
> > +static int starfive_dwmac_probe(struct platform_device *pdev)
> > +{
> > + struct plat_stmmacenet_data *plat_dat;
> > + struct stmmac_resources stmmac_res;
> > + struct starfive_dwmac *dwmac;
> > + int err;
> > +
> > + err = stmmac_get_platform_resources(pdev, &stmmac_res);
> > + if (err)
> > + return err;
> > +
> > + plat_dat = stmmac_probe_config_dt(pdev, stmmac_res.mac);
> > + if (IS_ERR(plat_dat)) {
> > + dev_err(&pdev->dev, "dt configuration failed\n");
> > + return PTR_ERR(plat_dat);
> > + }
> > +
> > + dwmac = devm_kzalloc(&pdev->dev, sizeof(*dwmac), GFP_KERNEL);
> > + if (!dwmac)
> > + return -ENOMEM;
> > +
> > + dwmac->clk_tx = devm_clk_get_enabled(&pdev->dev, "tx");
> > + if (IS_ERR(dwmac->clk_tx))
> > + return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_tx),
> > + "error getting tx clock\n");
> > +
> > + dwmac->clk_gtx = devm_clk_get_enabled(&pdev->dev, "gtx");
> > + if (IS_ERR(dwmac->clk_gtx))
> > + return dev_err_probe(&pdev->dev, PTR_ERR(dwmac->clk_gtx),
> > + "error getting gtx clock\n");
> > +
> > + if (device_property_read_bool(&pdev->dev, "starfive,tx-use-rgmii-clk"))
> > + dwmac->tx_use_rgmii_rxin_clk = true;
> > +
> > + dwmac->dev = &pdev->dev;
> > + plat_dat->fix_mac_speed = starfive_eth_fix_mac_speed;
> > + plat_dat->init = NULL;
> > + plat_dat->bsp_priv = dwmac;
> > + plat_dat->dma_cfg->dche = true;
> > +
> > + err = stmmac_dvr_probe(&pdev->dev, plat_dat, &stmmac_res);
> > + if (err) {
> > + stmmac_remove_config_dt(pdev, plat_dat);
> > + return err;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct of_device_id starfive_dwmac_match[] = {
> > + { .compatible = "starfive,jh7110-dwmac" },
> > + { /* sentinel */ }
> > +};
> > +MODULE_DEVICE_TABLE(of, starfive_dwmac_match);
> > +
> > +static struct platform_driver starfive_dwmac_driver = {
> > + .probe = starfive_dwmac_probe,
> > + .remove = stmmac_pltfr_remove,
> > + .driver = {
> > + .name = "starfive-dwmac",
> > + .pm = &stmmac_pltfr_pm_ops,
> > + .of_match_table = starfive_dwmac_match,
> > + },
> > +};
> > +module_platform_driver(starfive_dwmac_driver);
> > +
> > +MODULE_LICENSE("GPL");
> > +MODULE_DESCRIPTION("StarFive DWMAC platform driver");
> > +MODULE_AUTHOR("Emil Renner Berthing <kernel@esmil.dk>");
> > +MODULE_AUTHOR("Samin Guo <samin.guo@starfivetech.com>");
> > --
> > 2.17.1
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv