This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
Co-developed-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Emil Renner Berthing <[email protected]>
Signed-off-by: Samin Guo <[email protected]>
---
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 <[email protected]>
M: Samin Guo <[email protected]>
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 <[email protected]>
+ *
+ */
+
+#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 <[email protected]>");
+MODULE_AUTHOR("Samin Guo <[email protected]>");
--
2.17.1
On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> wrote:
> This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
>
> Co-developed-by: Emil Renner Berthing <[email protected]>
> Signed-off-by: Emil Renner Berthing <[email protected]>
> Signed-off-by: Samin Guo <[email protected]>
> ---
> 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 <[email protected]>
> M: Samin Guo <[email protected]>
> 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 <[email protected]>
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 <[email protected]>
* 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 <[email protected]>");
> +MODULE_AUTHOR("Samin Guo <[email protected]>");
> --
> 2.17.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> 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 <[email protected]> wrote:
>> This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
>>
>> Co-developed-by: Emil Renner Berthing <[email protected]>
>> Signed-off-by: Emil Renner Berthing <[email protected]>
>> Signed-off-by: Samin Guo <[email protected]>
>> ---
>> 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 <[email protected]>
>> M: Samin Guo <[email protected]>
>> 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 <[email protected]>
>
> 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 <[email protected]>
> * 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 <[email protected]>");
>> +MODULE_AUTHOR("Samin Guo <[email protected]>");
>> --
>> 2.17.1
>>
>>
>> _______________________________________________
>> linux-riscv mailing list
>> [email protected]
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
Best regards,
Samin
--
Best regards,
Samin
On Mon, 6 Mar 2023 at 08:16, Guo Samin <[email protected]> wrote:
> 在 2023/3/4 0:18:20, Emil Renner Berthing 写道:
> > On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> wrote:
> >> This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
> >>
> >> Co-developed-by: Emil Renner Berthing <[email protected]>
> >> Signed-off-by: Emil Renner Berthing <[email protected]>
> >> Signed-off-by: Samin Guo <[email protected]>
> >> ---
> >> 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 <[email protected]>
> >> M: Samin Guo <[email protected]>
> >> 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 <[email protected]>
> >
> > 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 <[email protected]>
> > * 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 <[email protected]>");
> >> +MODULE_AUTHOR("Samin Guo <[email protected]>");
> >> --
> >> 2.17.1
> >>
> >>
> >> _______________________________________________
> >> linux-riscv mailing list
> >> [email protected]
> >> 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 <[email protected]> wrote:
>> 在 2023/3/4 0:18:20, Emil Renner Berthing 写道:
>>> On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> wrote:
>>>> This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
>>>>
>>>> Co-developed-by: Emil Renner Berthing <[email protected]>
>>>> Signed-off-by: Emil Renner Berthing <[email protected]>
>>>> Signed-off-by: Samin Guo <[email protected]>
>>>> ---
>>>> 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 <[email protected]>
>>>> M: Samin Guo <[email protected]>
>>>> 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 <[email protected]>
>>>
>>> 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 <[email protected]>
>>> * 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 <[email protected]>");
>>>> +MODULE_AUTHOR("Samin Guo <[email protected]>");
>>>> --
>>>> 2.17.1
>>>>
>>>>
>>>> _______________________________________________
>>>> linux-riscv mailing list
>>>> [email protected]
>>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>> Best regards,
>> Samin
>>
>> --
>> Best regards,
>> Samin
--
Best regards,
Samin
On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> wrote:
>
> This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
>
> Co-developed-by: Emil Renner Berthing <[email protected]>
> Signed-off-by: Emil Renner Berthing <[email protected]>
> Signed-off-by: Samin Guo <[email protected]>
> ---
> 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 <[email protected]>
> M: Samin Guo <[email protected]>
> 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 <[email protected]>
> + *
> + */
> +
> +#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 <[email protected]>");
> +MODULE_AUTHOR("Samin Guo <[email protected]>");
> --
> 2.17.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Fri, 10 Mar 2023 at 01:02, Emil Renner Berthing
<[email protected]> wrote:
> On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> wrote:
> >
> > This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
> >
> > Co-developed-by: Emil Renner Berthing <[email protected]>
> > Signed-off-by: Emil Renner Berthing <[email protected]>
> > Signed-off-by: Samin Guo <[email protected]>
> > ---
> > 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 <[email protected]>
> > M: Samin Guo <[email protected]>
> > 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 <[email protected]>
> > + *
> > + */
> > +
> > +#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 <[email protected]>");
> > +MODULE_AUTHOR("Samin Guo <[email protected]>");
> > --
> > 2.17.1
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > [email protected]
> > 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 <[email protected]>
收件人: Samin Guo <[email protected]>
日期: 2023/3/10
> On Fri, 10 Mar 2023 at 01:02, Emil Renner Berthing
> <[email protected]> wrote:
>> On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> wrote:
>>>
>>> This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
>>>
>>> Co-developed-by: Emil Renner Berthing <[email protected]>
>>> Signed-off-by: Emil Renner Berthing <[email protected]>
>>> Signed-off-by: Samin Guo <[email protected]>
>>> ---
>>> 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 <[email protected]>
>>> M: Samin Guo <[email protected]>
>>> 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 <[email protected]>
>>> + *
>>> + */
>>> +
>>> +#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 <[email protected]>");
>>> +MODULE_AUTHOR("Samin Guo <[email protected]>");
>>> --
>>> 2.17.1
>>>
>>>
>>> _______________________________________________
>>> linux-riscv mailing list
>>> [email protected]
>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Fri, 10 Mar 2023 at 02:55, Guo Samin <[email protected]> wrote:
> -------- 原始信息 --------
> 主题: Re: [PATCH v5 06/12] net: stmmac: Add glue layer for StarFive JH7110 SoC
> From: Emil Renner Berthing <[email protected]>
> 收件人: Samin Guo <[email protected]>
> 日期: 2023/3/10
>
> > On Fri, 10 Mar 2023 at 01:02, Emil Renner Berthing
> > <[email protected]> wrote:
> >> On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> wrote:
> >>>
> >>> This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
> >>>
> >>> Co-developed-by: Emil Renner Berthing <[email protected]>
> >>> Signed-off-by: Emil Renner Berthing <[email protected]>
> >>> Signed-off-by: Samin Guo <[email protected]>
> >>> ---
> >>> 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 <[email protected]>
> >>> M: Samin Guo <[email protected]>
> >>> 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 <[email protected]>
> >>> + *
> >>> + */
> >>> +
> >>> +#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 <[email protected]>");
> >>> +MODULE_AUTHOR("Samin Guo <[email protected]>");
> >>> --
> >>> 2.17.1
> >>>
> >>>
> >>> _______________________________________________
> >>> linux-riscv mailing list
> >>> [email protected]
> >>> 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 <[email protected]>
收件人: Guo Samin <[email protected]>
日期: 2023/3/10
> On Fri, 10 Mar 2023 at 02:55, Guo Samin <[email protected]> wrote:
>> -------- 原始信息 --------
>> 主题: Re: [PATCH v5 06/12] net: stmmac: Add glue layer for StarFive JH7110 SoC
>> From: Emil Renner Berthing <[email protected]>
>> 收件人: Samin Guo <[email protected]>
>> 日期: 2023/3/10
>>
>>> On Fri, 10 Mar 2023 at 01:02, Emil Renner Berthing
>>> <[email protected]> wrote:
>>>> On Fri, 3 Mar 2023 at 10:01, Samin Guo <[email protected]> wrote:
>>>>>
>>>>> This adds StarFive dwmac driver support on the StarFive JH7110 SoC.
>>>>>
>>>>> Co-developed-by: Emil Renner Berthing <[email protected]>
>>>>> Signed-off-by: Emil Renner Berthing <[email protected]>
>>>>> Signed-off-by: Samin Guo <[email protected]>
>>>>> ---
>>>>> 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 <[email protected]>
>>>>> M: Samin Guo <[email protected]>
>>>>> 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 <[email protected]>
>>>>> + *
>>>>> + */
>>>>> +
>>>>> +#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 <[email protected]>");
>>>>> +MODULE_AUTHOR("Samin Guo <[email protected]>");
>>>>> --
>>>>> 2.17.1
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> linux-riscv mailing list
>>>>> [email protected]
>>>>> http://lists.infradead.org/mailman/listinfo/linux-riscv
>>
>>
--
Best regards,
Samin