| Message ID | 20230303085928.4535-7-samin.guo@starfivetech.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | Add Ethernet driver for StarFive JH7110 SoC | expand |
| Context | Check | Description |
|---|---|---|
| netdev/tree_selection | success | Guessing tree name failed - patch did not apply |
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) There is an extra space between "OF" and "&&" here. > + depends on STMMAC_ETH It's not visible in this patch context, but this whole config option is surrounded by "if STMMAC_ETH" and "if STMMAC_PLATFORM", so "depends on STMMAC_ETH" should not be needed. > + default ARCH_STARFIVE This driver is not required to boot the JH7110, so we should just default to building it as a module. Eg. default m if 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> Sorry, after looking at my old git branches where this started as a driver for the JH7100 this should really be * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk> * Copyright (C) 2022 StarFive Technology Co., Ltd. > + */ > + > +#include <linux/of_device.h> > + > +#include "stmmac_platform.h" > + > +struct starfive_dwmac { > + struct device *dev; > + struct clk *clk_tx; > + struct clk *clk_gtx; This pointer is only set, but never read. Please remove it. > + bool tx_use_rgmii_rxin_clk; > +}; > + > +static void starfive_eth_fix_mac_speed(void *priv, unsigned int speed) This should be starfive_dwmac_fix_mac_speed for consistency. > +{ > + 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; If this function is only needed in certain situations, why not just set the plat_dat->fix_mac_speed callback when it is needed? > + 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); > + 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; Eg.: if (!device_property_read_bool(&pdev->dev, "starfive,tx_use_rgmii_clk")) plat_dat->fix_mac_speed = starfive_dwmac_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
在 2023/3/4 0:18:20, Emil Renner Berthing 写道: > 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) > > There is an extra space between "OF" and "&&" here. > will drop it > >> + depends on STMMAC_ETH > > It's not visible in this patch context, but this whole config option > is surrounded by "if STMMAC_ETH" and "if STMMAC_PLATFORM", so "depends > on STMMAC_ETH" should not be needed. > will drop it. >> + default ARCH_STARFIVE > > This driver is not required to boot the JH7110, so we should just > default to building it as a module. Eg. > default m if ARCH_STARFIVE Yes, this driver is not required to boot the JH7110, but the network is a very basic module, it seems that other dwmac-platforms have been compiled into the kernel instead of modules. > >> + 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> > > Sorry, after looking at my old git branches where this started as a > driver for the JH7100 this should really be > * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk> > * Copyright (C) 2022 StarFive Technology Co., Ltd. > OK, It should be. >> + */ >> + >> +#include <linux/of_device.h> >> + >> +#include "stmmac_platform.h" >> + >> +struct starfive_dwmac { >> + struct device *dev; >> + struct clk *clk_tx; >> + struct clk *clk_gtx; > > This pointer is only set, but never read. Please remove it. >> >> + bool tx_use_rgmii_rxin_clk; >> +}; >> + >> +static void starfive_eth_fix_mac_speed(void *priv, unsigned int speed) > > This should be starfive_dwmac_fix_mac_speed for consistency. > Sorry，I missed this, will fix next version. >> +{ >> + 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; > > If this function is only needed in certain situations, why not just > set the plat_dat->fix_mac_speed callback when it is needed? > Sounds good idea. >> + 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); >> + 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; > > Eg.: > if (!device_property_read_bool(&pdev->dev, "starfive,tx_use_rgmii_clk")) > plat_dat->fix_mac_speed = starfive_dwmac_fix_mac_speed; > Good idea, so we can remove flag 'tx_use_rgmii_rxin_clk' in struct starfive_dwmac. >> + 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 Best regards, Samin
On Mon, 6 Mar 2023 at 08:16, Guo Samin <samin.guo@starfivetech.com> wrote: > 在 2023/3/4 0:18:20, Emil Renner Berthing 写道: > > 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) > > > > There is an extra space between "OF" and "&&" here. > > > will drop it > > > >> + depends on STMMAC_ETH > > > > It's not visible in this patch context, but this whole config option > > is surrounded by "if STMMAC_ETH" and "if STMMAC_PLATFORM", so "depends > > on STMMAC_ETH" should not be needed. > > > will drop it. > >> + default ARCH_STARFIVE > > > > This driver is not required to boot the JH7110, so we should just > > default to building it as a module. Eg. > > default m if ARCH_STARFIVE > > Yes, this driver is not required to boot the JH7110, but the network is a very basic module, > it seems that other dwmac-platforms have been compiled into the kernel instead of modules. Right, but the defconfig should work on as many platforms as possible, so if we build in every "basic" module for every platform the kernel will be huge and waste a lot of memory on drivers that will never be used. Also even if this driver was built in the gmac0 would still not work until the driver for the AON CRG is loaded, which also defaults to m for the same reasons. > > > >> + 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> > > > > Sorry, after looking at my old git branches where this started as a > > driver for the JH7100 this should really be > > * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk> > > * Copyright (C) 2022 StarFive Technology Co., Ltd. > > > OK, It should be. > >> + */ > >> + > >> +#include <linux/of_device.h> > >> + > >> +#include "stmmac_platform.h" > >> + > >> +struct starfive_dwmac { > >> + struct device *dev; > >> + struct clk *clk_tx; > >> + struct clk *clk_gtx; > > > > This pointer is only set, but never read. Please remove it. > >> > >> + bool tx_use_rgmii_rxin_clk; > >> +}; > >> + > >> +static void starfive_eth_fix_mac_speed(void *priv, unsigned int speed) > > > > This should be starfive_dwmac_fix_mac_speed for consistency. > > > Sorry，I missed this, will fix next version. > >> +{ > >> + 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; > > > > If this function is only needed in certain situations, why not just > > set the plat_dat->fix_mac_speed callback when it is needed? > > > Sounds good idea. > >> + 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); > >> + 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; > > > > Eg.: > > if (!device_property_read_bool(&pdev->dev, "starfive,tx_use_rgmii_clk")) > > plat_dat->fix_mac_speed = starfive_dwmac_fix_mac_speed; > > > Good idea, so we can remove flag 'tx_use_rgmii_rxin_clk' in struct starfive_dwmac. > >> + plat_dat->init = NULL; Btw. plat_dat is initialized by kzalloc in stmmac_probe_config_dt and I can't seem to find anything that sets plat_dat->init, so I think this is redundant. > >> + 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 > Best regards, > Samin > > -- > Best regards, > Samin
在 2023/3/6 20:21:28, Emil Renner Berthing 写道: > On Mon, 6 Mar 2023 at 08:16, Guo Samin <samin.guo@starfivetech.com> wrote: >> 在 2023/3/4 0:18:20, Emil Renner Berthing 写道: >>> 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) >>> >>> There is an extra space between "OF" and "&&" here. >>> >> will drop it >>> >>>> + depends on STMMAC_ETH >>> >>> It's not visible in this patch context, but this whole config option >>> is surrounded by "if STMMAC_ETH" and "if STMMAC_PLATFORM", so "depends >>> on STMMAC_ETH" should not be needed. >>> >> will drop it. >>>> + default ARCH_STARFIVE >>> >>> This driver is not required to boot the JH7110, so we should just >>> default to building it as a module. Eg. >>> default m if ARCH_STARFIVE >> >> Yes, this driver is not required to boot the JH7110, but the network is a very basic module, >> it seems that other dwmac-platforms have been compiled into the kernel instead of modules. > > Right, but the defconfig should work on as many platforms as possible, > so if we build in every "basic" module for every platform the kernel > will be huge and waste a lot of memory on drivers that will never be > used. > > Also even if this driver was built in the gmac0 would still not work > until the driver for the AON CRG is loaded, which also defaults to m > for the same reasons. > OK, I will compile it into a module in the next version. >>> >>>> + 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> >>> >>> Sorry, after looking at my old git branches where this started as a >>> driver for the JH7100 this should really be >>> * Copyright (C) 2021 Emil Renner Berthing <kernel@esmil.dk> >>> * Copyright (C) 2022 StarFive Technology Co., Ltd. >>> >> OK, It should be. >>>> + */ >>>> + >>>> +#include <linux/of_device.h> >>>> + >>>> +#include "stmmac_platform.h" >>>> + >>>> +struct starfive_dwmac { >>>> + struct device *dev; >>>> + struct clk *clk_tx; >>>> + struct clk *clk_gtx; >>> >>> This pointer is only set, but never read. Please remove it. >>>> >>>> + bool tx_use_rgmii_rxin_clk; >>>> +}; >>>> + >>>> +static void starfive_eth_fix_mac_speed(void *priv, unsigned int speed) >>> >>> This should be starfive_dwmac_fix_mac_speed for consistency. >>> >> Sorry，I missed this, will fix next version. >>>> +{ >>>> + 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; >>> >>> If this function is only needed in certain situations, why not just >>> set the plat_dat->fix_mac_speed callback when it is needed? >>> >> Sounds good idea. >>>> + 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); >>>> + 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; >>> >>> Eg.: >>> if (!device_property_read_bool(&pdev->dev, "starfive,tx_use_rgmii_clk")) >>> plat_dat->fix_mac_speed = starfive_dwmac_fix_mac_speed; >>> >> Good idea, so we can remove flag 'tx_use_rgmii_rxin_clk' in struct starfive_dwmac. >>>> + plat_dat->init = NULL; > > Btw. plat_dat is initialized by kzalloc in stmmac_probe_config_dt and > I can't seem to find anything that sets plat_dat->init, so I think > this is redundant. > >>>> + 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 >> Best regards, >> Samin >> >> -- >> Best regards, >> Samin
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 > + 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
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
-------- 原始信息 -------- 主题: Re: [PATCH v5 06/12] net: stmmac: Add glue layer for StarFive JH7110 SoC From: Emil Renner Berthing <emil.renner.berthing@canonical.com> 收件人: Samin Guo <samin.guo@starfivetech.com> 日期: 2023/3/10 > 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. > Hi Emil, V1.2A gmac0 only supports 1000M due to known problem, and v1.2A gmac1 supports 100M/10M. V1.2A gmac1 uses a parent clock from gmac1_rmii_rtx, whose parent clock is from external phy clock gmac1_rmii_refin (fixed is 50M). The default frequency division value of gmac1_rmii_rtx is 2, so it can work in 100M mode. （clk_tx: 50/2=25M ===> 100M mode）. When gmac1 switches to 10M mode, the clock frequency of gmac1_rmii_rtx needs to be modified to 2.5M. So,if 1.2A gmac1 is skipped the starfive_eth_fix_mac_speed, 10M mode will be unavailable. gmac1_rmii_refin（50M) ==> gmac1_rmii_rtx（div 2, by default) ==> clk_tx (25M) （100M mode） gmac1_rmii_refin（50M）==> gmac1_rmii_rtx（div 20) ==> clk_tx (2.5M) （10M mode） Of course, as you mentioned earlier, we need to add gmac1_clk_tx uses CLK_SET_RATE_PARENT flag. Best regards, Samin >>> + 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
On Fri, 10 Mar 2023 at 02:55, Guo Samin <samin.guo@starfivetech.com> wrote: > -------- 原始信息 -------- > 主题: Re: [PATCH v5 06/12] net: stmmac: Add glue layer for StarFive JH7110 SoC > From: Emil Renner Berthing <emil.renner.berthing@canonical.com> > 收件人: Samin Guo <samin.guo@starfivetech.com> > 日期: 2023/3/10 > > > 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. > > > Hi Emil, > > V1.2A gmac0 only supports 1000M due to known problem, and v1.2A gmac1 supports 100M/10M. > > V1.2A gmac1 uses a parent clock from gmac1_rmii_rtx, whose parent clock is from external phy clock gmac1_rmii_refin (fixed is 50M). > The default frequency division value of gmac1_rmii_rtx is 2, so it can work in 100M mode. （clk_tx: 50/2=25M ===> 100M mode）. > When gmac1 switches to 10M mode, the clock frequency of gmac1_rmii_rtx needs to be modified to 2.5M. > So,if 1.2A gmac1 is skipped the starfive_eth_fix_mac_speed, 10M mode will be unavailable. > > gmac1_rmii_refin（50M) ==> gmac1_rmii_rtx（div 2, by default) ==> clk_tx (25M) （100M mode） > gmac1_rmii_refin（50M）==> gmac1_rmii_rtx（div 20) ==> clk_tx (2.5M) （10M mode） > I see. So on the JH7110 it is only when using gmac{0,1}_rmii_rtx -> clk_tx with the rmii interface that this function is needed? As noted above using the current fix_mac_speed with gmac{0,1}_gtxclk -> clk_tx will produce wrong results, so for the VF2 v1.2A you probably just want something like this in the device tree &gmac0 { assigned-clocks = <&aoncrg JH7110_AONCLK_GMAC0_TX>, <&syscrg JH7110_SYSCLK_GMAC0_GTXCLK>; assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC0_GTXCLK>; assigned-clock-rates = <0> <125000000>; }; ..and then don't set the fix_mac_speed callback. > Of course, as you mentioned earlier, we need to add gmac1_clk_tx uses CLK_SET_RATE_PARENT flag. Yes, I'm not too sure how clk_set_rate on mux'es are supposed to work, but if you can convince Hal and Stephen (the clock maintainer) that clk_set_rate should always propagate the rate change to the current parent, then I'm fine with it. Alternatively you can add an optional clock to the bindings, and only if the optional clock is set then set the fix_mac_speed callback to modify the rate of that clock. This way you won't need the special "starfive,tx-use-rgmii-clk" flag either. /Emil > > Best regards, > Samin > > >>> + 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 > >
-------- 原始信息 -------- 主题: Re: [PATCH v5 06/12] net: stmmac: Add glue layer for StarFive JH7110 SoC From: Emil Renner Berthing <emil.renner.berthing@canonical.com> 收件人: Guo Samin <samin.guo@starfivetech.com> 日期: 2023/3/10 > On Fri, 10 Mar 2023 at 02:55, Guo Samin <samin.guo@starfivetech.com> wrote: >> -------- 原始信息 -------- >> 主题: Re: [PATCH v5 06/12] net: stmmac: Add glue layer for StarFive JH7110 SoC >> From: Emil Renner Berthing <emil.renner.berthing@canonical.com> >> 收件人: Samin Guo <samin.guo@starfivetech.com> >> 日期: 2023/3/10 >> >>> 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. >>> >> Hi Emil, >> >> V1.2A gmac0 only supports 1000M due to known problem, and v1.2A gmac1 supports 100M/10M. >> >> V1.2A gmac1 uses a parent clock from gmac1_rmii_rtx, whose parent clock is from external phy clock gmac1_rmii_refin (fixed is 50M). >> The default frequency division value of gmac1_rmii_rtx is 2, so it can work in 100M mode. （clk_tx: 50/2=25M ===> 100M mode）. >> When gmac1 switches to 10M mode, the clock frequency of gmac1_rmii_rtx needs to be modified to 2.5M. >> So,if 1.2A gmac1 is skipped the starfive_eth_fix_mac_speed, 10M mode will be unavailable. >> >> gmac1_rmii_refin（50M) ==> gmac1_rmii_rtx（div 2, by default) ==> clk_tx (25M) （100M mode） >> gmac1_rmii_refin（50M）==> gmac1_rmii_rtx（div 20) ==> clk_tx (2.5M) （10M mode） >> > > I see. So on the JH7110 it is only when using gmac{0,1}_rmii_rtx -> > clk_tx with the rmii interface that this function is needed? > > As noted above using the current fix_mac_speed with gmac{0,1}_gtxclk > -> clk_tx will produce wrong results, so for the VF2 v1.2A you > probably just want something like this in the device tree > &gmac0 { > assigned-clocks = <&aoncrg JH7110_AONCLK_GMAC0_TX>, <&syscrg > JH7110_SYSCLK_GMAC0_GTXCLK>; > assigned-clock-parents = <&syscrg JH7110_SYSCLK_GMAC0_GTXCLK>; > assigned-clock-rates = <0> <125000000>; > }; > > ..and then don't set the fix_mac_speed callback. > Hi Emil, This works on v1.2a with gmac0, but gmac1 does not. gmac1 have to setting the fix_mac_speed as mentioned above。 Moreover，fix_mac_speed is also can be used on JH7100 I guess. So, keeping fix_mac_speed callback can make the code more compatible? Best regards, Samin >> Of course, as you mentioned earlier, we need to add gmac1_clk_tx uses CLK_SET_RATE_PARENT flag. > > Yes, I'm not too sure how clk_set_rate on mux'es are supposed to work, > but if you can convince Hal and Stephen (the clock maintainer) that > clk_set_rate should always propagate the rate change to the current > parent, then I'm fine with it. > I tested it with CLK_SET_RATE_PARENT flag in gmacX_clk_tx with HAL's clktree driver, Setting the frequency of gmacX_clk_tx can take effect on clk_rmii_rtx. Tested on v1.2A: # mount -t debugfs none /sys/kernel/debug # cat /sys/kernel/debug/clk/gmac1_tx/clk_parent gmac1_rmii_rtx # cat /sys/kernel/debug/clk/gmac1_tx/clk_rate 25000000 # cat /sys/kernel/debug/clk/gmac1_rmii_rtx/clk_rate 25000000 # ethtool -s eth1 speed 10 duplex full # starfive-dwmac 16040000.ethernet eth1: Link is Down starfive-dwmac 16040000.ethernet eth1: Link is Up - 10Mbps/Full - flow control rx/tx # cat /sys/kernel/debug/clk/gmac1_tx/clk_rate 2500000 # cat /sys/kernel/debug/clk/gmac1_rmii_rtx/clk_rate 2500000 In addition, I have tested that all the following modes can work: For v1.2A: gmac0: 1000M gmac1: 100M/10M For v1.3B: gmac0: 1000M/100M/10M gmac1: 1000M/100M/10M Best regards, Samin > Alternatively you can add an optional clock to the bindings, and only > if the optional clock is set then set the fix_mac_speed callback to > modify the rate of that clock. This way you won't need the special > "starfive,tx-use-rgmii-clk" flag either. > > /Emil >> >> Best regards, >> Samin >> >>>>> + 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 >> >>
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); + 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>");