| Message ID | 20200512095603.29126-5-david.wu@rock-chips.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add dwc_eth_qos support for rockchip | expand |
Hi David On 5/12/20 11:56 AM, David Wu wrote: > For others using, clk_rx and clk_tx may not be necessary, > and their clock names are different. > > Signed-off-by: David Wu <david.wu at rock-chips.com> > --- > > Changes in v2: > - Don't change the Rx and Tx clock names. (Patrice, Stephen) > > drivers/net/dwc_eth_qos.c | 61 +++++++++++++++++++-------------------- > 1 file changed, 29 insertions(+), 32 deletions(-) > > diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c > index ae2167637f..bec9bf556b 100644 > --- a/drivers/net/dwc_eth_qos.c > +++ b/drivers/net/dwc_eth_qos.c > @@ -613,16 +613,20 @@ static int eqos_start_clks_stm32(struct udevice *dev) > goto err; > } > > - ret = clk_enable(&eqos->clk_rx); > - if (ret < 0) { > - pr_err("clk_enable(clk_rx) failed: %d", ret); > - goto err_disable_clk_master_bus; > + if (clk_valid(&eqos->clk_rx)) { > + ret = clk_enable(&eqos->clk_rx); > + if (ret < 0) { > + pr_err("clk_enable(clk_rx) failed: %d", ret); > + goto err_disable_clk_master_bus; > + } > } > > - ret = clk_enable(&eqos->clk_tx); > - if (ret < 0) { > - pr_err("clk_enable(clk_tx) failed: %d", ret); > - goto err_disable_clk_rx; > + if (clk_valid(&eqos->clk_tx)) { > + ret = clk_enable(&eqos->clk_tx); > + if (ret < 0) { > + pr_err("clk_enable(clk_tx) failed: %d", ret); > + goto err_disable_clk_rx; > + } > } > > if (clk_valid(&eqos->clk_ck)) { > @@ -639,9 +643,11 @@ static int eqos_start_clks_stm32(struct udevice *dev) > > #ifdef CONFIG_CLK > err_disable_clk_tx: > - clk_disable(&eqos->clk_tx); > + if (clk_valid(&eqos->clk_tx)) > + clk_disable(&eqos->clk_tx); > err_disable_clk_rx: > - clk_disable(&eqos->clk_rx); > + if (clk_valid(&eqos->clk_rx)) > + clk_disable(&eqos->clk_rx); > err_disable_clk_master_bus: > clk_disable(&eqos->clk_master_bus); > err: > @@ -679,8 +685,10 @@ static void eqos_stop_clks_stm32(struct udevice *dev) > > debug("%s(dev=%p):\n", __func__, dev); > > - clk_disable(&eqos->clk_tx); > - clk_disable(&eqos->clk_rx); > + if (clk_valid(&eqos->clk_tx)) > + clk_disable(&eqos->clk_tx); > + if (clk_valid(&eqos->clk_rx)) > + clk_disable(&eqos->clk_rx); > clk_disable(&eqos->clk_master_bus); > if (clk_valid(&eqos->clk_ck)) > clk_disable(&eqos->clk_ck); > @@ -1843,20 +1851,16 @@ static int eqos_probe_resources_stm32(struct udevice *dev) > ret = clk_get_by_name(dev, "stmmaceth", &eqos->clk_master_bus); > if (ret) { > pr_err("clk_get_by_name(master_bus) failed: %d", ret); > - goto err_probe; > + return ret; > } > > ret = clk_get_by_name(dev, "mac-clk-rx", &eqos->clk_rx); > - if (ret) { > - pr_err("clk_get_by_name(rx) failed: %d", ret); > - goto err_free_clk_master_bus; > - } > + if (ret) > + pr_warn("clk_get_by_name(rx) failed: %d", ret); > > ret = clk_get_by_name(dev, "mac-clk-tx", &eqos->clk_tx); > - if (ret) { > - pr_err("clk_get_by_name(tx) failed: %d", ret); > - goto err_free_clk_rx; > - } > + if (ret) > + pr_warn("clk_get_by_name(tx) failed: %d", ret); > > /* Get ETH_CLK clocks (optional) */ > ret = clk_get_by_name(dev, "eth-ck", &eqos->clk_ck); > @@ -1901,15 +1905,6 @@ static int eqos_probe_resources_stm32(struct udevice *dev) > > debug("%s: OK\n", __func__); > return 0; > - > -err_free_clk_rx: > - clk_free(&eqos->clk_rx); > -err_free_clk_master_bus: > - clk_free(&eqos->clk_master_bus); > -err_probe: > - > - debug("%s: returns %d\n", __func__, ret); > - return ret; > } > > static phy_interface_t eqos_get_interface_stm32(struct udevice *dev) > @@ -1991,8 +1986,10 @@ static int eqos_remove_resources_stm32(struct udevice *dev) > > debug("%s(dev=%p):\n", __func__, dev); > > - clk_free(&eqos->clk_tx); > - clk_free(&eqos->clk_rx); > + if (clk_valid(&eqos->clk_tx)) > + clk_free(&eqos->clk_tx); > + if (clk_valid(&eqos->clk_rx)) > + clk_free(&eqos->clk_rx); > clk_free(&eqos->clk_master_bus); > if (clk_valid(&eqos->clk_ck)) > clk_free(&eqos->clk_ck); Reviewed-by: Patrice Chotard <patrice.chotard at st.com> Thanks
Hi David, > From: David Wu <david.wu at rock-chips.com> > Sent: mardi 12 mai 2020 11:56 > > For others using, clk_rx and clk_tx may not be necessary, and their clock names > are different. > > Signed-off-by: David Wu <david.wu at rock-chips.com> > --- > > Changes in v2: > - Don't change the Rx and Tx clock names. (Patrice, Stephen) > > drivers/net/dwc_eth_qos.c | 61 +++++++++++++++++++-------------------- > 1 file changed, 29 insertions(+), 32 deletions(-) > > diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index > ae2167637f..bec9bf556b 100644 > --- a/drivers/net/dwc_eth_qos.c > +++ b/drivers/net/dwc_eth_qos.c > @@ -613,16 +613,20 @@ static int eqos_start_clks_stm32(struct udevice *dev) > goto err; > } > > - ret = clk_enable(&eqos->clk_rx); > - if (ret < 0) { > - pr_err("clk_enable(clk_rx) failed: %d", ret); > - goto err_disable_clk_master_bus; > + if (clk_valid(&eqos->clk_rx)) { > + ret = clk_enable(&eqos->clk_rx); > + if (ret < 0) { > + pr_err("clk_enable(clk_rx) failed: %d", ret); > + goto err_disable_clk_master_bus; > + } > } > > - ret = clk_enable(&eqos->clk_tx); > - if (ret < 0) { > - pr_err("clk_enable(clk_tx) failed: %d", ret); > - goto err_disable_clk_rx; > + if (clk_valid(&eqos->clk_tx)) { > + ret = clk_enable(&eqos->clk_tx); > + if (ret < 0) { > + pr_err("clk_enable(clk_tx) failed: %d", ret); > + goto err_disable_clk_rx; > + } > } > > if (clk_valid(&eqos->clk_ck)) { > @@ -639,9 +643,11 @@ static int eqos_start_clks_stm32(struct udevice *dev) > > #ifdef CONFIG_CLK > err_disable_clk_tx: > - clk_disable(&eqos->clk_tx); > + if (clk_valid(&eqos->clk_tx)) > + clk_disable(&eqos->clk_tx); > err_disable_clk_rx: > - clk_disable(&eqos->clk_rx); > + if (clk_valid(&eqos->clk_rx)) > + clk_disable(&eqos->clk_rx); > err_disable_clk_master_bus: > clk_disable(&eqos->clk_master_bus); > err: > @@ -679,8 +685,10 @@ static void eqos_stop_clks_stm32(struct udevice *dev) > > debug("%s(dev=%p):\n", __func__, dev); > > - clk_disable(&eqos->clk_tx); > - clk_disable(&eqos->clk_rx); > + if (clk_valid(&eqos->clk_tx)) > + clk_disable(&eqos->clk_tx); > + if (clk_valid(&eqos->clk_rx)) > + clk_disable(&eqos->clk_rx); > clk_disable(&eqos->clk_master_bus); > if (clk_valid(&eqos->clk_ck)) > clk_disable(&eqos->clk_ck); > @@ -1843,20 +1851,16 @@ static int eqos_probe_resources_stm32(struct udevice > *dev) > ret = clk_get_by_name(dev, "stmmaceth", &eqos->clk_master_bus); > if (ret) { > pr_err("clk_get_by_name(master_bus) failed: %d", ret); > - goto err_probe; > + return ret; > } > > ret = clk_get_by_name(dev, "mac-clk-rx", &eqos->clk_rx); > - if (ret) { > - pr_err("clk_get_by_name(rx) failed: %d", ret); > - goto err_free_clk_master_bus; > - } > + if (ret) > + pr_warn("clk_get_by_name(rx) failed: %d", ret); > > ret = clk_get_by_name(dev, "mac-clk-tx", &eqos->clk_tx); > - if (ret) { > - pr_err("clk_get_by_name(tx) failed: %d", ret); > - goto err_free_clk_rx; > - } > + if (ret) > + pr_warn("clk_get_by_name(tx) failed: %d", ret); > > /* Get ETH_CLK clocks (optional) */ > ret = clk_get_by_name(dev, "eth-ck", &eqos->clk_ck); @@ -1901,15 > +1905,6 @@ static int eqos_probe_resources_stm32(struct udevice *dev) > > debug("%s: OK\n", __func__); > return 0; > - > -err_free_clk_rx: > - clk_free(&eqos->clk_rx); > -err_free_clk_master_bus: > - clk_free(&eqos->clk_master_bus); > -err_probe: > - > - debug("%s: returns %d\n", __func__, ret); > - return ret; > } > > static phy_interface_t eqos_get_interface_stm32(struct udevice *dev) @@ -1991,8 > +1986,10 @@ static int eqos_remove_resources_stm32(struct udevice *dev) > > debug("%s(dev=%p):\n", __func__, dev); > > - clk_free(&eqos->clk_tx); > - clk_free(&eqos->clk_rx); > + if (clk_valid(&eqos->clk_tx)) > + clk_free(&eqos->clk_tx); > + if (clk_valid(&eqos->clk_rx)) > + clk_free(&eqos->clk_rx); > clk_free(&eqos->clk_master_bus); > if (clk_valid(&eqos->clk_ck)) > clk_free(&eqos->clk_ck); > -- > 2.19.1 > > These clock are mandatory for STM32 glue as explain in Linux binding Documentation/devicetree/bindings/net/stm32-dwmac.txt But I fact when when I check the code, I see perhaps an issue in the current U-Boot glue: we don't select the STM32 glue for the correct compatible, I think I will push static const struct udevice_id eqos_ids[] = { { .compatible = "nvidia,tegra186-eqos", .data = (ulong)&eqos_tegra186_config }, { - .compatible = "snps,dwmac-4.20a", + .compatible = "st,stm32mp1-dwmac", .data = (ulong)&eqos_stm32_config }, { .compatible = "fsl,imx-eqos", .data = (ulong)&eqos_imx_config }, { } }; Then you can manage your own glue for rockchip ETH for your compatible. Regards Patrick
On 5/13/20 3:17 PM, Patrick DELAUNAY wrote: > Hi David, > >> From: David Wu <david.wu at rock-chips.com> >> Sent: mardi 12 mai 2020 11:56 >> >> For others using, clk_rx and clk_tx may not be necessary, and their clock names >> are different. >> >> Signed-off-by: David Wu <david.wu at rock-chips.com> >> --- >> >> Changes in v2: >> - Don't change the Rx and Tx clock names. (Patrice, Stephen) >> >> drivers/net/dwc_eth_qos.c | 61 +++++++++++++++++++-------------------- >> 1 file changed, 29 insertions(+), 32 deletions(-) >> >> diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index >> ae2167637f..bec9bf556b 100644 >> --- a/drivers/net/dwc_eth_qos.c >> +++ b/drivers/net/dwc_eth_qos.c >> @@ -613,16 +613,20 @@ static int eqos_start_clks_stm32(struct udevice *dev) >> goto err; >> } >> >> - ret = clk_enable(&eqos->clk_rx); >> - if (ret < 0) { >> - pr_err("clk_enable(clk_rx) failed: %d", ret); >> - goto err_disable_clk_master_bus; >> + if (clk_valid(&eqos->clk_rx)) { >> + ret = clk_enable(&eqos->clk_rx); >> + if (ret < 0) { >> + pr_err("clk_enable(clk_rx) failed: %d", ret); >> + goto err_disable_clk_master_bus; >> + } >> } >> >> - ret = clk_enable(&eqos->clk_tx); >> - if (ret < 0) { >> - pr_err("clk_enable(clk_tx) failed: %d", ret); >> - goto err_disable_clk_rx; >> + if (clk_valid(&eqos->clk_tx)) { >> + ret = clk_enable(&eqos->clk_tx); >> + if (ret < 0) { >> + pr_err("clk_enable(clk_tx) failed: %d", ret); >> + goto err_disable_clk_rx; >> + } >> } >> >> if (clk_valid(&eqos->clk_ck)) { >> @@ -639,9 +643,11 @@ static int eqos_start_clks_stm32(struct udevice *dev) >> >> #ifdef CONFIG_CLK >> err_disable_clk_tx: >> - clk_disable(&eqos->clk_tx); >> + if (clk_valid(&eqos->clk_tx)) >> + clk_disable(&eqos->clk_tx); >> err_disable_clk_rx: >> - clk_disable(&eqos->clk_rx); >> + if (clk_valid(&eqos->clk_rx)) >> + clk_disable(&eqos->clk_rx); >> err_disable_clk_master_bus: >> clk_disable(&eqos->clk_master_bus); >> err: >> @@ -679,8 +685,10 @@ static void eqos_stop_clks_stm32(struct udevice *dev) >> >> debug("%s(dev=%p):\n", __func__, dev); >> >> - clk_disable(&eqos->clk_tx); >> - clk_disable(&eqos->clk_rx); >> + if (clk_valid(&eqos->clk_tx)) >> + clk_disable(&eqos->clk_tx); >> + if (clk_valid(&eqos->clk_rx)) >> + clk_disable(&eqos->clk_rx); >> clk_disable(&eqos->clk_master_bus); >> if (clk_valid(&eqos->clk_ck)) >> clk_disable(&eqos->clk_ck); >> @@ -1843,20 +1851,16 @@ static int eqos_probe_resources_stm32(struct udevice >> *dev) >> ret = clk_get_by_name(dev, "stmmaceth", &eqos->clk_master_bus); >> if (ret) { >> pr_err("clk_get_by_name(master_bus) failed: %d", ret); >> - goto err_probe; >> + return ret; >> } >> >> ret = clk_get_by_name(dev, "mac-clk-rx", &eqos->clk_rx); >> - if (ret) { >> - pr_err("clk_get_by_name(rx) failed: %d", ret); >> - goto err_free_clk_master_bus; >> - } >> + if (ret) >> + pr_warn("clk_get_by_name(rx) failed: %d", ret); >> >> ret = clk_get_by_name(dev, "mac-clk-tx", &eqos->clk_tx); >> - if (ret) { >> - pr_err("clk_get_by_name(tx) failed: %d", ret); >> - goto err_free_clk_rx; >> - } >> + if (ret) >> + pr_warn("clk_get_by_name(tx) failed: %d", ret); >> >> /* Get ETH_CLK clocks (optional) */ >> ret = clk_get_by_name(dev, "eth-ck", &eqos->clk_ck); @@ -1901,15 >> +1905,6 @@ static int eqos_probe_resources_stm32(struct udevice *dev) >> >> debug("%s: OK\n", __func__); >> return 0; >> - >> -err_free_clk_rx: >> - clk_free(&eqos->clk_rx); >> -err_free_clk_master_bus: >> - clk_free(&eqos->clk_master_bus); >> -err_probe: >> - >> - debug("%s: returns %d\n", __func__, ret); >> - return ret; >> } >> >> static phy_interface_t eqos_get_interface_stm32(struct udevice *dev) @@ -1991,8 >> +1986,10 @@ static int eqos_remove_resources_stm32(struct udevice *dev) >> >> debug("%s(dev=%p):\n", __func__, dev); >> >> - clk_free(&eqos->clk_tx); >> - clk_free(&eqos->clk_rx); >> + if (clk_valid(&eqos->clk_tx)) >> + clk_free(&eqos->clk_tx); >> + if (clk_valid(&eqos->clk_rx)) >> + clk_free(&eqos->clk_rx); >> clk_free(&eqos->clk_master_bus); >> if (clk_valid(&eqos->clk_ck)) >> clk_free(&eqos->clk_ck); >> -- >> 2.19.1 >> >> > > These clock are mandatory for STM32 glue as explain in Linux binding > Documentation/devicetree/bindings/net/stm32-dwmac.txt > > But I fact when when I check the code, I see perhaps an issue in the current U-Boot glue: > we don't select the STM32 glue for the correct compatible, I think I will push > > static const struct udevice_id eqos_ids[] = { > { > .compatible = "nvidia,tegra186-eqos", > .data = (ulong)&eqos_tegra186_config > }, > { > - .compatible = "snps,dwmac-4.20a", > + .compatible = "st,stm32mp1-dwmac", > .data = (ulong)&eqos_stm32_config > }, > { > .compatible = "fsl,imx-eqos", > .data = (ulong)&eqos_imx_config > }, > > { } > }; > > Then you can manage your own glue for rockchip ETH for your compatible. You might even want to drop the tegra support on ARM32 , thus save space by dropping useless code.
diff --git a/drivers/net/dwc_eth_qos.c b/drivers/net/dwc_eth_qos.c index ae2167637f..bec9bf556b 100644 --- a/drivers/net/dwc_eth_qos.c +++ b/drivers/net/dwc_eth_qos.c @@ -613,16 +613,20 @@ static int eqos_start_clks_stm32(struct udevice *dev) goto err; } - ret = clk_enable(&eqos->clk_rx); - if (ret < 0) { - pr_err("clk_enable(clk_rx) failed: %d", ret); - goto err_disable_clk_master_bus; + if (clk_valid(&eqos->clk_rx)) { + ret = clk_enable(&eqos->clk_rx); + if (ret < 0) { + pr_err("clk_enable(clk_rx) failed: %d", ret); + goto err_disable_clk_master_bus; + } } - ret = clk_enable(&eqos->clk_tx); - if (ret < 0) { - pr_err("clk_enable(clk_tx) failed: %d", ret); - goto err_disable_clk_rx; + if (clk_valid(&eqos->clk_tx)) { + ret = clk_enable(&eqos->clk_tx); + if (ret < 0) { + pr_err("clk_enable(clk_tx) failed: %d", ret); + goto err_disable_clk_rx; + } } if (clk_valid(&eqos->clk_ck)) { @@ -639,9 +643,11 @@ static int eqos_start_clks_stm32(struct udevice *dev) #ifdef CONFIG_CLK err_disable_clk_tx: - clk_disable(&eqos->clk_tx); + if (clk_valid(&eqos->clk_tx)) + clk_disable(&eqos->clk_tx); err_disable_clk_rx: - clk_disable(&eqos->clk_rx); + if (clk_valid(&eqos->clk_rx)) + clk_disable(&eqos->clk_rx); err_disable_clk_master_bus: clk_disable(&eqos->clk_master_bus); err: @@ -679,8 +685,10 @@ static void eqos_stop_clks_stm32(struct udevice *dev) debug("%s(dev=%p):\n", __func__, dev); - clk_disable(&eqos->clk_tx); - clk_disable(&eqos->clk_rx); + if (clk_valid(&eqos->clk_tx)) + clk_disable(&eqos->clk_tx); + if (clk_valid(&eqos->clk_rx)) + clk_disable(&eqos->clk_rx); clk_disable(&eqos->clk_master_bus); if (clk_valid(&eqos->clk_ck)) clk_disable(&eqos->clk_ck); @@ -1843,20 +1851,16 @@ static int eqos_probe_resources_stm32(struct udevice *dev) ret = clk_get_by_name(dev, "stmmaceth", &eqos->clk_master_bus); if (ret) { pr_err("clk_get_by_name(master_bus) failed: %d", ret); - goto err_probe; + return ret; } ret = clk_get_by_name(dev, "mac-clk-rx", &eqos->clk_rx); - if (ret) { - pr_err("clk_get_by_name(rx) failed: %d", ret); - goto err_free_clk_master_bus; - } + if (ret) + pr_warn("clk_get_by_name(rx) failed: %d", ret); ret = clk_get_by_name(dev, "mac-clk-tx", &eqos->clk_tx); - if (ret) { - pr_err("clk_get_by_name(tx) failed: %d", ret); - goto err_free_clk_rx; - } + if (ret) + pr_warn("clk_get_by_name(tx) failed: %d", ret); /* Get ETH_CLK clocks (optional) */ ret = clk_get_by_name(dev, "eth-ck", &eqos->clk_ck); @@ -1901,15 +1905,6 @@ static int eqos_probe_resources_stm32(struct udevice *dev) debug("%s: OK\n", __func__); return 0; - -err_free_clk_rx: - clk_free(&eqos->clk_rx); -err_free_clk_master_bus: - clk_free(&eqos->clk_master_bus); -err_probe: - - debug("%s: returns %d\n", __func__, ret); - return ret; } static phy_interface_t eqos_get_interface_stm32(struct udevice *dev) @@ -1991,8 +1986,10 @@ static int eqos_remove_resources_stm32(struct udevice *dev) debug("%s(dev=%p):\n", __func__, dev); - clk_free(&eqos->clk_tx); - clk_free(&eqos->clk_rx); + if (clk_valid(&eqos->clk_tx)) + clk_free(&eqos->clk_tx); + if (clk_valid(&eqos->clk_rx)) + clk_free(&eqos->clk_rx); clk_free(&eqos->clk_master_bus); if (clk_valid(&eqos->clk_ck)) clk_free(&eqos->clk_ck);
For others using, clk_rx and clk_tx may not be necessary, and their clock names are different. Signed-off-by: David Wu <david.wu at rock-chips.com> --- Changes in v2: - Don't change the Rx and Tx clock names. (Patrice, Stephen) drivers/net/dwc_eth_qos.c | 61 +++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 32 deletions(-)