On 2/17/22 23:23, Stephen Boyd wrote:
Quoting Marek Vasut (2022-02-15 00:44:11)
Some platforms require clock to be always running, e.g. because those clock
supply devices which are not otherwise attached to the system. One example
is a system where the SoC serves as a crystal oscillator replacement for a
programmable logic device. The critical-clock property of a clock controller
allows listing clock which must never be turned off.
The implementation here is similar to "protected-clock", except protected
clock property is currently driver specific. This patch attempts to make
a generic implementation of "critical-clock" instead.
Unlike "assigned-clocks", the "critical-clock" must be parsed much earlier
in __clk_register() to assign CLK_IS_CRITICAL flag to clk_init_data .flags
field. The parsing code obviously need to be cleaned up and factor out into
separate function.
The new match_clkspec() callback is used to determine whether struct clk_hw
that is currently being registered matches the clock specifier in the DT
"critical-clock" property, and if so, then the CLK_IS_CRITICAL is added to
these newly registered clock. This callback is currently driver specific,
although I suspect a common and/or generic version of the callback could
be added. Also, this new callback could possibly be used to replace (*get)
argument of of_clk_add_hw_provider() later on too.
I don't see any mention of of_clk_detect_critical() here. We don't want
to enshrine the critical clk flag in DT. There was a bunch of discussion
about this on the mailing list years ago and the end result was this
instantly deprecated function to set the flag based on a DT property.
That thread isn't mentioned here either.
I wasn't aware of clock-critical DT prop, but it seems deprecated and
not generic enough anyway.
I see that there isn't any more 'clock-critical' in the kernel's dts so
I wonder if we would be able to get rid of that function or at least
hollow it out and see if anyone complains. Either way, what is the
actual problem trying to be solved? If the crystal oscillator isn't used
anywhere in the kernel why are we registering it with the clk framework?
The problem is the other way around -- the SoC clock IPs often have a
couple of general purpose clock routed to various SoC IO pins, those
clock can be used for any purpose, and those are already registered with
kernel clock framework. Some devices save on BoM and use those general
purpose clock to supply clock networks which are otherwise not
interacting with the kernel, like some CPLD for example. Since from the
kernel point of view, those clock are unused, the kernel can turn those
clock OFF and that will make the entire device fail.
So this critical-clocks property permits marking clock which must not
ever be turned OFF accordingly.
[...]
