Re: [PATCH] [RFC] dt-bindings: clk: Introduce 'critical-clocks' property

[Marek Vasut <marex@denx.de>]

On 11/29/21 21:18, Rob Herring wrote:
> On Mon, Nov 08, 2021 at 11:42:42PM +0100, 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.
>>
>> 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.
>>
>> Thoughts (on the overall design, not code quality or patch splitting) ?
>>
>> Signed-off-by: Marek Vasut <marex@denx.de>
>> Cc: Matti Vaittinen <matti.vaittinen@fi.rohmeurope.com>
>> Cc: Michael Turquette <mturquette@baylibre.com>
>> Cc: Rob Herring <robh+dt@kernel.org>
>> Cc: Stephen Boyd <sboyd@kernel.org>
>> Cc: devicetree@vger.kernel.org
>> Cc: linux-power@fi.rohmeurope.com
>> To: linux-clk@vger.kernel.org
>> ---
>>   .../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(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt
>> index f2ea53832ac63..d9a783c35c5a1 100644
>> --- a/Documentation/devicetree/bindings/clock/clock-bindings.txt
>> +++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
>> @@ -169,6 +169,22 @@ a shared clock is forbidden.
>>   Configuration of common clocks, which affect multiple consumer devices can
>>   be similarly specified in the clock provider node.
>>   
>> +==Critical clocks==
>> +
>> +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.
>> +
>> +   clock-controller@a000f000 {
>> +        compatible = "vendor,clk95;
>> +        reg = <0xa000f000 0x1000>
>> +        #clocks-cells = <1>;
>> +        ...
>> +        critical-clocks = <UART3_CLK>, <SPI5_CLK>;
> 
> This will need a schema definition in dtschema.
> 
> Otherwise, the concept is fine for me.

I sent out proper patches, also for the schemas.