On 11/9/21 00:42, Marek Vasut wrote: > NOTE: This is an RFC patch showing how this mechanism might be workable. > > 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. Hm. It slightly bugs me to parse the clock properties form DT in many places. I was thinking that perhaps we could parse all the clk properties at __clk_register() and store them to be later used when of_clk_provider is added. After having a chat with Marek I was kindly explained the DT node might not always be present at __clk_register() - phase. I don't easily see a better way so I'd better to just learn to live with the bugging feeling :) > > 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 do also like the idea of being able to replace the get-callback - and a driver specific callback with a generic op. Can the clock-indices be somehow used for generic one? Perhaps allowing also adding a driver specific callback for cases where generic one can't do the job? (I don't have any concrete idea/code how to do that right now though). But what ever it is worth, I do like the overall idea. It sounds right to me. > Thoughts (on the overall design, not code quality or patch splitting) ? > > Signed-off-by: Marek Vasut <marex@xxxxxxx> > Cc: Matti Vaittinen <matti.vaittinen@xxxxxxxxxxxxxxxxx> > Cc: Michael Turquette <mturquette@xxxxxxxxxxxx> > Cc: Rob Herring <robh+dt@xxxxxxxxxx> > Cc: Stephen Boyd <sboyd@xxxxxxxxxx> > Cc: devicetree@xxxxxxxxxxxxxxx > Cc: linux-power@xxxxxxxxxxxxxxxxx > To: linux-clk@xxxxxxxxxxxxxxx > --- > .../bindings/clock/clock-bindings.txt | 16 ++++++++++++ > drivers/clk/clk-bd718x7.c | 15 +++++++++++ > drivers/clk/clk.c | 25 +++++++++++++++++++ > include/linux/clk-provider.h | 2 ++ > 4 files changed, 58 insertions(+) > Best Regards --Matti Vaittinen
