Re: [RESEND][PATCH v2 1/2] dt-bindings: clk: Introduce 'critical-clocks' property

From: Rob Herring <robh@kernel.org>
To: Marek Vasut <marex@denx.de>
Cc: linux-clk@vger.kernel.org,
	Michael Turquette <mturquette@baylibre.com>,
	Stephen Boyd <sboyd@kernel.org>,
	devicetree@vger.kernel.org
Subject: Re: [RESEND][PATCH v2 1/2] dt-bindings: clk: Introduce 'critical-clocks' property
Date: Tue, 3 May 2022 16:16:29 -0500	[thread overview]
Message-ID: <YnGbrbtla8YHmKM1@robh.at.kernel.org> (raw)
In-Reply-To: <20220428110107.149524-1-marex@denx.de>

On Thu, Apr 28, 2022 at 01:01:06PM +0200, Marek Vasut wrote:
> Some platforms require select clock to be always running, e.g. because
> those clock supply vital devices which are not otherwise attached to
> the system and thus do not have a matching DT node and clock consumer.
> 
> An example is a system where the SoC serves as a crystal oscillator
> replacement for a programmable logic device. The "critical-clocks"
> property of a clock controller allows listing clock which must never
> be turned off.
> 
> Clock listed in the "critical-clocks" property may have other consumers
> in DT, listing the clock in "critical-clocks" only assures those clock
> are never turned off, and none of these optional additional consumers
> can turn the clock off either.
> 
> The implementation is modeled after "protected-clocks".
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Michael Turquette <mturquette@baylibre.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Stephen Boyd <sboyd@kernel.org>
> Cc: devicetree@vger.kernel.org
> To: linux-clk@vger.kernel.org
> ---
> V2: Update the commit message to clarify the behavior
> ---
>  .../devicetree/bindings/clock/clock-bindings.txt | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)

As mentioned on IRC, I'm okay with the property, but want Stephen's 
opinion on it. It also needs to be a schema in dtschema with the rest of 
the clock binding. I've submitted a patch to redirect this file to the 
schema.

> 
> diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt
> index f2ea53832ac6..975230e603d1 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

require some clocks

> +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-clocks 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>;
> +   };
> +
>  ==Protected clocks==
>  
>  Some platforms or firmwares may not fully expose all the clocks to the OS, such
> -- 
> 2.35.1
> 
> 