Re: [PATCH 2/3] clk: Introduce 'critical-clocks' property

[Stephen Boyd <sboyd@kernel.org>]

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 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?