RE: [RESEND PATCH v2 04/11] net: dwc_eth_qos: Make clk_rx and clk_tx optional

[Patrick DELAUNAY]

Hi David,

> From: David Wu <david...@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...@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