[PATCH 0/4] clk: Provide support for always-on clocks

[PATCH 0/4] clk: Provide support for always-on clocks

[Lee Jones]

Some hardware contains bunches of clocks which must never be
turned off.  If drivers a) fail to obtain a reference to any
of these or b) give up a previously obtained reference
during suspend, the common clk framework will attempt to
disable them and a platform can fail irrecoverably as a
result.  Usually the only way to recover from these failures
is to reboot.

To avoid either of these two scenarios from catastrophically
disabling an otherwise perfectly healthy running system,
clocks can be identified as always-on using this property
from inside a clocksource's node.  The CLK_IGNORE_UNUSED
flag will be applied to each clock instance named in this
property, thus preventing them from being shut down by the
framework.

Lee Jones (4):
  ARM: sti: stih407-family: Supply defines for CLOCKGEN A0
  ARM: sti: stih410-clocks: Identify critical clocks as always-on
  clk: Provide always-on clock support
  clk: dt: Introduce binding for always-on clock support

 .../devicetree/bindings/clock/clock-bindings.txt   | 34 ++++++++++++++++++++++
 arch/arm/boot/dts/stih410-clock.dtsi               | 10 +++++++
 drivers/clk/clk.c                                  | 11 +++++++
 include/dt-bindings/clock/stih407-clks.h           |  4 +++
 4 files changed, 59 insertions(+)

-- 
1.9.1

[PATCH 1/4] ARM: sti: stih407-family: Supply defines for CLOCKGEN A0

[Lee Jones]

There are 2 LMI clocks generated by CLOCKGEN A0.  We wish to control
them individually and need to use these indexes to do so.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 include/dt-bindings/clock/stih407-clks.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/dt-bindings/clock/stih407-clks.h b/include/dt-bindings/clock/stih407-clks.h
index 7af2b71..082edd9 100644
--- a/include/dt-bindings/clock/stih407-clks.h
+++ b/include/dt-bindings/clock/stih407-clks.h
@@ -5,6 +5,10 @@
 #ifndef _DT_BINDINGS_CLK_STIH407
 #define _DT_BINDINGS_CLK_STIH407
 
+/* CLOCKGEN A0 */
+#define CLK_IC_LMI0		0
+#define CLK_IC_LMI1		1
+
 /* CLOCKGEN C0 */
 #define CLK_ICN_GPU		0
 #define CLK_FDMA		1
-- 
1.9.1

[PATCH 2/4] ARM: sti: stih410-clocks: Identify critical clocks as always-on

[Lee Jones]

Lots of platforms contain clocks which if turned off would prove fatal.
The only way to recover is to restart the board(s).  This driver takes
references to clocks which are required to be always-on in order to
prevent the common clk framework from trying to turn them off during
the clk_disabled_unused() procedure.

In this patch we are identifying clocks, which if gated would render
the STiH410 development board unserviceable.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 arch/arm/boot/dts/stih410-clock.dtsi | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/stih410-clock.dtsi b/arch/arm/boot/dts/stih410-clock.dtsi
index 6b5803a..7b257c2 100644
--- a/arch/arm/boot/dts/stih410-clock.dtsi
+++ b/arch/arm/boot/dts/stih410-clock.dtsi
@@ -103,6 +103,7 @@
 				clocks = <&clk_sysin>;
 
 				clock-output-names = "clk-s-a0-pll-ofd-0";
+				clock-always-on = "clk-s-a0-pll-ofd-0";
 			};
 
 			clk_s_a0_flexgen: clk-s-a0-flexgen {
@@ -115,6 +116,8 @@
 
 				clock-output-names = "clk-ic-lmi0",
 						     "clk-ic-lmi1";
+
+				clock-always-on = "clk-ic-lmi0";
 			};
 		};
 
@@ -142,6 +145,7 @@
 				clocks = <&clk_sysin>;
 
 				clock-output-names = "clk-s-c0-pll0-odf-0";
+				clock-always-on = "clk-s-c0-pll0-odf-0";
 			};
 
 			clk_s_c0_pll1: clk-s-c0-pll1 {
@@ -204,6 +208,12 @@
 						     "clk-clust-hades",
 						     "clk-hwpe-hades",
 						     "clk-fc-hades";
+
+				clock-always-on = "clk-icn-cpu",
+						  "clk-tx-icn-dmu",
+						  "clk-ext2fa9",
+						  "clk-icn-lmi",
+						  "clk-icn-sbc";
 			};
 		};
 
-- 
1.9.1

Re: [STLinux Kernel] [PATCH 2/4] ARM: sti: stih410-clocks: Identify critical clocks as always-on

[Peter Griffin]

Hi Lee,

On Fri, 27 Feb 2015, Lee Jones wrote:

> Lots of platforms contain clocks which if turned off would prove fatal.
> The only way to recover is to restart the board(s).  This driver takes
> references to clocks which are required to be always-on in order to
> prevent the common clk framework from trying to turn them off during
> the clk_disabled_unused() procedure.
> In this patch we are identifying clocks, which if gated would render
> the STiH410 development board unserviceable.

Can you also add <clk_s_c0_flexgen CLK_PROC_STFE> to the list of always
on clocks for stih407 and stih410?

This clock is slightly unusual in that the clock doesn't need to be on
at boot, but once enabled by the kernel, can't be disabled without causing
the system to hang.

So slightly different from the others, but I would prefer to keep balanced
enable/disable calls in the tsin driver, in case this (bug?) goes away
on future SoC's.

regards,

Peter.

[PATCH 3/4] clk: Provide always-on clock support

[Lee Jones]

Lots of platforms contain clocks which if turned off would prove fatal.
The only way to recover from these catastrophic failures is to restart
the board(s).  Now, when a clock is registered with the framework it is
compared against a list of provided always-on clock names which must be
kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
flag, which will prevent the common clk framework from attempting to
gate it during the clk_disable_unused() procedure.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/clk/clk.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 61c3fc5..3aab75e 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2856,6 +2856,9 @@ int of_clk_add_provider(struct device_node *np,
 			void *data)
 {
 	struct of_clk_provider *cp;
+	struct property *prop;
+	struct clk *clk;
+	const char *clkname;
 	int ret;
 
 	cp = kzalloc(sizeof(struct of_clk_provider), GFP_KERNEL);
@@ -2875,6 +2878,14 @@ int of_clk_add_provider(struct device_node *np,
 	if (ret < 0)
 		of_clk_del_provider(np);
 
+	of_property_for_each_string(np, "clock-always-on", prop, clkname) {
+		clk = __clk_lookup(clkname);
+		if (!clk)
+			continue;
+
+		clk->core->flags |= CLK_IGNORE_UNUSED;
+	}
+
 	return ret;
 }
 EXPORT_SYMBOL_GPL(of_clk_add_provider);
-- 
1.9.1

Re: [PATCH 3/4] clk: Provide always-on clock support

[Lee Jones]

On Fri, 27 Feb 2015, Lee Jones wrote:

> Lots of platforms contain clocks which if turned off would prove fatal.
> The only way to recover from these catastrophic failures is to restart
> the board(s).  Now, when a clock is registered with the framework it is
> compared against a list of provided always-on clock names which must be
> kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
> flag, which will prevent the common clk framework from attempting to
> gate it during the clk_disable_unused() procedure.
> 
> Signed-off-by: Lee Jones <lee.jones@linaro.org>
> ---
>  drivers/clk/clk.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 61c3fc5..3aab75e 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2856,6 +2856,9 @@ int of_clk_add_provider(struct device_node *np,
>  			void *data)
>  {
>  	struct of_clk_provider *cp;
> +	struct property *prop;
> +	struct clk *clk;
> +	const char *clkname;
>  	int ret;
>  
>  	cp = kzalloc(sizeof(struct of_clk_provider), GFP_KERNEL);
> @@ -2875,6 +2878,14 @@ int of_clk_add_provider(struct device_node *np,
>  	if (ret < 0)
>  		of_clk_del_provider(np);
>  
> +	of_property_for_each_string(np, "clock-always-on", prop, clkname) {
> +		clk = __clk_lookup(clkname);
> +		if (!clk)
> +			continue;
> +
> +		clk->core->flags |= CLK_IGNORE_UNUSED;

Might save a few cycles by breaking here.

> +	}
> +
>  	return ret;
>  }
>  EXPORT_SYMBOL_GPL(of_clk_add_provider);

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [STLinux Kernel] [PATCH 3/4] clk: Provide always-on clock support

[Maxime Coquelin]

Hi Lee,

On 02/27/2015 10:14 PM, Lee Jones wrote:
> Lots of platforms contain clocks which if turned off would prove fatal.
> The only way to recover from these catastrophic failures is to restart
> the board(s).  Now, when a clock is registered with the framework it is
> compared against a list of provided always-on clock names which must be
> kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
> flag, which will prevent the common clk framework from attempting to
> gate it during the clk_disable_unused() procedure.

Please correct me if I'm wrong, but your patch does not fix the issue 
you had initially.
Let's take an example:
A clock is critical for the system, and should never be gated, so you 
add the CLK_IGNORE_UNUSED
flag so that it is not disabled by clk_disable_unused() procedure.
The same clock is also used by other IPs, for example spi 0 instance.
When starting a spi transfer, clk_enable() is called on this clock, so 
its usecount becomes 1.
Once transfer done, clk_disable() is called, usecount becomes 0 and the 
clock gets disabled: system freeze.

BR,
Maxime

Re: [STLinux Kernel] [PATCH 3/4] clk: Provide always-on clock support

[Lee Jones]

On Sat, 28 Feb 2015, Maxime Coquelin wrote:

> Hi Lee,
> 
> On 02/27/2015 10:14 PM, Lee Jones wrote:
> >Lots of platforms contain clocks which if turned off would prove fatal.
> >The only way to recover from these catastrophic failures is to restart
> >the board(s).  Now, when a clock is registered with the framework it is
> >compared against a list of provided always-on clock names which must be
> >kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
> >flag, which will prevent the common clk framework from attempting to
> >gate it during the clk_disable_unused() procedure.
> 
> Please correct me if I'm wrong, but your patch does not fix the
> issue you had initially.
> Let's take an example:
> A clock is critical for the system, and should never be gated, so
> you add the CLK_IGNORE_UNUSED
> flag so that it is not disabled by clk_disable_unused() procedure.
> The same clock is also used by other IPs, for example spi 0 instance.
> When starting a spi transfer, clk_enable() is called on this clock,
> so its usecount becomes 1.
> Once transfer done, clk_disable() is called, usecount becomes 0 and
> the clock gets disabled: system freeze.

You're right.  I also need to extend clk_core_disable() to take notice
of CLK_IGNORE_UNUSED.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [STLinux Kernel] [PATCH 3/4] clk: Provide always-on clock support

[Michael Turquette]

Quoting Lee Jones (2015-03-02 00:16:06)
> On Sat, 28 Feb 2015, Maxime Coquelin wrote:
> 
> > Hi Lee,
> > 
> > On 02/27/2015 10:14 PM, Lee Jones wrote:
> > >Lots of platforms contain clocks which if turned off would prove fatal.
> > >The only way to recover from these catastrophic failures is to restart
> > >the board(s).  Now, when a clock is registered with the framework it is
> > >compared against a list of provided always-on clock names which must be
> > >kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
> > >flag, which will prevent the common clk framework from attempting to
> > >gate it during the clk_disable_unused() procedure.
> > 
> > Please correct me if I'm wrong, but your patch does not fix the
> > issue you had initially.
> > Let's take an example:
> > A clock is critical for the system, and should never be gated, so
> > you add the CLK_IGNORE_UNUSED
> > flag so that it is not disabled by clk_disable_unused() procedure.
> > The same clock is also used by other IPs, for example spi 0 instance.
> > When starting a spi transfer, clk_enable() is called on this clock,
> > so its usecount becomes 1.
> > Once transfer done, clk_disable() is called, usecount becomes 0 and
> > the clock gets disabled: system freeze.
> 
> You're right.  I also need to extend clk_core_disable() to take notice
> of CLK_IGNORE_UNUSED.

Hi Lee,

I'd like to combine elements of your V3 and this series (V4?). Firstly
let's stay away from modifying the semantics around CLK_IGNORE_UNUSED.
There was discussion around adding a new flag which I'd like to avoid if
possible.

If we're leaving clocks enabled then it would be best to reflect that
reality with the prepare/enable refcounting in the core. The best way to
do this is with clk_prepare_enable(my_alwon_clk). Drivers already do
this today by hacking calls to clk_prepare_enable into their driver's
probe function, and we can support it more explicitly in DT.

Since we already have assigned-clock-rate and assigned-clock-parent in
drivers/clk/clk-conf.c I propose that we add a third property here,
assigned-clock-alwon. I don't really care about the name, so feel free
to change it, so long as your suggestion is Measurably Better.

This property can only be added to clock providers (not consumers),
because if we have a clock consumer then that consumer should just call
clk_prepare_enable. See the pseudo-code below:



diff --git a/drivers/clk/clk-conf.c b/drivers/clk/clk-conf.c
index 48a65b2..19dce89 100644
--- a/drivers/clk/clk-conf.c
+++ b/drivers/clk/clk-conf.c
@@ -115,6 +115,14 @@ static int __set_clk_rates(struct device_node *node, bool clk_supplier)
 	return 0;
 }
 
+static int __set_clk_alwon(struct device_node *node, bool clk_supplier)
+{
+	if (!clk_supplier)
+		return -EINVAL;
+
+	clk_prepare_enable(...);
+}
+
 /**
  * of_clk_set_defaults() - parse and set assigned clocks configuration
  * @node: device node to apply clock settings for
@@ -138,6 +146,10 @@ int of_clk_set_defaults(struct device_node *node, bool clk_supplier)
 	if (rc < 0)
 		return rc;
 
-	return __set_clk_rates(node, clk_supplier);
+	rc = __set_clk_rates(node, clk_supplier);
+	if (rc < 0)
+		return rc;
+
+	return __set_clk_alwon(node, clk_supplier);
 }
 EXPORT_SYMBOL_GPL(of_clk_set_defaults);



Amazing, I know. Is there any aspect of this approach which does not
solve your use case?

We should mark this new property (assigned-clk-alwon) as unstable since
it can (and likely will) grow into more stuff in the future (e.g.
Stephen Boyd mentioned a "hand-off" which tells the core to keep clocks
enabled until the correct driver claims them).

Regards,
Mike

Re: [PATCH 3/4] clk: Provide always-on clock support

[Jassi Brar]

On 28 February 2015 at 02:44, Lee Jones <lee.jones@linaro.org> wrote:
> Lots of platforms contain clocks which if turned off would prove fatal.
> The only way to recover from these catastrophic failures is to restart
> the board(s).  Now, when a clock is registered with the framework it is
> compared against a list of provided always-on clock names which must be
> kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
> flag, which will prevent the common clk framework from attempting to
> gate it during the clk_disable_unused() procedure.
>
If a clock is critical on a certain board, it could be got+enabled
during early boot so there is always a user.
To be able to do that from DT, maybe add a new, say, CLK_ALWAYS_ON
flag could be made to initialize the clock with one phantom user
already. Or just reuse the CLK_IGNORE_UNUSED?

-Jassi

Re: [PATCH 3/4] clk: Provide always-on clock support

[Lee Jones]

On Sat, 28 Feb 2015, Jassi Brar wrote:

> On 28 February 2015 at 02:44, Lee Jones <lee.jones@linaro.org> wrote:
> > Lots of platforms contain clocks which if turned off would prove fatal.
> > The only way to recover from these catastrophic failures is to restart
> > the board(s).  Now, when a clock is registered with the framework it is
> > compared against a list of provided always-on clock names which must be
> > kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
> > flag, which will prevent the common clk framework from attempting to
> > gate it during the clk_disable_unused() procedure.
> >
> If a clock is critical on a certain board, it could be got+enabled
> during early boot so there is always a user.

I tried this.  There was push-back from the DT maintainers.

  http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/324417.html

> To be able to do that from DT, maybe add a new, say, CLK_ALWAYS_ON
> flag could be made to initialize the clock with one phantom user
> already. Or just reuse the CLK_IGNORE_UNUSED?

How is that different to what this set is doing?

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 3/4] clk: Provide always-on clock support

[Jassi Brar]

On Mon, Mar 2, 2015 at 2:06 PM, Lee Jones <lee.jones@linaro.org> wrote:
> On Sat, 28 Feb 2015, Jassi Brar wrote:
>
>> On 28 February 2015 at 02:44, Lee Jones <lee.jones@linaro.org> wrote:
>> > Lots of platforms contain clocks which if turned off would prove fatal.
>> > The only way to recover from these catastrophic failures is to restart
>> > the board(s).  Now, when a clock is registered with the framework it is
>> > compared against a list of provided always-on clock names which must be
>> > kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
>> > flag, which will prevent the common clk framework from attempting to
>> > gate it during the clk_disable_unused() procedure.
>> >
>> If a clock is critical on a certain board, it could be got+enabled
>> during early boot so there is always a user.
>
> I tried this.  There was push-back from the DT maintainers.
>
>   http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/324417.html
>
Thanks, I wasn't aware of the history.

>> To be able to do that from DT, maybe add a new, say, CLK_ALWAYS_ON
>> flag could be made to initialize the clock with one phantom user
>> already. Or just reuse the CLK_IGNORE_UNUSED?
>
> How is that different to what this set is doing?
>
The phantom user - that's there but none can see it.

How about?

+       of_property_for_each_string(np, "clock-always-on", prop, clkname) {
+               clk = __clk_lookup(clkname);
+               if (!clk)
+                       continue;
+
+               clk->core->enable_count = 1;
+               clk->core->prepare_count = 1;
+       }

-jassi

Re: [PATCH 3/4] clk: Provide always-on clock support

[Lee Jones]

On Mon, 02 Mar 2015, Jassi Brar wrote:

> On Mon, Mar 2, 2015 at 2:06 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > On Sat, 28 Feb 2015, Jassi Brar wrote:
> >
> >> On 28 February 2015 at 02:44, Lee Jones <lee.jones@linaro.org> wrote:
> >> > Lots of platforms contain clocks which if turned off would prove fatal.
> >> > The only way to recover from these catastrophic failures is to restart
> >> > the board(s).  Now, when a clock is registered with the framework it is
> >> > compared against a list of provided always-on clock names which must be
> >> > kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
> >> > flag, which will prevent the common clk framework from attempting to
> >> > gate it during the clk_disable_unused() procedure.
> >> >
> >> If a clock is critical on a certain board, it could be got+enabled
> >> during early boot so there is always a user.
> >
> > I tried this.  There was push-back from the DT maintainers.
> >
> >   http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/324417.html
> >
> Thanks, I wasn't aware of the history.
> 
> >> To be able to do that from DT, maybe add a new, say, CLK_ALWAYS_ON
> >> flag could be made to initialize the clock with one phantom user
> >> already. Or just reuse the CLK_IGNORE_UNUSED?
> >
> > How is that different to what this set is doing?
> >
> The phantom user - that's there but none can see it.
> 
> How about?
> 
> +       of_property_for_each_string(np, "clock-always-on", prop, clkname) {
> +               clk = __clk_lookup(clkname);
> +               if (!clk)
> +                       continue;
> +
> +               clk->core->enable_count = 1;
> +               clk->core->prepare_count = 1;
> +       }

This is only fractionally different from the current implementation. 

I believe the current way it slightly nicer, as we don't have to fake
the user count.  This solution is saying "one of the drivers is still
consuming this clock", instead, in the original implementation we're
saying "we know there are no consumers of this clock, but keep it on
anyway due to [insert reason here]".

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [STLinux Kernel] [PATCH 3/4] clk: Provide always-on clock support

[Maxime Coquelin]

On 03/02/2015 11:18 AM, Lee Jones wrote:
> On Mon, 02 Mar 2015, Jassi Brar wrote:
>
>> On Mon, Mar 2, 2015 at 2:06 PM, Lee Jones <lee.jones@linaro.org> wrote:
>>> On Sat, 28 Feb 2015, Jassi Brar wrote:
>>>
>>>> On 28 February 2015 at 02:44, Lee Jones <lee.jones@linaro.org> wrote:
>>>>> Lots of platforms contain clocks which if turned off would prove fatal.
>>>>> The only way to recover from these catastrophic failures is to restart
>>>>> the board(s).  Now, when a clock is registered with the framework it is
>>>>> compared against a list of provided always-on clock names which must be
>>>>> kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
>>>>> flag, which will prevent the common clk framework from attempting to
>>>>> gate it during the clk_disable_unused() procedure.
>>>>>
>>>> If a clock is critical on a certain board, it could be got+enabled
>>>> during early boot so there is always a user.
>>> I tried this.  There was push-back from the DT maintainers.
>>>
>>>    http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/324417.html
>>>
>> Thanks, I wasn't aware of the history.
>>
>>>> To be able to do that from DT, maybe add a new, say, CLK_ALWAYS_ON
>>>> flag could be made to initialize the clock with one phantom user
>>>> already. Or just reuse the CLK_IGNORE_UNUSED?
>>> How is that different to what this set is doing?
>>>
>> The phantom user - that's there but none can see it.
>>
>> How about?
>>
>> +       of_property_for_each_string(np, "clock-always-on", prop, clkname) {
>> +               clk = __clk_lookup(clkname);
>> +               if (!clk)
>> +                       continue;
>> +
>> +               clk->core->enable_count = 1;
>> +               clk->core->prepare_count = 1;
>> +       }
> This is only fractionally different from the current implementation.
>
> I believe the current way it slightly nicer, as we don't have to fake
> the user count.  This solution is saying "one of the drivers is still
> consuming this clock", instead, in the original implementation we're
> saying "we know there are no consumers of this clock, but keep it on
> anyway due to [insert reason here]".
>
So maybe introducing a new "CLK_DISABLE_NEVER" flag will be more
explicit than hacking around "CLK_IGNORE_UNUSED" one?

BR,
Maxime

Re: [STLinux Kernel] [PATCH 3/4] clk: Provide always-on clock support

[Lee Jones]

On Mon, 02 Mar 2015, Maxime Coquelin wrote:
> On 03/02/2015 11:18 AM, Lee Jones wrote:
> >On Mon, 02 Mar 2015, Jassi Brar wrote:
> >
> >>On Mon, Mar 2, 2015 at 2:06 PM, Lee Jones <lee.jones@linaro.org> wrote:
> >>>On Sat, 28 Feb 2015, Jassi Brar wrote:
> >>>
> >>>>On 28 February 2015 at 02:44, Lee Jones <lee.jones@linaro.org> wrote:
> >>>>>Lots of platforms contain clocks which if turned off would prove fatal.
> >>>>>The only way to recover from these catastrophic failures is to restart
> >>>>>the board(s).  Now, when a clock is registered with the framework it is
> >>>>>compared against a list of provided always-on clock names which must be
> >>>>>kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
> >>>>>flag, which will prevent the common clk framework from attempting to
> >>>>>gate it during the clk_disable_unused() procedure.
> >>>>>
> >>>>If a clock is critical on a certain board, it could be got+enabled
> >>>>during early boot so there is always a user.
> >>>I tried this.  There was push-back from the DT maintainers.
> >>>
> >>>   http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/324417.html
> >>>
> >>Thanks, I wasn't aware of the history.
> >>
> >>>>To be able to do that from DT, maybe add a new, say, CLK_ALWAYS_ON
> >>>>flag could be made to initialize the clock with one phantom user
> >>>>already. Or just reuse the CLK_IGNORE_UNUSED?
> >>>How is that different to what this set is doing?
> >>>
> >>The phantom user - that's there but none can see it.
> >>
> >>How about?
> >>
> >>+       of_property_for_each_string(np, "clock-always-on", prop, clkname) {
> >>+               clk = __clk_lookup(clkname);
> >>+               if (!clk)
> >>+                       continue;
> >>+
> >>+               clk->core->enable_count = 1;
> >>+               clk->core->prepare_count = 1;
> >>+       }
> >This is only fractionally different from the current implementation.
> >
> >I believe the current way it slightly nicer, as we don't have to fake
> >the user count.  This solution is saying "one of the drivers is still
> >consuming this clock", instead, in the original implementation we're
> >saying "we know there are no consumers of this clock, but keep it on
> >anyway due to [insert reason here]".
> >
> So maybe introducing a new "CLK_DISABLE_NEVER" flag will be more
> explicit than hacking around "CLK_IGNORE_UNUSED" one?

I am okay with that if Mike is; however, personally I think
CLK_IGNORE_UNUSED is pretty descriptive.  If clocks are unused
(reference count reaches zero) the framework will disable the clock --
fair enough.  So in order to prevent that from happening in cases
where these clocks are used by entities other than device drivers
(perhaps these devices are even invisible to Linux) we have to tell
the framework to "ignore the fact that this clock _appears_ to be
unused".  Hence CLK_IGNORE_UNUSED.

I'm struggling to think of a solid reason for inventing new flags when
we have one that is perfectly suitable/descriptive.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 3/4] clk: Provide always-on clock support

[Jassi Brar]

On 2 March 2015 at 15:48, Lee Jones <lee.jones@linaro.org> wrote:
> On Mon, 02 Mar 2015, Jassi Brar wrote:
>
>> On Mon, Mar 2, 2015 at 2:06 PM, Lee Jones <lee.jones@linaro.org> wrote:
>> > On Sat, 28 Feb 2015, Jassi Brar wrote:
>> >
>> >> On 28 February 2015 at 02:44, Lee Jones <lee.jones@linaro.org> wrote:
>> >> > Lots of platforms contain clocks which if turned off would prove fatal.
>> >> > The only way to recover from these catastrophic failures is to restart
>> >> > the board(s).  Now, when a clock is registered with the framework it is
>> >> > compared against a list of provided always-on clock names which must be
>> >> > kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
>> >> > flag, which will prevent the common clk framework from attempting to
>> >> > gate it during the clk_disable_unused() procedure.
>> >> >
>> >> If a clock is critical on a certain board, it could be got+enabled
>> >> during early boot so there is always a user.
>> >
>> > I tried this.  There was push-back from the DT maintainers.
>> >
>> >   http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/324417.html
>> >
>> Thanks, I wasn't aware of the history.
>>
>> >> To be able to do that from DT, maybe add a new, say, CLK_ALWAYS_ON
>> >> flag could be made to initialize the clock with one phantom user
>> >> already. Or just reuse the CLK_IGNORE_UNUSED?
>> >
>> > How is that different to what this set is doing?
>> >
>> The phantom user - that's there but none can see it.
>>
>> How about?
>>
>> +       of_property_for_each_string(np, "clock-always-on", prop, clkname) {
>> +               clk = __clk_lookup(clkname);
>> +               if (!clk)
>> +                       continue;
>> +
>> +               clk->core->enable_count = 1;
>> +               clk->core->prepare_count = 1;
>> +       }
>
> This is only fractionally different from the current implementation.
>
> I believe the current way it slightly nicer, as we don't have to fake
> the user count.
>
Well... the user is indeed there, isn't it? It's just not known to
Linux. So 'fake' isn't most applicable here.
Otherwise you might have to stub out some existing and future
functions for CLK_IGNORE_UNUSED. And how do we explain to userspace
which would see power drawn but no user of the clock?

Anways, I am OK either way.

Cheers!
Jassi

Re: [PATCH 3/4] clk: Provide always-on clock support

[Lee Jones]

On Mon, 02 Mar 2015, Jassi Brar wrote:

> On 2 March 2015 at 15:48, Lee Jones <lee.jones@linaro.org> wrote:
> > On Mon, 02 Mar 2015, Jassi Brar wrote:
> >
> >> On Mon, Mar 2, 2015 at 2:06 PM, Lee Jones <lee.jones@linaro.org> wrote:
> >> > On Sat, 28 Feb 2015, Jassi Brar wrote:
> >> >
> >> >> On 28 February 2015 at 02:44, Lee Jones <lee.jones@linaro.org> wrote:
> >> >> > Lots of platforms contain clocks which if turned off would prove fatal.
> >> >> > The only way to recover from these catastrophic failures is to restart
> >> >> > the board(s).  Now, when a clock is registered with the framework it is
> >> >> > compared against a list of provided always-on clock names which must be
> >> >> > kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
> >> >> > flag, which will prevent the common clk framework from attempting to
> >> >> > gate it during the clk_disable_unused() procedure.
> >> >> >
> >> >> If a clock is critical on a certain board, it could be got+enabled
> >> >> during early boot so there is always a user.
> >> >
> >> > I tried this.  There was push-back from the DT maintainers.
> >> >
> >> >   http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/324417.html
> >> >
> >> Thanks, I wasn't aware of the history.
> >>
> >> >> To be able to do that from DT, maybe add a new, say, CLK_ALWAYS_ON
> >> >> flag could be made to initialize the clock with one phantom user
> >> >> already. Or just reuse the CLK_IGNORE_UNUSED?
> >> >
> >> > How is that different to what this set is doing?
> >> >
> >> The phantom user - that's there but none can see it.
> >>
> >> How about?
> >>
> >> +       of_property_for_each_string(np, "clock-always-on", prop, clkname) {
> >> +               clk = __clk_lookup(clkname);
> >> +               if (!clk)
> >> +                       continue;
> >> +
> >> +               clk->core->enable_count = 1;
> >> +               clk->core->prepare_count = 1;
> >> +       }
> >
> > This is only fractionally different from the current implementation.
> >
> > I believe the current way it slightly nicer, as we don't have to fake
> > the user count.
> >
> Well... the user is indeed there, isn't it? It's just not known to
> Linux. So 'fake' isn't most applicable here.
> Otherwise you might have to stub out some existing and future
> functions for CLK_IGNORE_UNUSED. And how do we explain to userspace
> which would see power drawn but no user of the clock?

I would hope that the user would know that some communication channels
(interconnects in our case) would have to stay alive.  When everything
is turned off, the platform is considered powered down.  Most people
who are going to care about this stuff know that platforms consume
power when suspended.

I should think the user would be even more confused if the reference
count was >0, but there was no information regarding who is consuming
it.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [PATCH 3/4] clk: Provide always-on clock support

[Michael Turquette]

Quoting Jassi Brar (2015-03-02 02:28:44)
> On 2 March 2015 at 15:48, Lee Jones <lee.jones@linaro.org> wrote:
> > On Mon, 02 Mar 2015, Jassi Brar wrote:
> >
> >> On Mon, Mar 2, 2015 at 2:06 PM, Lee Jones <lee.jones@linaro.org> wrote:
> >> > On Sat, 28 Feb 2015, Jassi Brar wrote:
> >> >
> >> >> On 28 February 2015 at 02:44, Lee Jones <lee.jones@linaro.org> wrote:
> >> >> > Lots of platforms contain clocks which if turned off would prove fatal.
> >> >> > The only way to recover from these catastrophic failures is to restart
> >> >> > the board(s).  Now, when a clock is registered with the framework it is
> >> >> > compared against a list of provided always-on clock names which must be
> >> >> > kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
> >> >> > flag, which will prevent the common clk framework from attempting to
> >> >> > gate it during the clk_disable_unused() procedure.
> >> >> >
> >> >> If a clock is critical on a certain board, it could be got+enabled
> >> >> during early boot so there is always a user.
> >> >
> >> > I tried this.  There was push-back from the DT maintainers.
> >> >
> >> >   http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/324417.html
> >> >
> >> Thanks, I wasn't aware of the history.
> >>
> >> >> To be able to do that from DT, maybe add a new, say, CLK_ALWAYS_ON
> >> >> flag could be made to initialize the clock with one phantom user
> >> >> already. Or just reuse the CLK_IGNORE_UNUSED?
> >> >
> >> > How is that different to what this set is doing?
> >> >
> >> The phantom user - that's there but none can see it.
> >>
> >> How about?
> >>
> >> +       of_property_for_each_string(np, "clock-always-on", prop, clkname) {
> >> +               clk = __clk_lookup(clkname);
> >> +               if (!clk)
> >> +                       continue;
> >> +
> >> +               clk->core->enable_count = 1;
> >> +               clk->core->prepare_count = 1;
> >> +       }
> >
> > This is only fractionally different from the current implementation.
> >
> > I believe the current way it slightly nicer, as we don't have to fake
> > the user count.
> >
> Well... the user is indeed there, isn't it? It's just not known to
> Linux. So 'fake' isn't most applicable here.
> Otherwise you might have to stub out some existing and future
> functions for CLK_IGNORE_UNUSED. And how do we explain to userspace
> which would see power drawn but no user of the clock?

Jassi,

This is broken. What if the parent of this clock has
{enable,prepare}_count of zero? The way we propagate these refcounts up
the tree would fall over.

Regards,
Mike

> 
> Anways, I am OK either way.
> 
> Cheers!
> Jassi

Re: [PATCH 3/4] clk: Provide always-on clock support

[Jassi Brar]

On Wed, Apr 1, 2015 at 7:12 AM, Michael Turquette <mturquette@linaro.org> wrote:
> Quoting Jassi Brar (2015-03-02 02:28:44)
>> On 2 March 2015 at 15:48, Lee Jones <lee.jones@linaro.org> wrote:
>> > On Mon, 02 Mar 2015, Jassi Brar wrote:
>> >
>> >> On Mon, Mar 2, 2015 at 2:06 PM, Lee Jones <lee.jones@linaro.org> wrote:
>> >> > On Sat, 28 Feb 2015, Jassi Brar wrote:
>> >> >
>> >> >> On 28 February 2015 at 02:44, Lee Jones <lee.jones@linaro.org> wrote:
>> >> >> > Lots of platforms contain clocks which if turned off would prove fatal.
>> >> >> > The only way to recover from these catastrophic failures is to restart
>> >> >> > the board(s).  Now, when a clock is registered with the framework it is
>> >> >> > compared against a list of provided always-on clock names which must be
>> >> >> > kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
>> >> >> > flag, which will prevent the common clk framework from attempting to
>> >> >> > gate it during the clk_disable_unused() procedure.
>> >> >> >
>> >> >> If a clock is critical on a certain board, it could be got+enabled
>> >> >> during early boot so there is always a user.
>> >> >
>> >> > I tried this.  There was push-back from the DT maintainers.
>> >> >
>> >> >   http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/324417.html
>> >> >
>> >> Thanks, I wasn't aware of the history.
>> >>
>> >> >> To be able to do that from DT, maybe add a new, say, CLK_ALWAYS_ON
>> >> >> flag could be made to initialize the clock with one phantom user
>> >> >> already. Or just reuse the CLK_IGNORE_UNUSED?
>> >> >
>> >> > How is that different to what this set is doing?
>> >> >
>> >> The phantom user - that's there but none can see it.
>> >>
>> >> How about?
>> >>
>> >> +       of_property_for_each_string(np, "clock-always-on", prop, clkname) {
>> >> +               clk = __clk_lookup(clkname);
>> >> +               if (!clk)
>> >> +                       continue;
>> >> +
>> >> +               clk->core->enable_count = 1;
>> >> +               clk->core->prepare_count = 1;
>> >> +       }
>> >
>> > This is only fractionally different from the current implementation.
>> >
>> > I believe the current way it slightly nicer, as we don't have to fake
>> > the user count.
>> >
>> Well... the user is indeed there, isn't it? It's just not known to
>> Linux. So 'fake' isn't most applicable here.
>> Otherwise you might have to stub out some existing and future
>> functions for CLK_IGNORE_UNUSED. And how do we explain to userspace
>> which would see power drawn but no user of the clock?
>
> Jassi,
>
> This is broken. What if the parent of this clock has
> {enable,prepare}_count of zero? The way we propagate these refcounts up
> the tree would fall over.
>
Yeah it needs to be done at higher level,
- clk->core->enable_count = 1;
- clk->core->prepare_count = 1;
+ clk_prepare_enable(clk);

cheers!

Re: [PATCH 3/4] clk: Provide always-on clock support

[Lee Jones]

On Thu, 02 Apr 2015, Jassi Brar wrote:

> On Wed, Apr 1, 2015 at 7:12 AM, Michael Turquette <mturquette@linaro.org> wrote:
> > Quoting Jassi Brar (2015-03-02 02:28:44)
> >> On 2 March 2015 at 15:48, Lee Jones <lee.jones@linaro.org> wrote:
> >> > On Mon, 02 Mar 2015, Jassi Brar wrote:
> >> >
> >> >> On Mon, Mar 2, 2015 at 2:06 PM, Lee Jones <lee.jones@linaro.org> wrote:
> >> >> > On Sat, 28 Feb 2015, Jassi Brar wrote:
> >> >> >
> >> >> >> On 28 February 2015 at 02:44, Lee Jones <lee.jones@linaro.org> wrote:
> >> >> >> > Lots of platforms contain clocks which if turned off would prove fatal.
> >> >> >> > The only way to recover from these catastrophic failures is to restart
> >> >> >> > the board(s).  Now, when a clock is registered with the framework it is
> >> >> >> > compared against a list of provided always-on clock names which must be
> >> >> >> > kept ungated.  If it matches, we enable the existing CLK_IGNORE_UNUSED
> >> >> >> > flag, which will prevent the common clk framework from attempting to
> >> >> >> > gate it during the clk_disable_unused() procedure.
> >> >> >> >
> >> >> >> If a clock is critical on a certain board, it could be got+enabled
> >> >> >> during early boot so there is always a user.
> >> >> >
> >> >> > I tried this.  There was push-back from the DT maintainers.
> >> >> >
> >> >> >   http://lists.infradead.org/pipermail/linux-arm-kernel/2015-February/324417.html
> >> >> >
> >> >> Thanks, I wasn't aware of the history.
> >> >>
> >> >> >> To be able to do that from DT, maybe add a new, say, CLK_ALWAYS_ON
> >> >> >> flag could be made to initialize the clock with one phantom user
> >> >> >> already. Or just reuse the CLK_IGNORE_UNUSED?
> >> >> >
> >> >> > How is that different to what this set is doing?
> >> >> >
> >> >> The phantom user - that's there but none can see it.
> >> >>
> >> >> How about?
> >> >>
> >> >> +       of_property_for_each_string(np, "clock-always-on", prop, clkname) {
> >> >> +               clk = __clk_lookup(clkname);
> >> >> +               if (!clk)
> >> >> +                       continue;
> >> >> +
> >> >> +               clk->core->enable_count = 1;
> >> >> +               clk->core->prepare_count = 1;
> >> >> +       }
> >> >
> >> > This is only fractionally different from the current implementation.
> >> >
> >> > I believe the current way it slightly nicer, as we don't have to fake
> >> > the user count.
> >> >
> >> Well... the user is indeed there, isn't it? It's just not known to
> >> Linux. So 'fake' isn't most applicable here.
> >> Otherwise you might have to stub out some existing and future
> >> functions for CLK_IGNORE_UNUSED. And how do we explain to userspace
> >> which would see power drawn but no user of the clock?
> >
> > Jassi,
> >
> > This is broken. What if the parent of this clock has
> > {enable,prepare}_count of zero? The way we propagate these refcounts up
> > the tree would fall over.
> >
> Yeah it needs to be done at higher level,
> - clk->core->enable_count = 1;
> - clk->core->prepare_count = 1;
> + clk_prepare_enable(clk);

FYI: https://lkml.org/lkml/2015/4/1/267

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 4/4] clk: dt: Introduce binding for always-on clock support

[Lee Jones]

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 .../devicetree/bindings/clock/clock-bindings.txt   | 34 ++++++++++++++++++++++
 1 file changed, 34 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/clock-bindings.txt b/Documentation/devicetree/bindings/clock/clock-bindings.txt
index 06fc6d5..ad17121 100644
--- a/Documentation/devicetree/bindings/clock/clock-bindings.txt
+++ b/Documentation/devicetree/bindings/clock/clock-bindings.txt
@@ -44,6 +44,40 @@ For example:
   clocks by index. The names should reflect the clock output signal
   names for the device.
 
+clock-always-on:    Some hardware contains bunches of clocks which must never be
+		    turned off.  If drivers a) fail to obtain a reference to any
+		    of these or b) give up a previously obtained reference
+		    during suspend, the common clk framework will attempt to
+		    disable them and a platform can fail irrecoverably as a
+		    result.  Usually the only way to recover from these failures
+		    is to reboot.
+
+		    To avoid either of these two scenarios from catastrophically
+		    disabling an otherwise perfectly healthy running system,
+		    clocks can be identified as always-on using this property
+		    from inside a clocksource's node.  The CLK_IGNORE_UNUSED
+		    flag will be applied to each clock instance named in this
+		    property, thus preventing them from being shut down by the
+		    framework.
+
+		    This property is not to be abused.  It is only to be used to
+		    protect platforms from being crippled by gated clocks, not
+		    as a convenience function to avoid using the framework
+		    correctly inside device drivers.
+
+For example:
+
+    oscillator {
+        #clock-cells = <1>;
+        clock-output-names = "ckil", "ckih";
+        clock-always-on = "ckil";
+    };
+
+- this node defines a device with two clock outputs, just as in the
+  example above.  The only difference being that 'ckil' is now
+  identified as an always-on clock, so the framework will know to
+  never attempt to gate it.
+
 clock-indices:	   If the identifying number for the clocks in the node
 		   is not linear from zero, then this allows the mapping of
 		   identifiers into the clock-output-names array.
-- 
1.9.1

Re: [STLinux Kernel] [PATCH 0/4] clk: Provide support for always-on clocks

[Peter Griffin]

Hi Lee,

On Fri, 27 Feb 2015, Lee Jones wrote:

> Some hardware contains bunches of clocks which must never be
> turned off.  If drivers a) fail to obtain a reference to any
> of these or b) give up a previously obtained reference
> during suspend, the common clk framework will attempt to
> disable them and a platform can fail irrecoverably as a
> result.  Usually the only way to recover from these failures
> is to reboot.
> 
> To avoid either of these two scenarios from catastrophically
> disabling an otherwise perfectly healthy running system,
> clocks can be identified as always-on using this property
> from inside a clocksource's node.  The CLK_IGNORE_UNUSED
> flag will be applied to each clock instance named in this
> property, thus preventing them from being shut down by the
> framework.

Great stuff.

One minor comment is that assuming this works on stih407 and stih410
to the extent that the platform can now boot without clk_ignore_unused
kenel parameter then you should have an additional patch to remove
clk_ignore_unused from the default bootargs in stih407-b2120.dts and
stih410-b2120.dts files.

Maxime - Is it possible for you to test this series on stih418-b2199 as
a well? As it could most likely also be removed from stih418-b2199.dts file
to, but neither Lee or myself have the hardware to test.

Apart from that, for the series: -
 Acked-by: Peter Griffin <peter.griffin@linaro.org>

regards,

Peter.

Re: [STLinux Kernel] [PATCH 0/4] clk: Provide support for always-on clocks

[Gabriel Fernandez]

Hi Peter, Lee,

With these series as they are, we need  'clk_ignore_unused'  on
sthi407-b2120.dts and stih418-b2199.dts.

We have to modificate stih407-clock.dtsi and stih418-clock.dtsi in same way.

BR
Gabriel

On 2 April 2015 at 10:12, Peter Griffin <peter.griffin@linaro.org> wrote:
> Hi Lee,
>
> On Fri, 27 Feb 2015, Lee Jones wrote:
>
>> Some hardware contains bunches of clocks which must never be
>> turned off.  If drivers a) fail to obtain a reference to any
>> of these or b) give up a previously obtained reference
>> during suspend, the common clk framework will attempt to
>> disable them and a platform can fail irrecoverably as a
>> result.  Usually the only way to recover from these failures
>> is to reboot.
>>
>> To avoid either of these two scenarios from catastrophically
>> disabling an otherwise perfectly healthy running system,
>> clocks can be identified as always-on using this property
>> from inside a clocksource's node.  The CLK_IGNORE_UNUSED
>> flag will be applied to each clock instance named in this
>> property, thus preventing them from being shut down by the
>> framework.
>
> Great stuff.
>
> One minor comment is that assuming this works on stih407 and stih410
> to the extent that the platform can now boot without clk_ignore_unused
> kenel parameter then you should have an additional patch to remove
> clk_ignore_unused from the default bootargs in stih407-b2120.dts and
> stih410-b2120.dts files.
>
> Maxime - Is it possible for you to test this series on stih418-b2199 as
> a well? As it could most likely also be removed from stih418-b2199.dts file
> to, but neither Lee or myself have the hardware to test.
>
> Apart from that, for the series: -
>  Acked-by: Peter Griffin <peter.griffin@linaro.org>
>
> regards,
>
> Peter.
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [STLinux Kernel] [PATCH 0/4] clk: Provide support for always-on clocks

[Lee Jones]

On Thu, 02 Apr 2015, Gabriel Fernandez wrote:

> Hi Peter, Lee,
> 
> With these series as they are, we need  'clk_ignore_unused'  on
> sthi407-b2120.dts and stih418-b2199.dts.
> 
> We have to modificate stih407-clock.dtsi and stih418-clock.dtsi in same way.

Absolutely.  It's on my backlog.

> On 2 April 2015 at 10:12, Peter Griffin <peter.griffin@linaro.org> wrote:
> > Hi Lee,
> >
> > On Fri, 27 Feb 2015, Lee Jones wrote:
> >
> >> Some hardware contains bunches of clocks which must never be
> >> turned off.  If drivers a) fail to obtain a reference to any
> >> of these or b) give up a previously obtained reference
> >> during suspend, the common clk framework will attempt to
> >> disable them and a platform can fail irrecoverably as a
> >> result.  Usually the only way to recover from these failures
> >> is to reboot.
> >>
> >> To avoid either of these two scenarios from catastrophically
> >> disabling an otherwise perfectly healthy running system,
> >> clocks can be identified as always-on using this property
> >> from inside a clocksource's node.  The CLK_IGNORE_UNUSED
> >> flag will be applied to each clock instance named in this
> >> property, thus preventing them from being shut down by the
> >> framework.
> >
> > Great stuff.
> >
> > One minor comment is that assuming this works on stih407 and stih410
> > to the extent that the platform can now boot without clk_ignore_unused
> > kenel parameter then you should have an additional patch to remove
> > clk_ignore_unused from the default bootargs in stih407-b2120.dts and
> > stih410-b2120.dts files.
> >
> > Maxime - Is it possible for you to test this series on stih418-b2199 as
> > a well? As it could most likely also be removed from stih418-b2199.dts file
> > to, but neither Lee or myself have the hardware to test.
> >
> > Apart from that, for the series: -
> >  Acked-by: Peter Griffin <peter.griffin@linaro.org>
> >
> > regards,
> >
> > Peter.
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [STLinux Kernel] [PATCH 0/4] clk: Provide support for always-on clocks

[Lee Jones]

> With these series as they are, we need  'clk_ignore_unused'  on
> sthi407-b2120.dts and stih418-b2199.dts.
> 
> We have to modificate stih407-clock.dtsi and stih418-clock.dtsi in same way.

Just to clarify this point for all interested parties; this patch-set
is not the completed article.  It's purpose is to add the core
functionality and the documentation _only_.  The STiH410 support is
only provided in this set as an example of how the bindings are to be
used.  Other platforms are to be enabled subsequently as part of a
different submission.

> On 2 April 2015 at 10:12, Peter Griffin <peter.griffin@linaro.org> wrote:
> > Hi Lee,
> >
> > On Fri, 27 Feb 2015, Lee Jones wrote:
> >
> >> Some hardware contains bunches of clocks which must never be
> >> turned off.  If drivers a) fail to obtain a reference to any
> >> of these or b) give up a previously obtained reference
> >> during suspend, the common clk framework will attempt to
> >> disable them and a platform can fail irrecoverably as a
> >> result.  Usually the only way to recover from these failures
> >> is to reboot.
> >>
> >> To avoid either of these two scenarios from catastrophically
> >> disabling an otherwise perfectly healthy running system,
> >> clocks can be identified as always-on using this property
> >> from inside a clocksource's node.  The CLK_IGNORE_UNUSED
> >> flag will be applied to each clock instance named in this
> >> property, thus preventing them from being shut down by the
> >> framework.
> >
> > Great stuff.
> >
> > One minor comment is that assuming this works on stih407 and stih410
> > to the extent that the platform can now boot without clk_ignore_unused
> > kenel parameter then you should have an additional patch to remove
> > clk_ignore_unused from the default bootargs in stih407-b2120.dts and
> > stih410-b2120.dts files.
> >
> > Maxime - Is it possible for you to test this series on stih418-b2199 as
> > a well? As it could most likely also be removed from stih418-b2199.dts file
> > to, but neither Lee or myself have the hardware to test.
> >
> > Apart from that, for the series: -
> >  Acked-by: Peter Griffin <peter.griffin@linaro.org>
> >
> > regards,
> >
> > Peter.
> >
> > _______________________________________________
> > linux-arm-kernel mailing list
> > linux-arm-kernel@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

Re: [STLinux Kernel] [PATCH 0/4] clk: Provide support for always-on clocks

[Lee Jones]

On Thu, 02 Apr 2015, Peter Griffin wrote:

> Hi Lee,
> 
> On Fri, 27 Feb 2015, Lee Jones wrote:
> 
> > Some hardware contains bunches of clocks which must never be
> > turned off.  If drivers a) fail to obtain a reference to any
> > of these or b) give up a previously obtained reference
> > during suspend, the common clk framework will attempt to
> > disable them and a platform can fail irrecoverably as a
> > result.  Usually the only way to recover from these failures
> > is to reboot.
> > 
> > To avoid either of these two scenarios from catastrophically
> > disabling an otherwise perfectly healthy running system,
> > clocks can be identified as always-on using this property
> > from inside a clocksource's node.  The CLK_IGNORE_UNUSED
> > flag will be applied to each clock instance named in this
> > property, thus preventing them from being shut down by the
> > framework.
> 
> Great stuff.
> 
> One minor comment is that assuming this works on stih407 and stih410
> to the extent that the platform can now boot without clk_ignore_unused
> kenel parameter then you should have an additional patch to remove
> clk_ignore_unused from the default bootargs in stih407-b2120.dts and
> stih410-b2120.dts files.

It's already on my TODO list:

TODO:
  - Clock Domain
    - Once the clk domain driver has been accepted, remove early returns in SPI driver
    - Remove clk_ignore_unused from the DT files
    - Add LMI1 clk to STiH418

> Maxime - Is it possible for you to test this series on stih418-b2199 as
> a well? As it could most likely also be removed from stih418-b2199.dts file
> to, but neither Lee or myself have the hardware to test.

Take a look at the third item on my TODO. :)

Yes Maxime, if you can test the set once I've completed item 3, that
would be splendid.

> Apart from that, for the series: -
>  Acked-by: Peter Griffin <peter.griffin@linaro.org>

Thanks Peter.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[PATCH 0/4] clk: st: New clock domain

[Lee Jones]

ST hardware have a bunch of clocks which must not be turned off.
If drivers a) fail to obtain a reference to any of these or b) give
up a previously obtained reference during suspend, the common clk
framework will attempt to turn them off and the hardware will
subsequently die.  The only way to recover from this failure is to
restart.

To avoid either of these two scenarios from catastrophically
disabling the running system we have implemented a clock domain
where clocks are consumed and references are taken, thus preventing
them from being shut down by the framework.

Lee Jones (4):
  ARM: sti: stih407-family: Supply defines for CLOCKGEN A0
  ARM: sti: stih407-family: Provide Clock Domain information
  clk: st: Provide a clock domain
  clk: dt: st: Introduce clock domain documentation

 .../devicetree/bindings/clock/st/st,clk-domain.txt | 34 ++++++++++++
 arch/arm/boot/dts/stih407-family.dtsi              | 13 +++++
 drivers/clk/st/Makefile                            |  2 +-
 drivers/clk/st/clk-domain.c                        | 63 ++++++++++++++++++++++
 include/dt-bindings/clock/stih407-clks.h           |  4 ++
 5 files changed, 115 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/clock/st/st,clk-domain.txt
 create mode 100644 drivers/clk/st/clk-domain.c

-- 
1.9.1

[PATCH 1/4] ARM: sti: stih407-family: Supply defines for CLOCKGEN A0

[Lee Jones]

There are 2 LMI clocks generated by CLOCKGEN A0.  We wish to control
them individually and need to use these indexes to do so.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 include/dt-bindings/clock/stih407-clks.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/include/dt-bindings/clock/stih407-clks.h b/include/dt-bindings/clock/stih407-clks.h
index 7af2b71..082edd9 100644
--- a/include/dt-bindings/clock/stih407-clks.h
+++ b/include/dt-bindings/clock/stih407-clks.h
@@ -5,6 +5,10 @@
 #ifndef _DT_BINDINGS_CLK_STIH407
 #define _DT_BINDINGS_CLK_STIH407
 
+/* CLOCKGEN A0 */
+#define CLK_IC_LMI0		0
+#define CLK_IC_LMI1		1
+
 /* CLOCKGEN C0 */
 #define CLK_ICN_GPU		0
 #define CLK_FDMA		1
-- 
1.9.1