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

[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

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[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: stih407-family: Provide Clock Domain information

[Lee Jones]

Certain clocks should not be turned of by clk_disable_unused.  Until
now we have been using the kernel command-line of the same name to
prevent common clk from turning off all clocks without a reference,
as this would ensure hardware lockup.  This patch lists each clock
which need to stay on to prevent the aforementioned issue from
arising.

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

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index 3e31d32..0478b41 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -34,6 +34,19 @@
 		reg = <0x08761000 0x1000>, <0x08760100 0x100>;
 	};
 
+	clk-domain {
+		compatible = "st,clk-domain";
+		clocks = <&clk_s_c0_flexgen CLK_EXT2F_A9>,
+			 <&clk_s_c0_flexgen CLK_COMPO_DVP>,
+			 <&clk_s_c0_flexgen CLK_MMC_1>,
+			 <&clk_s_c0_flexgen CLK_ICN_SBC>,
+			 <&clk_s_c0_flexgen CLK_ICN_LMI>,
+			 <&clk_s_c0_flexgen CLK_ICN_CPU>,
+			 <&clk_s_c0_flexgen CLK_TX_ICN_DMU>,
+			 <&clk_s_a0_flexgen CLK_IC_LMI0>,
+			 <&clk_m_a9>;
+	};
+
 	scu@08760000 {
 		compatible = "arm,cortex-a9-scu";
 		reg = <0x08760000 0x1000>;
-- 
1.9.1

[PATCH 3/4] clk: st: Provide a clock domain

[Lee Jones]

ST's h/w contains 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.

Signed-off-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/clk/st/Makefile     |  2 +-
 drivers/clk/st/clk-domain.c | 63 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 1 deletion(-)
 create mode 100644 drivers/clk/st/clk-domain.c

diff --git a/drivers/clk/st/Makefile b/drivers/clk/st/Makefile
index ede7b2f..ac09be6 100644
--- a/drivers/clk/st/Makefile
+++ b/drivers/clk/st/Makefile
@@ -1 +1 @@
-obj-y += clkgen-mux.o clkgen-pll.o clkgen-fsyn.o clk-flexgen.o
+obj-y += clkgen-mux.o clkgen-pll.o clkgen-fsyn.o clk-flexgen.o clk-domain.o
diff --git a/drivers/clk/st/clk-domain.c b/drivers/clk/st/clk-domain.c
new file mode 100644
index 0000000..e34049b
--- /dev/null
+++ b/drivers/clk/st/clk-domain.c
@@ -0,0 +1,63 @@
+/*
+ * ST Clock Domain
+ *
+ * Copyright (C) 2015 STMicroelectronics – All Rights Reserved
+ *
+ * Author: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
+#include <linux/clk-private.h>
+#include <linux/clk-provider.h>
+#include <linux/of_address.h>
+#include <linux/of.h>
+#include <linux/platform_device.h>
+
+static void st_clk_domain_hog_clock(struct platform_device *pdev, int index)
+{
+	struct device_node *np = pdev->dev.of_node;
+	struct clk *clk;
+	int ret;
+
+	clk = of_clk_get(np, index);
+	if (IS_ERR(clk)) {
+		dev_warn(&pdev->dev, "Failed get clock %s[%d]: %li\n",
+			 np->full_name, index, PTR_ERR(clk));
+		return;
+	}
+
+	ret = clk_prepare_enable(clk);
+	if (ret)
+		dev_warn(&pdev->dev, "Failed to enable clock: %s\n", clk->name);
+}
+
+static int st_clk_domain_probe(struct platform_device *pdev)
+{
+	struct device_node *np = pdev->dev.of_node;
+	int nclks, i;
+
+	nclks = of_count_phandle_with_args(np, "clocks", "#clock-cells");
+
+	for (i = 0; i < nclks; i++)
+		st_clk_domain_hog_clock(pdev, i);
+
+	return 0;
+}
+
+static const struct of_device_id st_clk_domain_match[] = {
+	{ .compatible = "st,clk-domain" },
+	{ },
+};
+
+static struct platform_driver st_clk_domain_driver = {
+	.probe = st_clk_domain_probe,
+	.driver = {
+		.name = "st-clk-domain",
+		.of_match_table = st_clk_domain_match,
+	},
+};
+module_platform_driver(st_clk_domain_driver);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 4/4] clk: dt: st: Introduce clock domain documentation

[Lee Jones]

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

diff --git a/Documentation/devicetree/bindings/clock/st/st,clk-domain.txt b/Documentation/devicetree/bindings/clock/st/st,clk-domain.txt
new file mode 100644
index 0000000..7309937
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/st/st,clk-domain.txt
@@ -0,0 +1,34 @@
+STMicroelectronics Clock Domain
+
+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.
+
+We use the generic clock bindings found in:
+  Documentation/devicetree/bindings/clock/clock-bindings.txt
+
+Required properties:
+- compatible : Must be "st,clk-domain"
+
+Example:
+
+clk-domain {
+	compatible = "st,clk-domain";
+	clocks = <&clk_s_c0_flexgen CLK_EXT2F_A9>,
+		 <&clk_s_c0_flexgen CLK_COMPO_DVP>,
+		 <&clk_s_c0_flexgen CLK_MMC_1>,
+		 <&clk_s_c0_flexgen CLK_ICN_SBC>,
+		 <&clk_s_c0_flexgen CLK_ICN_LMI>,
+		 <&clk_s_c0_flexgen CLK_ICN_CPU>,
+		 <&clk_s_c0_flexgen CLK_TX_ICN_DMU>,
+		 <&clk_s_a0_flexgen CLK_IC_LMI0>,
+		 <&clk_m_a9>;
+};
-- 
1.9.1

Re: [PATCH 4/4] clk: dt: st: Introduce clock domain documentation

[Mike Turquette]

Quoting Lee Jones (2015-01-26 03:14:00)
> Signed-off-by: Lee Jones <lee.jones-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  .../devicetree/bindings/clock/st/st,clk-domain.txt | 34 ++++++++++++++++++++++
>  1 file changed, 34 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/clock/st/st,clk-domain.txt
> 
> diff --git a/Documentation/devicetree/bindings/clock/st/st,clk-domain.txt b/Documentation/devicetree/bindings/clock/st/st,clk-domain.txt
> new file mode 100644
> index 0000000..7309937
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/clock/st/st,clk-domain.txt
> @@ -0,0 +1,34 @@
> +STMicroelectronics Clock Domain
> +
> +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.
> +
> +We use the generic clock bindings found in:
> +  Documentation/devicetree/bindings/clock/clock-bindings.txt
> +
> +Required properties:
> +- compatible : Must be "st,clk-domain"

Seems like a useful feature for any clock provider, not just ST's. Have
you thought about making this solution generic for DT-based clock
providers?

We could amend the common clock binding to include a special "always on"
clock group that is automagically prepared and enabled when the clock
provider/driver is registered, using a common function.

Regards,
Mike

> +
> +Example:
> +
> +clk-domain {
> +       compatible = "st,clk-domain";
> +       clocks = <&clk_s_c0_flexgen CLK_EXT2F_A9>,
> +                <&clk_s_c0_flexgen CLK_COMPO_DVP>,
> +                <&clk_s_c0_flexgen CLK_MMC_1>,
> +                <&clk_s_c0_flexgen CLK_ICN_SBC>,
> +                <&clk_s_c0_flexgen CLK_ICN_LMI>,
> +                <&clk_s_c0_flexgen CLK_ICN_CPU>,
> +                <&clk_s_c0_flexgen CLK_TX_ICN_DMU>,
> +                <&clk_s_a0_flexgen CLK_IC_LMI0>,
> +                <&clk_m_a9>;
> +};
> -- 
> 1.9.1
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 4/4] clk: dt: st: Introduce clock domain documentation

[Lee Jones]

On Tue, 27 Jan 2015, Mike Turquette wrote:

> Quoting Lee Jones (2015-01-26 03:14:00)
> > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > ---
> >  .../devicetree/bindings/clock/st/st,clk-domain.txt | 34 ++++++++++++++++++++++
> >  1 file changed, 34 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/clock/st/st,clk-domain.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/clock/st/st,clk-domain.txt b/Documentation/devicetree/bindings/clock/st/st,clk-domain.txt
> > new file mode 100644
> > index 0000000..7309937
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/st/st,clk-domain.txt
> > @@ -0,0 +1,34 @@
> > +STMicroelectronics Clock Domain
> > +
> > +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.
> > +
> > +We use the generic clock bindings found in:
> > +  Documentation/devicetree/bindings/clock/clock-bindings.txt
> > +
> > +Required properties:
> > +- compatible : Must be "st,clk-domain"
> 
> Seems like a useful feature for any clock provider, not just ST's. Have
> you thought about making this solution generic for DT-based clock
> providers?
> 
> We could amend the common clock binding to include a special "always on"
> clock group that is automagically prepared and enabled when the clock
> provider/driver is registered, using a common function.

OMG, I'm actually going to strangle you!

This is what I've been proposing to you (privately) for weeks.

Does this ring any bells?

  "Just FYI, I am not going to add any method to the kernel that
  permanently enables a clock via some new api. At the worst case your
  clock driver can simply call clk_prepare_enable in its probe
  function (there are some examples of this)."

I will be more than happy to make this a generic driver, if thats what
you want (now). ;)

> > +Example:
> > +
> > +clk-domain {
> > +       compatible = "st,clk-domain";
> > +       clocks = <&clk_s_c0_flexgen CLK_EXT2F_A9>,
> > +                <&clk_s_c0_flexgen CLK_COMPO_DVP>,
> > +                <&clk_s_c0_flexgen CLK_MMC_1>,
> > +                <&clk_s_c0_flexgen CLK_ICN_SBC>,
> > +                <&clk_s_c0_flexgen CLK_ICN_LMI>,
> > +                <&clk_s_c0_flexgen CLK_ICN_CPU>,
> > +                <&clk_s_c0_flexgen CLK_TX_ICN_DMU>,
> > +                <&clk_s_a0_flexgen CLK_IC_LMI0>,
> > +                <&clk_m_a9>;
> > +};

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

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 4/4] clk: dt: st: Introduce clock domain documentation

[Mike Turquette]

Quoting Lee Jones (2015-01-27 23:58:35)
> On Tue, 27 Jan 2015, Mike Turquette wrote:
> 
> > Quoting Lee Jones (2015-01-26 03:14:00)
> > > Signed-off-by: Lee Jones <lee.jones@linaro.org>
> > > ---
> > >  .../devicetree/bindings/clock/st/st,clk-domain.txt | 34 ++++++++++++++++++++++
> > >  1 file changed, 34 insertions(+)
> > >  create mode 100644 Documentation/devicetree/bindings/clock/st/st,clk-domain.txt
> > > 
> > > diff --git a/Documentation/devicetree/bindings/clock/st/st,clk-domain.txt b/Documentation/devicetree/bindings/clock/st/st,clk-domain.txt
> > > new file mode 100644
> > > index 0000000..7309937
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/clock/st/st,clk-domain.txt
> > > @@ -0,0 +1,34 @@
> > > +STMicroelectronics Clock Domain
> > > +
> > > +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.
> > > +
> > > +We use the generic clock bindings found in:
> > > +  Documentation/devicetree/bindings/clock/clock-bindings.txt
> > > +
> > > +Required properties:
> > > +- compatible : Must be "st,clk-domain"
> > 
> > Seems like a useful feature for any clock provider, not just ST's. Have
> > you thought about making this solution generic for DT-based clock
> > providers?
> > 
> > We could amend the common clock binding to include a special "always on"
> > clock group that is automagically prepared and enabled when the clock
> > provider/driver is registered, using a common function.
> 
> OMG, I'm actually going to strangle you!
> 
> This is what I've been proposing to you (privately) for weeks.

Yikes! Please do not strangle me yet. I have books on loan from my
public library that I must return before I expire.

> 
> Does this ring any bells?
> 
>   "Just FYI, I am not going to add any method to the kernel that
>   permanently enables a clock via some new api. At the worst case your
>   clock driver can simply call clk_prepare_enable in its probe
>   function (there are some examples of this)."
> 

In all fairness I found the chat log where I wrote the above quote and
there was not much context for the problem you were trying to solve. And
we were both trapped in different meetings at a conference and could not
discuss it face to face before your flight. Some things are done better
over email than over a quick Google chat.

> I will be more than happy to make this a generic driver, if thats what
> you want (now). ;)

Well your method does use clk_get and clk_prepare_enable which satisfies
my statement above. When I said "no" to this idea earlier I had imagined
you banging on the register to keep the clock enabled in hardware, but
have the framework keep the enable_count & prepare_count at 0. Folks
have asked for this before and it is a no-go.

Right now I think that the common clock binding (which is the root of
all of the vendor-specific bindings) could create a special container
for always-on clocks. We would need to define this behavior. For
simplicity I think the only clocks that should go into this container
are clocks that we never gate during the lifetime of the device being
powered on, and they are clocks that do not need to be managed by
external drivers.

of_clk_set_defaults is called from of_clk_add_provider which might be a
good place to start looking.

What do you think?

Regards,
Mike

> 
> > > +Example:
> > > +
> > > +clk-domain {
> > > +       compatible = "st,clk-domain";
> > > +       clocks = <&clk_s_c0_flexgen CLK_EXT2F_A9>,
> > > +                <&clk_s_c0_flexgen CLK_COMPO_DVP>,
> > > +                <&clk_s_c0_flexgen CLK_MMC_1>,
> > > +                <&clk_s_c0_flexgen CLK_ICN_SBC>,
> > > +                <&clk_s_c0_flexgen CLK_ICN_LMI>,
> > > +                <&clk_s_c0_flexgen CLK_ICN_CPU>,
> > > +                <&clk_s_c0_flexgen CLK_TX_ICN_DMU>,
> > > +                <&clk_s_a0_flexgen CLK_IC_LMI0>,
> > > +                <&clk_m_a9>;
> > > +};
> 
> -- 
> Lee Jones
> Linaro STMicroelectronics Landing Team Lead
> Linaro.org │ Open source software for ARM SoCs
> Follow Linaro: Facebook | Twitter | Blog

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel