Update to clock warning paragraph in device tree?

[HinTak]

@pelwell can you clarify and perhaps update your commit from over 4 years ago, on May 18 2016, on clocks in dynamic overlays? b91bcb7#diff-16b28acb48c9229c4a306d4f533f9ccd

I am currently looking at a "regression" : I am finding that dynamic overlays in clocks actually works with 4.19.x on 4-months-old Raspbian (current raspbian is already 5.4 based), but fails with 5.4.x on current 64-bit Ubuntu . The failing call is devm_get_clk_from_child. It returns a valid clk struct on 4.19.x but fails with 5.4.

Unfortunately tracing further is difficult as most of its guts are static / hidden. But have managed to get as far as most of of_parse_clkspec being successful, until it gets to of_clk_get_from_provider. This seems to suggest a registration problem alluded in your commit.

Anyway, since it works for 4.19 (for a specific overlay with adding a new fixed-clock), I am wondering if the 4-year-old statement could be updated a bit at least. Also would appreciate if you can point me in the right direction to find help on this "regression". There were piles of clk-related changes in 5.1 and 5.2 underneath the devm_clk_* calls.

[pelwell]

They linked change says:

• Adding clocks under the /clocks node at run-time doesn't cause a new clock provider to be registered, so devm_clk_get will fail for a clock created in an overlay.

A more accurate and complete warning with the benefit of a bit more experience would say something like:

• Only Device Tree nodes at the top level of the tree and children of a bus node will be probed. For nodes added at run-time there is the further limitation that the bus must register for notifications of the addition and removal of children. However, there are exceptions that break this rule and cause confusion: the kernel explicitly scans the entire tree for some device types - clocks and interrupt controller being the two main ones - in order to (for clocks) initialise them early and/or (for interrupt controllers) in a particular order. This search mechanism only happens during booting and so doesn't work for nodes added by an overlay at run-time. It is therefore recommended for overlays to place fixed-clock nodes in the root of the tree unless it is guaranteed that the overlay will not be used at run-time.

If that makes sense to you I'll get it added to the documentation.

[HinTak]

Yes, it makes a lot of sense! I read other devices' device tree declarations and wondered why others put their "fixed-clocks" in the bottom, instead of under the "/clocks" branch! I thought it might be just untidiness. That explains it. I wonder why it did work on 4.19 though. Anyway, I'll move the clock node and see if that fixes the problem. Yes, please add that paragraph to the documentation!

[pelwell]

See #1673.

[HinTak]

@pelwell thanks a lot! I have also just tested relocating the fixed-clock node to the bottom on Ubuntu. Yes, that fixes the "regression". The driver code now finds a valid clock.

I guess there might be some undocumented and subtle hacks in the kernel to do a more thorough scan, that got removed during the clk re-org in early v5.x ?

[HinTak]

Thanks for the merge!