Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1311580 > unrolled thread

[PATCH 0/3] clk: Add support for critical clocks

Started byLee Jones <lee.jones@linaro.org>
First post2016-01-18 15:40 +0100
Last post2016-01-19 09:00 +0100
Articles 6 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH 0/3] clk: Add support for critical clocks Lee Jones <lee.jones@linaro.org> - 2016-01-18 15:40 +0100
    [PATCH 2/3] clk: WARN_ON about to disable a critical clock Lee Jones <lee.jones@linaro.org> - 2016-01-18 15:40 +0100
    [PATCH 3/3] clk: Provide OF helper to mark clocks as CRITICAL Lee Jones <lee.jones@linaro.org> - 2016-01-18 15:40 +0100
    [PATCH 1/3] clk: Allow clocks to be marked as CRITICAL Lee Jones <lee.jones@linaro.org> - 2016-01-18 15:40 +0100
      Re: [PATCH 1/3] clk: Allow clocks to be marked as CRITICAL Geert Uytterhoeven <geert@linux-m68k.org> - 2016-01-18 18:20 +0100
        Re: [PATCH 1/3] clk: Allow clocks to be marked as CRITICAL Lee Jones <lee.jones@linaro.org> - 2016-01-19 09:00 +0100

#1311580 — [PATCH 0/3] clk: Add support for critical clocks

FromLee Jones <lee.jones@linaro.org>
Date2016-01-18 15:40 +0100
Subject[PATCH 0/3] clk: Add support for critical clocks
Message-ID<qSj8l-1Wu-7@gated-at.bofh.it>
Some platforms contain clocks which if gated, will cause undefined or
catastrophic behaviours.  As such they are not to be turned off, ever.
Many of these such clocks do not have devices, thus device drivers
where clocks may be enabled and references taken to ensure they stay
enabled do not exist.  Therefore, we must handle these such cases in
the core.

This patchset defines an CLK_IS_CRITICAL flag which the core can use
to identify critical clocks and subsequently refuse to gate them.
Once a clock has been recognised as critical, we take extra
references to ensure the continued functionality of the clock
whatever else happens.

Mike,

It's been 17 weeks since our meeting in San Francisco and I'm keen to
move this forward.  As per our meeting, the plan is to separate our
two requirements, as users who require both critical clocks AND the
hand-off feature do not currently exist.  If you'd like to continue
enablement of the hand-off functionality you were interested in, I'll
continue on with critical clocks, as we still need this for our
platform.

I'm hoping this isn't the wrong approach, but if it is, let me know
how it can be improved and I'll re-roll.

Kind regards,
Lee

Lee Jones (3):
  clk: Allow clocks to be marked as CRITICAL
  clk: WARN_ON about to disable a critical clock
  clk: Provide OF helper to mark clocks as CRITICAL

 drivers/clk/clk.c            | 13 ++++++++++++-
 include/linux/clk-provider.h | 23 +++++++++++++++++++++++
 2 files changed, 35 insertions(+), 1 deletion(-)

-- 
1.9.1

[toc] | [next] | [standalone]


#1311581 — [PATCH 2/3] clk: WARN_ON about to disable a critical clock

FromLee Jones <lee.jones@linaro.org>
Date2016-01-18 15:40 +0100
Subject[PATCH 2/3] clk: WARN_ON about to disable a critical clock
Message-ID<qSj8m-1Wu-17@gated-at.bofh.it>
In reply to#1311580
Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/clk/clk.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 835cb85..178b364 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -575,6 +575,9 @@ static void clk_core_unprepare(struct clk_core *core)
 	if (WARN_ON(core->prepare_count == 0))
 		return;
 
+	if (WARN_ON(core->prepare_count == 1 && core->flags & CLK_IS_CRITICAL))
+		return;
+
 	if (--core->prepare_count > 0)
 		return;
 
@@ -680,6 +683,9 @@ static void clk_core_disable(struct clk_core *core)
 	if (WARN_ON(core->enable_count == 0))
 		return;
 
+	if (WARN_ON(core->enable_count == 1 && core->flags & CLK_IS_CRITICAL))
+		return;
+
 	if (--core->enable_count > 0)
 		return;
 
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]


#1311585 — [PATCH 3/3] clk: Provide OF helper to mark clocks as CRITICAL

FromLee Jones <lee.jones@linaro.org>
Date2016-01-18 15:40 +0100
Subject[PATCH 3/3] clk: Provide OF helper to mark clocks as CRITICAL
Message-ID<qSj8m-1Wu-25@gated-at.bofh.it>
In reply to#1311580
This call matches clocks which have been marked as critical in DT
and sets the appropriate flag.  These flags can then be used to
mark the clock core flags appropriately prior to registration.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 include/linux/clk-provider.h | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index ffa0b2e..6f178b7 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -707,6 +707,23 @@ const char *of_clk_get_parent_name(struct device_node *np, int index);
 
 void of_clk_init(const struct of_device_id *matches);
 
+static inline int of_clk_mark_if_critical(struct device_node *np,
+					  int index, unsigned long *flags)
+{
+	struct property *prop;
+	const __be32 *cur;
+	uint32_t idx;
+
+	if (!np || !flags)
+		return -EINVAL;
+
+	of_property_for_each_u32(np, "critical-clock", prop, cur, idx)
+		if (index == idx)
+			*flags |= CLK_IS_CRITICAL;
+
+	return 0;
+}
+
 #else /* !CONFIG_OF */
 
 static inline int of_clk_add_provider(struct device_node *np,
@@ -742,6 +759,11 @@ static inline const char *of_clk_get_parent_name(struct device_node *np,
 {
 	return NULL;
 }
+static inline int of_clk_mark_if_critical(struct device_node *np, int index,
+					  unsigned long *flags)
+{
+	return 0;
+}
 #define of_clk_init(matches) \
 	{ while (0); }
 #endif /* CONFIG_OF */
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]


#1311586 — [PATCH 1/3] clk: Allow clocks to be marked as CRITICAL

FromLee Jones <lee.jones@linaro.org>
Date2016-01-18 15:40 +0100
Subject[PATCH 1/3] clk: Allow clocks to be marked as CRITICAL
Message-ID<qSj8m-1Wu-31@gated-at.bofh.it>
In reply to#1311580
Critical clocks are those which must not be gated, else undefined
or catastrophic failure would occur.  Here we have chosen to
ensure the prepare/enable counts are correctly incremented, so as
not to confuse users with enabled clocks with no visible users.

Signed-off-by: Lee Jones <lee.jones@linaro.org>
---
 drivers/clk/clk.c            | 7 ++++++-
 include/linux/clk-provider.h | 1 +
 2 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index f13c3f4..835cb85 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2576,8 +2576,13 @@ struct clk *clk_register(struct device *dev, struct clk_hw *hw)
 	}
 
 	ret = __clk_init(dev, hw->clk);
-	if (!ret)
+	if (!ret) {
+		if (core->flags & CLK_IS_CRITICAL) {
+			clk_core_prepare(core);
+			clk_core_enable(core);
+		}
 		return hw->clk;
+	}
 
 	__clk_free_clk(hw->clk);
 	hw->clk = NULL;
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index c56988a..ffa0b2e 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -31,6 +31,7 @@
 #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
 #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
 #define CLK_RECALC_NEW_RATES	BIT(9) /* recalc rates after notifications */
+#define CLK_IS_CRITICAL		BIT(10) /* do not gate, ever */
 
 struct clk;
 struct clk_hw;
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]


#1311696 — Re: [PATCH 1/3] clk: Allow clocks to be marked as CRITICAL

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2016-01-18 18:20 +0100
SubjectRe: [PATCH 1/3] clk: Allow clocks to be marked as CRITICAL
Message-ID<qSlDd-3Oc-31@gated-at.bofh.it>
In reply to#1311586
Hi Lee,

On Mon, Jan 18, 2016 at 3:28 PM, Lee Jones <lee.jones@linaro.org> wrote:
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> @@ -31,6 +31,7 @@
>  #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
>  #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
>  #define CLK_RECALC_NEW_RATES   BIT(9) /* recalc rates after notifications */
> +#define CLK_IS_CRITICAL                BIT(10) /* do not gate, ever */

10 is already taken, even upstream. Please rebase ;-)

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]


#1311983 — Re: [PATCH 1/3] clk: Allow clocks to be marked as CRITICAL

FromLee Jones <lee.jones@linaro.org>
Date2016-01-19 09:00 +0100
SubjectRe: [PATCH 1/3] clk: Allow clocks to be marked as CRITICAL
Message-ID<qSzmO-4JC-13@gated-at.bofh.it>
In reply to#1311696
On Mon, 18 Jan 2016, Geert Uytterhoeven wrote:
> On Mon, Jan 18, 2016 at 3:28 PM, Lee Jones <lee.jones@linaro.org> wrote:
> > --- a/include/linux/clk-provider.h
> > +++ b/include/linux/clk-provider.h
> > @@ -31,6 +31,7 @@
> >  #define CLK_SET_RATE_NO_REPARENT BIT(7) /* don't re-parent on rate change */
> >  #define CLK_GET_ACCURACY_NOCACHE BIT(8) /* do not use the cached clk accuracy */
> >  #define CLK_RECALC_NEW_RATES   BIT(9) /* recalc rates after notifications */
> > +#define CLK_IS_CRITICAL                BIT(10) /* do not gate, ever */
> 
> 10 is already taken, even upstream. Please rebase ;-)

Thanks for the heads-up.  Will pull Heiko's patch in and rebase.

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

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web