On 4/17/25 7:11 PM, Fabio Estevam wrote:
From: Fabio Estevam <feste...@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.
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.
This part above ^ is fine.
This part below v is wrong .
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.
This is wrong, because this is NOT how the clock look up works at all.
The lookup works this way. Take for example:
drivers/clk/imx/clk-imx8mp.c
240 clk_dm(IMX8MP_ARM_PLL_REF_SEL, imx_clk_mux(dev,
"arm_pll_ref_sel", base + 0x84, 0, 2, pll_ref_sels,
ARRAY_SIZE(pll_ref_sels)));
pll_ref_sels:
21 static const char * const pll_ref_sels[] = { "osc_24m", "dummy",
"dummy", "dummy", };
the "osc_24m" is looked up in the clock-controller@30380000 clock-names
property:
dts/upstream/src/arm64/freescale/imx8mp.dtsi
745 clk: clock-controller@30380000 {
...
751 clocks = <&osc_32k>, <&osc_24m>, <&clk_ext1>, <&clk_ext2>,
752 <&clk_ext3>, <&clk_ext4>;
753 clock-names = "osc_32k", "osc_24m", ...
^^^^^^^
The offset of osc_24m in that clock-names property is 1 , so the phandle
in clocks property array at offset 1 is used to look up those clock:
745 clk: clock-controller@30380000 {
...
751 clocks = <&osc_32k>, <&osc_24m>,
^^^^^^^^
Which leads to this clock node in DT:
188 osc_24m: clock-osc-24m {
^^^^^^^^
189 compatible = "fixed-clock";
190 #clock-cells = <0>;
The "clock-output-names" is NEVER used for any clock look up.
...
I am fine with the first half of the description, but the second half is
wrong and this actually does not fix anything. Simply remove
clock-output-names from the osc_24m and the code will be broken again.
The clk_resolve_parent_clk() is meant to perform the aforementioned
resolution(), that is what "[PATCH v2 00/24] clk: Add
clk_resolve_parent_clk() and fix up iMX clock drivers" actually fixed.
It seems there is something still missing and clk_resolve_parent_clk()
does not do the resolution properly for some clock, but that is what
needs to be understood and fixed, not worked around this way.
