* [PATCH 1/2] dt-bindings: mfd: rohm,bd71847-pmic: Document rohm,clock-output-is-critical property
@ 2021-10-20 8:49 Marek Vasut
2021-10-20 8:49 ` [PATCH 2/2] clk: bd718x7: Add " Marek Vasut
2021-10-20 10:14 ` [PATCH 1/2] dt-bindings: mfd: rohm,bd71847-pmic: Document " Vaittinen, Matti
0 siblings, 2 replies; 7+ messages in thread
From: Marek Vasut @ 2021-10-20 8:49 UTC (permalink / raw)
To: linux-clk
Cc: Marek Vasut, Matti Vaittinen, Michael Turquette, Rob Herring,
Stephen Boyd, devicetree, linux-power
Add the possibility to configure PMIC 32kHz output clock as CRITICAL,
so that they are never gated off. This is useful in case those clock
supply some vital clock net, which requires the clock to always run.
The iMX8M RTC XTAL input is one such example, if the clock are ever
gated off, the system locks up completely. The clock must be present
and enabled even if the RTC is unused.
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
---
Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml
index 5d531051a153..2497ade2bbd0 100644
--- a/Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml
+++ b/Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml
@@ -41,6 +41,11 @@ properties:
clock-output-names:
maxItems: 1
+ rohm,clock-output-is-critical:
+ description:
+ Never gate off C32K_OUT clock.
+ type: boolean
+
# The BD71847 abd BD71850 support two different HW states as reset target
# states. States are called as SNVS and READY. At READY state all the PMIC
# power outputs go down and OTP is reload. At the SNVS state all other logic
--
2.33.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/2] clk: bd718x7: Add rohm,clock-output-is-critical property
2021-10-20 8:49 [PATCH 1/2] dt-bindings: mfd: rohm,bd71847-pmic: Document rohm,clock-output-is-critical property Marek Vasut
@ 2021-10-20 8:49 ` Marek Vasut
2021-10-20 10:14 ` [PATCH 1/2] dt-bindings: mfd: rohm,bd71847-pmic: Document " Vaittinen, Matti
1 sibling, 0 replies; 7+ messages in thread
From: Marek Vasut @ 2021-10-20 8:49 UTC (permalink / raw)
To: linux-clk
Cc: Marek Vasut, Matti Vaittinen, Michael Turquette, Rob Herring,
Stephen Boyd, devicetree, linux-power
Add the possibility to configure PMIC 32kHz output clock as CRITICAL,
so that they are never gated off. This is useful in case those clock
supply some vital clock net, which requires the clock to always run.
The iMX8M RTC XTAL input is one such example, if the clock are ever
gated off, the system locks up completely. The clock must be present
and enabled even if the RTC is unused.
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
---
drivers/clk/clk-bd718x7.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/clk/clk-bd718x7.c b/drivers/clk/clk-bd718x7.c
index ac40b669d60b..a59bc57f13bc 100644
--- a/drivers/clk/clk-bd718x7.c
+++ b/drivers/clk/clk-bd718x7.c
@@ -125,6 +125,13 @@ static int bd71837_clk_probe(struct platform_device *pdev)
c->pdev = pdev;
c->hw.init = &init;
+ /*
+ * The clock supply vital clock net, e.g. SoC XTAL input,
+ * and the clock must not ever be turned off.
+ */
+ if (of_property_read_bool(parent->of_node, "rohm,clock-output-is-critical"))
+ init.flags |= CLK_IS_CRITICAL,
+
of_property_read_string_index(parent->of_node,
"clock-output-names", 0, &init.name);
--
2.33.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: rohm,bd71847-pmic: Document rohm,clock-output-is-critical property
2021-10-20 8:49 [PATCH 1/2] dt-bindings: mfd: rohm,bd71847-pmic: Document rohm,clock-output-is-critical property Marek Vasut
2021-10-20 8:49 ` [PATCH 2/2] clk: bd718x7: Add " Marek Vasut
@ 2021-10-20 10:14 ` Vaittinen, Matti
2021-10-20 11:06 ` Marek Vasut
1 sibling, 1 reply; 7+ messages in thread
From: Vaittinen, Matti @ 2021-10-20 10:14 UTC (permalink / raw)
To: Marek Vasut, linux-clk
Cc: Michael Turquette, Rob Herring, Stephen Boyd, devicetree, linux-power
Hi dee Ho Marek,
Thanks! I like the idea and appreciate the improvement.
On 10/20/21 11:49, Marek Vasut wrote:
> Add the possibility to configure PMIC 32kHz output clock as CRITICAL,
> so that they are never gated off. This is useful in case those clock
> supply some vital clock net, which requires the clock to always run.
>
> The iMX8M RTC XTAL input is one such example, if the clock are ever
> gated off, the system locks up completely. The clock must be present
> and enabled even if the RTC is unused.
>
> 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
> ---
> Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml b/Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml
> index 5d531051a153..2497ade2bbd0 100644
> --- a/Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml
> +++ b/Documentation/devicetree/bindings/mfd/rohm,bd71847-pmic.yaml
> @@ -41,6 +41,11 @@ properties:
> clock-output-names:
> maxItems: 1
>
> + rohm,clock-output-is-critical:
I wonder if this really is something specific to ROHM ICs? Do you think
this would warrant a generic, non vendor specific property? I am Ok with
the ROHM specific property too but it just seems to me this might not be
unique to ROHM IC(s).
By the way, the very same clk driver where you implemented the property
reading (patch 2/2) is used by few other ROHM PMICs. At least by
BD71837, BD71828, BD71815, BD9576 and BD9573. So the code change here
adds support for this property to all of those PMICs. I wonder if the
property should be mentioned in all of the binding docs... That could be
another argument for making this a generic property and describing it in
clk yaml ;)
Well, just my 10 Cents - I am ok with this change as you presented it
here if you don't think this should be generic one.
> + description:
> + Never gate off C32K_OUT clock.
> + type: boolean
> +
> # The BD71847 abd BD71850 support two different HW states as reset target
> # states. States are called as SNVS and READY. At READY state all the PMIC
> # power outputs go down and OTP is reload. At the SNVS state all other logic
>
Best Regards
Matti Vaittinen
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: rohm,bd71847-pmic: Document rohm,clock-output-is-critical property
2021-10-20 10:14 ` [PATCH 1/2] dt-bindings: mfd: rohm,bd71847-pmic: Document " Vaittinen, Matti
@ 2021-10-20 11:06 ` Marek Vasut
2021-10-28 21:24 ` Rob Herring
0 siblings, 1 reply; 7+ messages in thread
From: Marek Vasut @ 2021-10-20 11:06 UTC (permalink / raw)
To: Vaittinen, Matti, linux-clk
Cc: Michael Turquette, Rob Herring, Stephen Boyd, devicetree, linux-power
On 10/20/21 12:14 PM, Vaittinen, Matti wrote:
[...]
> I wonder if this really is something specific to ROHM ICs? Do you think
> this would warrant a generic, non vendor specific property? I am Ok with
> the ROHM specific property too but it just seems to me this might not be
> unique to ROHM IC(s).
>
> By the way, the very same clk driver where you implemented the property
> reading (patch 2/2) is used by few other ROHM PMICs. At least by
> BD71837, BD71828, BD71815, BD9576 and BD9573. So the code change here
> adds support for this property to all of those PMICs. I wonder if the
> property should be mentioned in all of the binding docs... That could be
> another argument for making this a generic property and describing it in
> clk yaml ;)
>
> Well, just my 10 Cents - I am ok with this change as you presented it
> here if you don't think this should be generic one.
I think we need something like gpio-hog, except for clock. Some clk-hog
maybe ? That would be useful not only here, but also for things where
some output generates clock for random stuff which cannot be described
in the DT for whatever reason (like e.g. the SoC is used as a substitute
for CPLD XTAL and the CPLD isn't connected to the SoC in any other way).
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: rohm,bd71847-pmic: Document rohm,clock-output-is-critical property
2021-10-20 11:06 ` Marek Vasut
@ 2021-10-28 21:24 ` Rob Herring
2021-10-29 7:43 ` Vaittinen, Matti
2021-11-08 15:01 ` Marek Vasut
0 siblings, 2 replies; 7+ messages in thread
From: Rob Herring @ 2021-10-28 21:24 UTC (permalink / raw)
To: Marek Vasut
Cc: Vaittinen, Matti, linux-clk, Michael Turquette, Stephen Boyd,
devicetree, linux-power
On Wed, Oct 20, 2021 at 01:06:13PM +0200, Marek Vasut wrote:
> On 10/20/21 12:14 PM, Vaittinen, Matti wrote:
> [...]
>
> > I wonder if this really is something specific to ROHM ICs? Do you think
> > this would warrant a generic, non vendor specific property? I am Ok with
> > the ROHM specific property too but it just seems to me this might not be
> > unique to ROHM IC(s).
I imagine we debated the need for a DT property when critical clocks was
added to the kernel.
> > By the way, the very same clk driver where you implemented the property
> > reading (patch 2/2) is used by few other ROHM PMICs. At least by
> > BD71837, BD71828, BD71815, BD9576 and BD9573. So the code change here
> > adds support for this property to all of those PMICs. I wonder if the
> > property should be mentioned in all of the binding docs... That could be
> > another argument for making this a generic property and describing it in
> > clk yaml ;)
> >
> > Well, just my 10 Cents - I am ok with this change as you presented it
> > here if you don't think this should be generic one.
>
> I think we need something like gpio-hog, except for clock. Some clk-hog
> maybe ? That would be useful not only here, but also for things where some
> output generates clock for random stuff which cannot be described in the DT
> for whatever reason (like e.g. the SoC is used as a substitute for CPLD XTAL
> and the CPLD isn't connected to the SoC in any other way).
The justification given in this patch was for an SoC input which should
get described so that the clock is handled and kept enabled properly.
The CPLD case would be more interesting, but is there an actual need or
just a possible case?
You could use the 'protected-clocks' property here. Maybe that's a bit
overloaded between can't access and don't turn off. But what it means is
really up the clock controller.
Rob
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: rohm,bd71847-pmic: Document rohm,clock-output-is-critical property
2021-10-28 21:24 ` Rob Herring
@ 2021-10-29 7:43 ` Vaittinen, Matti
2021-11-08 15:01 ` Marek Vasut
1 sibling, 0 replies; 7+ messages in thread
From: Vaittinen, Matti @ 2021-10-29 7:43 UTC (permalink / raw)
To: Rob Herring, Marek Vasut
Cc: linux-clk, Michael Turquette, Stephen Boyd, devicetree, linux-power
On 10/29/21 00:24, Rob Herring wrote:
> On Wed, Oct 20, 2021 at 01:06:13PM +0200, Marek Vasut wrote:
>> On 10/20/21 12:14 PM, Vaittinen, Matti wrote:
>> [...]
>>
>>> I wonder if this really is something specific to ROHM ICs? Do you think
>>> this would warrant a generic, non vendor specific property? I am Ok with
>>> the ROHM specific property too but it just seems to me this might not be
>>> unique to ROHM IC(s).
>
> I imagine we debated the need for a DT property when critical clocks was
> added to the kernel.
Sorry. I guess I've missed this. Maybe this was done back when I spent
my days tinkering with strictly proprietary systems - or then I have
just missed it.
>>> By the way, the very same clk driver where you implemented the property
>>> reading (patch 2/2) is used by few other ROHM PMICs. At least by
>>> BD71837, BD71828, BD71815, BD9576 and BD9573. So the code change here
>>> adds support for this property to all of those PMICs. I wonder if the
>>> property should be mentioned in all of the binding docs... That could be
>>> another argument for making this a generic property and describing it in
>>> clk yaml ;)
>>>
>>> Well, just my 10 Cents - I am ok with this change as you presented it
>>> here if you don't think this should be generic one.
>>
>> I think we need something like gpio-hog, except for clock. Some clk-hog
>> maybe ? That would be useful not only here, but also for things where some
>> output generates clock for random stuff which cannot be described in the DT
>> for whatever reason (like e.g. the SoC is used as a substitute for CPLD XTAL
>> and the CPLD isn't connected to the SoC in any other way).
>
> The justification given in this patch was for an SoC input which should
> get described so that the clock is handled and kept enabled properly.
>
> The CPLD case would be more interesting, but is there an actual need or
> just a possible case?
>
> You could use the 'protected-clocks' property here. Maybe that's a bit
> overloaded between can't access and don't turn off. But what it means is
> really up the clock controller.
I think I have seen some patch series which are aimed to providing
common implementation for the 'protected-clocks'. It seems to me the
intended 'protected-clocks' handling is simply not registering these
clocks. I don't see this handling in-tree yet though and I did not find
any discussion as to why the generic support has not been merged.
So, if the 'protected-clocks' is to be supported by the driver, then I
wonder if the handling should be 'ensure enabled and don't register to
clock core' or plain 'don't register to clock core'?
Best Regards
--Matti Vaittinen
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] dt-bindings: mfd: rohm,bd71847-pmic: Document rohm,clock-output-is-critical property
2021-10-28 21:24 ` Rob Herring
2021-10-29 7:43 ` Vaittinen, Matti
@ 2021-11-08 15:01 ` Marek Vasut
1 sibling, 0 replies; 7+ messages in thread
From: Marek Vasut @ 2021-11-08 15:01 UTC (permalink / raw)
To: Rob Herring
Cc: Vaittinen, Matti, linux-clk, Michael Turquette, Stephen Boyd,
devicetree, linux-power
On 10/28/21 11:24 PM, Rob Herring wrote:
> On Wed, Oct 20, 2021 at 01:06:13PM +0200, Marek Vasut wrote:
>> On 10/20/21 12:14 PM, Vaittinen, Matti wrote:
>> [...]
>>
>>> I wonder if this really is something specific to ROHM ICs? Do you think
>>> this would warrant a generic, non vendor specific property? I am Ok with
>>> the ROHM specific property too but it just seems to me this might not be
>>> unique to ROHM IC(s).
>
> I imagine we debated the need for a DT property when critical clocks was
> added to the kernel.
Have you got some reference to this debate ?
I think something like clk-hog , similar to gpio-hog , would be useful,
since we could also configure the critical clock frequency in DT.
>>> By the way, the very same clk driver where you implemented the property
>>> reading (patch 2/2) is used by few other ROHM PMICs. At least by
>>> BD71837, BD71828, BD71815, BD9576 and BD9573. So the code change here
>>> adds support for this property to all of those PMICs. I wonder if the
>>> property should be mentioned in all of the binding docs... That could be
>>> another argument for making this a generic property and describing it in
>>> clk yaml ;)
>>>
>>> Well, just my 10 Cents - I am ok with this change as you presented it
>>> here if you don't think this should be generic one.
>>
>> I think we need something like gpio-hog, except for clock. Some clk-hog
>> maybe ? That would be useful not only here, but also for things where some
>> output generates clock for random stuff which cannot be described in the DT
>> for whatever reason (like e.g. the SoC is used as a substitute for CPLD XTAL
>> and the CPLD isn't connected to the SoC in any other way).
>
> The justification given in this patch was for an SoC input which should
> get described so that the clock is handled and kept enabled properly.
This is the case I had here, yes. Although I've been running into
similar requirements repeatedly for almost a decade, I'm surprised
nobody implemented something like this yet.
> The CPLD case would be more interesting, but is there an actual need or
> just a possible case?
This is an iMX53 board from 2012 or so, where they figured they don't
need an XTal for the CPLD because the SoC has this OSC_OUT and that can
be used to supply clock to the CPLD at just the frequency they need. So
the SoC is a clock source for the CPLD, and that's all there is to it.
So far I hacked it in the clock driver to keep the clock running at
specific rate, but that hack has been a thorn in my side for long enough.
> You could use the 'protected-clocks' property here. Maybe that's a bit
> overloaded between can't access and don't turn off. But what it means is
> really up the clock controller.
This does not seem to describe what is needed here, protected-clock are
used to tell OS not to touch certain clock because they are protected by
e.g. firmware access restriction, it does not say anything about whether
the clock are critical. Also, it seems to be a non-generic property only
for some qualcomm clock driver.
commit 48d7f160b10711f014bf07b574c73452646c9fdd
[...]
dt-bindings: clk: Introduce 'protected-clocks' property
Add a generic clk property for clks which are not intended to be used by
the OS due to security restrictions put in place by firmware. For
example, on some Qualcomm firmwares reading or writing certain clk
registers causes the entire system to reboot, but on other firmwares
reading and writing those same registers is required to make devices
like QSPI work. Rather than adding one-off properties each time a new
set of clks appears to be protected, let's add a generic clk property to
describe any set of clks that shouldn't be touched by the OS. This way
we never need to register the clks or use them in certain firmware
configurations.
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-11-08 15:01 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-20 8:49 [PATCH 1/2] dt-bindings: mfd: rohm,bd71847-pmic: Document rohm,clock-output-is-critical property Marek Vasut
2021-10-20 8:49 ` [PATCH 2/2] clk: bd718x7: Add " Marek Vasut
2021-10-20 10:14 ` [PATCH 1/2] dt-bindings: mfd: rohm,bd71847-pmic: Document " Vaittinen, Matti
2021-10-20 11:06 ` Marek Vasut
2021-10-28 21:24 ` Rob Herring
2021-10-29 7:43 ` Vaittinen, Matti
2021-11-08 15:01 ` Marek Vasut
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.