[PATCH] clk: fixed-rate: Use "clock-output-names" to name fixed clocks

[Adam Ford]

On Thu, Apr 17, 2025 at 7:32 AM Fabio Estevam <festevam at gmail.com> wrote:
>
> From: Fabio Estevam <festevam at denx.de>
>
> Currently, fixed-rate clocks in U-Boot are named based on their devicetree
> node names. For example, given the following node:
>
> osc_24m: clock-osc-24m {
>         compatible = "fixed-clock";
>         #clock-cells = <0>;
>         clock-frequency = <24000000>;
>         clock-output-names = "osc_24m";
> };
>
> U-Boot registers the clock as "clock-osc-24m", derived from the node name,
> ignoring the clock-output-names property.
>
> This differs from Linux, which uses the clock-output-names property when
> assigning clock names. As a result, clock consumers expecting names
> like "osc_24m" (as defined in the property) may fail to resolve clocks
> correctly in U-Boot.

You read my mind!  I just sent out an e-mail to the other thread with
this very suggestion.

>
> Update the fixed-rate clock driver to check for the clock-output-names
> property and use it as the clock name if present. If not, the fallback
> remains the node name. This makes U-Boot behavior consistent with Linux.

Agreed.

>
> One concrete impact is on i.MX8MP, where USB clock lookup failed following
> commit b4734c9c333b ("clk: imx: Convert clock-osc-* back to osc_*").
>
> With this change applied, fixed-clocks are correctly registered with their
> expected names:
>
> u-boot=> clk dump
>  Rate               Usecnt      Name
> ------------------------------------------
>  32768                0        |-- osc_32k
>  24000000             5        |-- osc_24m
>
> This change restores i.MX8MP USB functionality by ensuring the proper clock
> names are used.

Tested-by: Adam Ford <aford173 at gmail.com> #imx8mp-beacon
>
> Fixes: b4734c9c333b ("clk: imx: Convert clock-osc-* back to osc_*")
> Reported-by: Francesco Dolcini <francesco.dolcini at toradex.com>
> Signed-off-by: Fabio Estevam <festevam at denx.de>
> ---
>  drivers/clk/clk_fixed_rate.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/drivers/clk/clk_fixed_rate.c b/drivers/clk/clk_fixed_rate.c
> index d1da05cc18a5..7b89f9c80e95 100644
> --- a/drivers/clk/clk_fixed_rate.c
> +++ b/drivers/clk/clk_fixed_rate.c
> @@ -35,6 +35,8 @@ void clk_fixed_rate_ofdata_to_plat_(struct udevice *dev,
>                                     struct clk_fixed_rate *plat)
>  {
>         struct clk *clk = &plat->clk;
> +       const char *clk_name;
> +
>         if (CONFIG_IS_ENABLED(OF_REAL))
>                 plat->fixed_rate = dev_read_u32_default(dev, "clock-frequency",
>                                                         0);
> @@ -45,6 +47,16 @@ void clk_fixed_rate_ofdata_to_plat_(struct udevice *dev,
>
>         clk->dev = dev;
>         clk->enable_count = 0;
> +
> +       /*
> +        * If the "clock-output-names" property is present, use it
> +        * as the clock name, like it is done in Linux.
> +        * Otherwise, the original behavior will be preserved: the fixed clock
> +        * name will be its node name.
> +        *
> +        */
> +       if (!(dev_read_string_index(dev, "clock-output-names", 0, &clk_name)))
> +               dev->name = clk_name;
>  }
>
>  static ulong clk_fixed_rate_raw_get_rate(struct clk *clk)
> --
> 2.34.1
>