[PATCH 0/4] clk: Support for smp_twd clock for ux500

STALE5067d

16 messages, 5 authors, 2012-09-19 · open the first message on its own page

[PATCH 0/4] clk: Support for smp_twd clock for ux500

From: Ulf Hansson <hidden>
Date: 2012-08-31 12:22:07

From: Ulf Hansson <redacted>

To implement support for the smp_twd clock for ux500 several steps
was needed. This patchseries has also proposed some new changes in the
common clock core, which the ux500 smp_twd clock definition are
relying on.

Moreover some crossdepency exist to the ux500 prcmu mfd driver.
My idea is that the corresponding maintainers may be able to ack these
changes, if they like them of course, so we can merge this through
Mike Turquette's clk git tree. Otherwise I can happily set up some other
way forward, if that suits better.

Patches is based upon Linux 3.6 rc2. The clock patches specific for ux500,
is based upon the series [PATCH V2 0/4] clk: Convert ARM ux500 to common
clock.

Michel Jaouen (1):
  mfd: dbx500: Provide a more accurate smp_twd clock

Ulf Hansson (3):
  clk: Provide option for clk_get_rate to issue hw for new rate
  clk: ux500: Support for prmcu_rate clock
  clk: ux500: Define smp_twd clock for u8500

 drivers/clk/clk.c                |   43 +++++++++++++++++++++-----------------
 drivers/clk/ux500/clk-prcmu.c    |   14 +++++++++++++
 drivers/clk/ux500/clk.h          |    5 +++++
 drivers/clk/ux500/u8500_clk.c    |   12 +++++++----
 drivers/mfd/db8500-prcmu.c       |   42 +++++++++++++++++++++++++++++++++++++
 drivers/mfd/dbx500-prcmu-regs.h  |    4 +++-
 include/linux/clk-provider.h     |    1 +
 include/linux/mfd/dbx500-prcmu.h |    1 +
 8 files changed, 98 insertions(+), 24 deletions(-)

-- 
1.7.10

[PATCH 1/4] clk: Provide option for clk_get_rate to issue hw for new rate

From: Ulf Hansson <hidden>
Date: 2012-08-31 12:22:15

From: Ulf Hansson <redacted>

By using CLK_GET_RATE_NOCACHE flag, we tell the clk_get_rate API to
issue the hw for an updated clock rate. This can be used for a clock
which rate may be updated without a client necessary modifying it.

Signed-off-by: Ulf Hansson <redacted>
---
 drivers/clk/clk.c            |   43 +++++++++++++++++++++++-------------------
 include/linux/clk-provider.h |    1 +
 2 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index efdfd00..d9cbae0 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -558,25 +558,6 @@ int clk_enable(struct clk *clk)
 EXPORT_SYMBOL_GPL(clk_enable);
 
 /**
- * clk_get_rate - return the rate of clk
- * @clk: the clk whose rate is being returned
- *
- * Simply returns the cached rate of the clk.  Does not query the hardware.  If
- * clk is NULL then returns 0.
- */
-unsigned long clk_get_rate(struct clk *clk)
-{
-	unsigned long rate;
-
-	mutex_lock(&prepare_lock);
-	rate = __clk_get_rate(clk);
-	mutex_unlock(&prepare_lock);
-
-	return rate;
-}
-EXPORT_SYMBOL_GPL(clk_get_rate);
-
-/**
  * __clk_round_rate - round the given rate for a clk
  * @clk: round the rate of this clock
  *
@@ -702,6 +683,30 @@ static void __clk_recalc_rates(struct clk *clk, unsigned long msg)
 }
 
 /**
+ * clk_get_rate - return the rate of clk
+ * @clk: the clk whose rate is being returned
+ *
+ * Simply returns the cached rate of the clk, unless CLK_GET_RATE_NOCACHE flag
+ * is set, which means a recalc_rate will be issued.
+ * If clk is NULL then returns 0.
+ */
+unsigned long clk_get_rate(struct clk *clk)
+{
+	unsigned long rate;
+
+	mutex_lock(&prepare_lock);
+
+	if (clk && (clk->flags & CLK_GET_RATE_NOCACHE))
+		__clk_recalc_rates(clk, 0);
+
+	rate = __clk_get_rate(clk);
+	mutex_unlock(&prepare_lock);
+
+	return rate;
+}
+EXPORT_SYMBOL_GPL(clk_get_rate);
+
+/**
  * __clk_speculate_rates
  * @clk: first clk in the subtree
  * @parent_rate: the "future" rate of clk's parent
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 77335fa..1b15307 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -26,6 +26,7 @@
 #define CLK_IGNORE_UNUSED	BIT(3) /* do not gate even if unused */
 #define CLK_IS_ROOT		BIT(4) /* root clk, has no parent */
 #define CLK_IS_BASIC		BIT(5) /* Basic clk, can't do a to_clk_foo() */
+#define CLK_GET_RATE_NOCACHE	BIT(6) /* do not use the cached clk rate */
 
 struct clk_hw;
 
-- 
1.7.10

[PATCH 3/4] mfd: dbx500: Provide a more accurate smp_twd clock

From: Ulf Hansson <hidden>
Date: 2012-08-31 12:22:19

From: Michel Jaouen <redacted>

The local timer clock is based on ARM subsystem clock. This patch
obtains a more exact value of that clock by reading PRCMU registers.
Using this increases the accuracy of the local timer events.

Signed-off-by: Ulf Hansson <redacted>
Signed-off-by: Rickard Andersson <redacted>
Signed-off-by: Michel Jaouen <redacted>
---
 drivers/mfd/db8500-prcmu.c       |   42 ++++++++++++++++++++++++++++++++++++++
 drivers/mfd/dbx500-prcmu-regs.h  |    4 +++-
 include/linux/mfd/dbx500-prcmu.h |    1 +
 3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
index 4f74529..e7f9539 100644
--- a/drivers/mfd/db8500-prcmu.c
+++ b/drivers/mfd/db8500-prcmu.c
@@ -418,6 +418,9 @@ static struct {
 
 static atomic_t ac_wake_req_state = ATOMIC_INIT(0);
 
+/* Functions definition */
+static void compute_armss_rate(void);
+
 /* Spinlocks */
 static DEFINE_SPINLOCK(prcmu_lock);
 static DEFINE_SPINLOCK(clkout_lock);
@@ -517,6 +520,7 @@ static struct dsiescclk dsiescclk[3] = {
 	}
 };
 
+
 /*
 * Used by MCDE to setup all necessary PRCMU registers
 */
@@ -1013,6 +1017,7 @@ int db8500_prcmu_set_arm_opp(u8 opp)
 		(mb1_transfer.ack.arm_opp != opp))
 		r = -EIO;
 
+	compute_armss_rate();
 	mutex_unlock(&mb1_transfer.lock);
 
 	return r;
@@ -1612,6 +1617,7 @@ static unsigned long pll_rate(void __iomem *reg, unsigned long src_rate,
 	if ((branch == PLL_FIX) || ((branch == PLL_DIV) &&
 		(val & PRCM_PLL_FREQ_DIV2EN) &&
 		((reg == PRCM_PLLSOC0_FREQ) ||
+		 (reg == PRCM_PLLARM_FREQ) ||
 		 (reg == PRCM_PLLDDR_FREQ))))
 		div *= 2;
 
@@ -1661,6 +1667,39 @@ static unsigned long clock_rate(u8 clock)
 	else
 		return 0;
 }
+static unsigned long latest_armss_rate;
+static unsigned long armss_rate(void)
+{
+	return latest_armss_rate;
+}
+
+static void compute_armss_rate(void)
+{
+	u32 r;
+	unsigned long rate;
+
+	r = readl(PRCM_ARM_CHGCLKREQ);
+
+	if (r & PRCM_ARM_CHGCLKREQ_PRCM_ARM_CHGCLKREQ) {
+		/* External ARMCLKFIX clock */
+
+		rate = pll_rate(PRCM_PLLDDR_FREQ, ROOT_CLOCK_RATE, PLL_FIX);
+
+		/* Check PRCM_ARM_CHGCLKREQ divider */
+		if (!(r & PRCM_ARM_CHGCLKREQ_PRCM_ARM_DIVSEL))
+			rate /= 2;
+
+		/* Check PRCM_ARMCLKFIX_MGT divider */
+		r = readl(PRCM_ARMCLKFIX_MGT);
+		r &= PRCM_CLK_MGT_CLKPLLDIV_MASK;
+		rate /= r;
+
+	} else {/* ARM PLL */
+		rate = pll_rate(PRCM_PLLARM_FREQ, ROOT_CLOCK_RATE, PLL_DIV);
+	}
+
+	latest_armss_rate = rate;
+}
 
 static unsigned long dsiclk_rate(u8 n)
 {
@@ -1707,6 +1746,8 @@ unsigned long prcmu_clock_rate(u8 clock)
 		return pll_rate(PRCM_PLLSOC0_FREQ, ROOT_CLOCK_RATE, PLL_RAW);
 	else if (clock == PRCMU_PLLSOC1)
 		return pll_rate(PRCM_PLLSOC1_FREQ, ROOT_CLOCK_RATE, PLL_RAW);
+	else if (clock == PRCMU_ARMSS)
+		return armss_rate();
 	else if (clock == PRCMU_PLLDDR)
 		return pll_rate(PRCM_PLLDDR_FREQ, ROOT_CLOCK_RATE, PLL_RAW);
 	else if (clock == PRCMU_PLLDSI)
@@ -2693,6 +2734,7 @@ void __init db8500_prcmu_early_init(void)
 					 handle_simple_irq);
 		set_irq_flags(irq, IRQF_VALID);
 	}
+	compute_armss_rate();
 }
 
 static void __init init_prcm_registers(void)
diff --git a/drivers/mfd/dbx500-prcmu-regs.h b/drivers/mfd/dbx500-prcmu-regs.h
index 23108a6..79c76eb 100644
--- a/drivers/mfd/dbx500-prcmu-regs.h
+++ b/drivers/mfd/dbx500-prcmu-regs.h
@@ -61,7 +61,8 @@
 #define PRCM_PLLARM_LOCKP_PRCM_PLLARM_LOCKP3	0x2
 
 #define PRCM_ARM_CHGCLKREQ	(_PRCMU_BASE + 0x114)
-#define PRCM_ARM_CHGCLKREQ_PRCM_ARM_CHGCLKREQ	0x1
+#define PRCM_ARM_CHGCLKREQ_PRCM_ARM_CHGCLKREQ	BIT(0)
+#define PRCM_ARM_CHGCLKREQ_PRCM_ARM_DIVSEL	BIT(16)
 
 #define PRCM_PLLARM_ENABLE	(_PRCMU_BASE + 0x98)
 #define PRCM_PLLARM_ENABLE_PRCM_PLLARM_ENABLE	0x1
@@ -140,6 +141,7 @@
 /* PRCMU clock/PLL/reset registers */
 #define PRCM_PLLSOC0_FREQ	   (_PRCMU_BASE + 0x080)
 #define PRCM_PLLSOC1_FREQ	   (_PRCMU_BASE + 0x084)
+#define PRCM_PLLARM_FREQ	   (_PRCMU_BASE + 0x088)
 #define PRCM_PLLDDR_FREQ	   (_PRCMU_BASE + 0x08C)
 #define PRCM_PLL_FREQ_D_SHIFT	0
 #define PRCM_PLL_FREQ_D_MASK	BITS(0, 7)
diff --git a/include/linux/mfd/dbx500-prcmu.h b/include/linux/mfd/dbx500-prcmu.h
index 5b90e94..c410d99 100644
--- a/include/linux/mfd/dbx500-prcmu.h
+++ b/include/linux/mfd/dbx500-prcmu.h
@@ -136,6 +136,7 @@ enum prcmu_clock {
 	PRCMU_TIMCLK,
 	PRCMU_PLLSOC0,
 	PRCMU_PLLSOC1,
+	PRCMU_ARMSS,
 	PRCMU_PLLDDR,
 	PRCMU_PLLDSI,
 	PRCMU_DSI0CLK,
-- 
1.7.10

[PATCH 4/4] clk: ux500: Define smp_twd clock for u8500

From: Ulf Hansson <hidden>
Date: 2012-08-31 12:22:22

From: Ulf Hansson <redacted>

The smp_twd clock is based upon a prcmu_rate clock type
for the PRCMU_ARMSS clock.

Signed-off-by: Ulf Hansson <redacted>
---
 drivers/clk/ux500/u8500_clk.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/drivers/clk/ux500/u8500_clk.c b/drivers/clk/ux500/u8500_clk.c
index 5c1fca1..ca4a25e 100644
--- a/drivers/clk/ux500/u8500_clk.c
+++ b/drivers/clk/ux500/u8500_clk.c
@@ -205,12 +205,16 @@ void u8500_clk_init(void)
 	clk_register_clkdev(clk, "dsilp2", "dsilink.2");
 	clk_register_clkdev(clk, "dsilp2", "mcde");
 
+	clk = clk_reg_prcmu_rate("smp_twd", NULL, PRCMU_ARMSS,
+				CLK_IS_ROOT|CLK_GET_RATE_NOCACHE|
+				CLK_IGNORE_UNUSED);
+	clk_register_clkdev(clk, NULL, "smp_twd");
+
 	/*
 	 * FIXME: Add special handled PRCMU clocks here:
-	 * 1. smp_twd, use PRCMU_ARMSS.
-	 * 2. clk_arm, use PRCMU_ARMCLK.
-	 * 3. clkout0yuv, use PRCMU as parent + need regulator + pinctrl.
-	 * 4. ab9540_clkout1yuv, see clkout0yuv
+	 * 1. clk_arm, use PRCMU_ARMCLK.
+	 * 2. clkout0yuv, use PRCMU as parent + need regulator + pinctrl.
+	 * 3. ab9540_clkout1yuv, see clkout0yuv
 	 */
 
 	/* PRCC P-clocks */
-- 
1.7.10

[PATCH 2/4] clk: ux500: Support for prmcu_rate clock

From: Ulf Hansson <hidden>
Date: 2012-08-31 12:22:52

From: Ulf Hansson <redacted>

The prmcu_rate clock is not gateable and has a rate which
only can be fetched.

Signed-off-by: Ulf Hansson <redacted>
---
 drivers/clk/ux500/clk-prcmu.c |   14 ++++++++++++++
 drivers/clk/ux500/clk.h       |    5 +++++
 2 files changed, 19 insertions(+)
diff --git a/drivers/clk/ux500/clk-prcmu.c b/drivers/clk/ux500/clk-prcmu.c
index 1d779ad..930cdfe 100644
--- a/drivers/clk/ux500/clk-prcmu.c
+++ b/drivers/clk/ux500/clk-prcmu.c
@@ -153,6 +153,11 @@ static struct clk_ops clk_prcmu_gate_ops = {
 	.recalc_rate = clk_prcmu_recalc_rate,
 };
 
+static struct clk_ops clk_prcmu_rate_ops = {
+	.is_enabled = clk_prcmu_is_enabled,
+	.recalc_rate = clk_prcmu_recalc_rate,
+};
+
 static struct clk_ops clk_prcmu_opp_gate_ops = {
 	.prepare = clk_prcmu_opp_prepare,
 	.unprepare = clk_prcmu_opp_unprepare,
@@ -228,6 +233,15 @@ struct clk *clk_reg_prcmu_gate(const char *name,
 			&clk_prcmu_gate_ops);
 }
 
+struct clk *clk_reg_prcmu_rate(const char *name,
+			       const char *parent_name,
+			       u8 cg_sel,
+			       unsigned long flags)
+{
+	return clk_reg_prcmu(name, parent_name, cg_sel, 0, flags,
+			&clk_prcmu_rate_ops);
+}
+
 struct clk *clk_reg_prcmu_opp_gate(const char *name,
 				   const char *parent_name,
 				   u8 cg_sel,
diff --git a/drivers/clk/ux500/clk.h b/drivers/clk/ux500/clk.h
index 32085aa..836d7d1 100644
--- a/drivers/clk/ux500/clk.h
+++ b/drivers/clk/ux500/clk.h
@@ -35,6 +35,11 @@ struct clk *clk_reg_prcmu_gate(const char *name,
 			       u8 cg_sel,
 			       unsigned long flags);
 
+struct clk *clk_reg_prcmu_rate(const char *name,
+			       const char *parent_name,
+			       u8 cg_sel,
+			       unsigned long flags);
+
 struct clk *clk_reg_prcmu_opp_gate(const char *name,
 				   const char *parent_name,
 				   u8 cg_sel,
-- 
1.7.10

Re: [PATCH 1/4] clk: Provide option for clk_get_rate to issue hw for new rate

From: Mike Turquette <hidden>
Date: 2012-08-31 19:29:26

Quoting Ulf Hansson (2012-08-31 05:21:28)
From: Ulf Hansson <redacted>

By using CLK_GET_RATE_NOCACHE flag, we tell the clk_get_rate API to
issue the hw for an updated clock rate. This can be used for a clock
which rate may be updated without a client necessary modifying it.
I'm glad to see this.  We discussed whether the default behavior should
be cached or from the hardware at length some time back, so having a
flag to support the non-default is great.
quoted hunk
Signed-off-by: Ulf Hansson <redacted>
---
 drivers/clk/clk.c            |   43 +++++++++++++++++++++++-------------------
 include/linux/clk-provider.h |    1 +
 2 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index efdfd00..d9cbae0 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -558,25 +558,6 @@ int clk_enable(struct clk *clk)
 EXPORT_SYMBOL_GPL(clk_enable);
 
 /**
- * clk_get_rate - return the rate of clk
- * @clk: the clk whose rate is being returned
- *
- * Simply returns the cached rate of the clk.  Does not query the hardware.  If
- * clk is NULL then returns 0.
- */
-unsigned long clk_get_rate(struct clk *clk)
-{
-       unsigned long rate;
-
-       mutex_lock(&prepare_lock);
-       rate = __clk_get_rate(clk);
-       mutex_unlock(&prepare_lock);
-
-       return rate;
-}
-EXPORT_SYMBOL_GPL(clk_get_rate);
-
-/**
  * __clk_round_rate - round the given rate for a clk
  * @clk: round the rate of this clock
  *
@@ -702,6 +683,30 @@ static void __clk_recalc_rates(struct clk *clk, unsigned long msg)
 }
 
 /**
+ * clk_get_rate - return the rate of clk
+ * @clk: the clk whose rate is being returned
+ *
+ * Simply returns the cached rate of the clk, unless CLK_GET_RATE_NOCACHE flag
+ * is set, which means a recalc_rate will be issued.
+ * If clk is NULL then returns 0.
+ */
+unsigned long clk_get_rate(struct clk *clk)
+{
+       unsigned long rate;
+
+       mutex_lock(&prepare_lock);
+
+       if (clk && (clk->flags & CLK_GET_RATE_NOCACHE))
+               __clk_recalc_rates(clk, 0);
This is a bit subtle.  Calling __clk_recalc_rates will walk the subtree
of children recalculating rates as well as firing off notifiers.  Is
this what you want?  If your clock changes rates behind your back AND
has chilren then this is probably the right thing to do.  However you
might be better off with:

	if (clk && (clk->flags & CLK_GET_RATE_NOCACHE))
		rate = clk->ops->recalc_rate(clk->hw, clk->parent->rate);

This doesn't update children or fire off notifiers.  What is best for
your platform?

Regards,
Mike
quoted hunk
+
+       rate = __clk_get_rate(clk);
+       mutex_unlock(&prepare_lock);
+
+       return rate;
+}
+EXPORT_SYMBOL_GPL(clk_get_rate);
+
+/**
  * __clk_speculate_rates
  * @clk: first clk in the subtree
  * @parent_rate: the "future" rate of clk's parent
diff --git a/include/linux/clk-provider.h b/include/linux/clk-provider.h
index 77335fa..1b15307 100644
--- a/include/linux/clk-provider.h
+++ b/include/linux/clk-provider.h
@@ -26,6 +26,7 @@
 #define CLK_IGNORE_UNUSED      BIT(3) /* do not gate even if unused */
 #define CLK_IS_ROOT            BIT(4) /* root clk, has no parent */
 #define CLK_IS_BASIC           BIT(5) /* Basic clk, can't do a to_clk_foo() */
+#define CLK_GET_RATE_NOCACHE   BIT(6) /* do not use the cached clk rate */
 
 struct clk_hw;
 
-- 
1.7.10

Re: [PATCH 0/4] clk: Support for smp_twd clock for ux500

From: Linus Walleij <hidden>
Date: 2012-08-31 23:32:57

On Fri, Aug 31, 2012 at 2:21 PM, Ulf Hansson [off-list ref] wrote:
From: Ulf Hansson <redacted>

To implement support for the smp_twd clock for ux500 several steps
was needed. This patchseries has also proposed some new changes in the
common clock core, which the ux500 smp_twd clock definition are
relying on.
This is a great series,
Acked-by: Linus Walleij <redacted>
For the ux500 parts.

Mike it'd be great if you could merge this once you have the semantics
of [1/4] sorted out with Ulf.

Yours,
Linus Walleij

Re: [PATCH 1/4] clk: Provide option for clk_get_rate to issue hw for new rate

From: Ulf Hansson <hidden>
Date: 2012-09-06 09:09:39

Hi Mike,

Thanks for your input, and sorry for my late reply!

On 31 August 2012 21:29, Mike Turquette [off-list ref] wrote:
Quoting Ulf Hansson (2012-08-31 05:21:28)
quoted
From: Ulf Hansson <redacted>

By using CLK_GET_RATE_NOCACHE flag, we tell the clk_get_rate API to
issue the hw for an updated clock rate. This can be used for a clock
which rate may be updated without a client necessary modifying it.
I'm glad to see this.  We discussed whether the default behavior should
be cached or from the hardware at length some time back, so having a
flag to support the non-default is great.
quoted
Signed-off-by: Ulf Hansson <redacted>
---
 drivers/clk/clk.c            |   43 +++++++++++++++++++++++-------------------
 include/linux/clk-provider.h |    1 +
 2 files changed, 25 insertions(+), 19 deletions(-)
diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index efdfd00..d9cbae0 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -558,25 +558,6 @@ int clk_enable(struct clk *clk)
 EXPORT_SYMBOL_GPL(clk_enable);

 /**
- * clk_get_rate - return the rate of clk
- * @clk: the clk whose rate is being returned
- *
- * Simply returns the cached rate of the clk.  Does not query the hardware.  If
- * clk is NULL then returns 0.
- */
-unsigned long clk_get_rate(struct clk *clk)
-{
-       unsigned long rate;
-
-       mutex_lock(&prepare_lock);
-       rate = __clk_get_rate(clk);
-       mutex_unlock(&prepare_lock);
-
-       return rate;
-}
-EXPORT_SYMBOL_GPL(clk_get_rate);
-
-/**
  * __clk_round_rate - round the given rate for a clk
  * @clk: round the rate of this clock
  *
@@ -702,6 +683,30 @@ static void __clk_recalc_rates(struct clk *clk, unsigned long msg)
 }

 /**
+ * clk_get_rate - return the rate of clk
+ * @clk: the clk whose rate is being returned
+ *
+ * Simply returns the cached rate of the clk, unless CLK_GET_RATE_NOCACHE flag
+ * is set, which means a recalc_rate will be issued.
+ * If clk is NULL then returns 0.
+ */
+unsigned long clk_get_rate(struct clk *clk)
+{
+       unsigned long rate;
+
+       mutex_lock(&prepare_lock);
+
+       if (clk && (clk->flags & CLK_GET_RATE_NOCACHE))
+               __clk_recalc_rates(clk, 0);
This is a bit subtle.  Calling __clk_recalc_rates will walk the subtree
of children recalculating rates as well as firing off notifiers.  Is
this what you want?  If your clock changes rates behind your back AND
has chilren then this is probably the right thing to do.  However you
might be better off with:

        if (clk && (clk->flags & CLK_GET_RATE_NOCACHE))
                rate = clk->ops->recalc_rate(clk->hw, clk->parent->rate);

This doesn't update children or fire off notifiers.  What is best for
your platform?
For my platform, ux500 and for the clock connected to this
patchseries, your suggesting above is enough. (Well some additional
error handling is needed in your code proposal though :-) )

The reason for why I used "__clk_recalc_rates" was because I think it
could make sense to have a more generic approach, not sure if it is
needed as you mention. Additionally, using  __clk_recalc_rates with
"0" as the notification argument, should prevent notifications from
happen, right?

So basically, I wanted the clock rates for the children to be updated
as well as the parent clock rate, but no notifications.

I can happily update the patch according to your proposal if you still
think it is the best way to do it, just tell me again then. :-)

Kind regards
Ulf Hansson

[PATCH 1/4] clk: Provide option for clk_get_rate to issue hw for new rate

From: Mike Turquette <hidden>
Date: 2012-09-07 00:19:27

Quoting Ulf Hansson (2012-09-06 02:09:33)
On 31 August 2012 21:29, Mike Turquette [off-list ref] wrote:
quoted
This is a bit subtle.  Calling __clk_recalc_rates will walk the subtree
of children recalculating rates as well as firing off notifiers.  Is
this what you want?  If your clock changes rates behind your back AND
has chilren then this is probably the right thing to do.  However you
might be better off with:

        if (clk && (clk->flags & CLK_GET_RATE_NOCACHE))
                rate = clk->ops->recalc_rate(clk->hw, clk->parent->rate);

This doesn't update children or fire off notifiers.  What is best for
your platform?
For my platform, ux500 and for the clock connected to this
patchseries, your suggesting above is enough. (Well some additional
error handling is needed in your code proposal though :-) )

The reason for why I used "__clk_recalc_rates" was because I think it
could make sense to have a more generic approach, not sure if it is
needed as you mention. Additionally, using  __clk_recalc_rates with
"0" as the notification argument, should prevent notifications from
happen, right?
You are right.  I didn't catch that when running through this patch the
first time.
So basically, I wanted the clock rates for the children to be updated
as well as the parent clock rate, but no notifications.
This is the answer I was looking for.  You DO want to walk the subtree
of children and recalc the rates.  Since you are the first user of such
a feature I am happy to shape it for your needs ;-)
I can happily update the patch according to your proposal if you still
think it is the best way to do it, just tell me again then. :-)
No your patch does the right thing for your platform and looks sane and
generic for others.  I feel much better about not firing off random
notifiers (which I missed when I reviewed your patch last time).

I'll take this series into clk-next.

Regards,
Mike
Kind regards
Ulf Hansson

Re: [PATCH 1/4] clk: Provide option for clk_get_rate to issue hw for new rate

From: Turquette, Mike <hidden>
Date: 2012-09-07 01:00:33

On Thu, Sep 6, 2012 at 5:19 PM, Mike Turquette [off-list ref] wrote:
I'll take this series into clk-next.
Oops, I forgot to ask about patch #3.  Which tree do you want that to
go through?

Thanks,
Mike
Regards,
Mike
quoted
Kind regards
Ulf Hansson

Re: [PATCH 1/4] clk: Provide option for clk_get_rate to issue hw for new rate

From: Linus Walleij <hidden>
Date: 2012-09-07 08:21:07

On Fri, Sep 7, 2012 at 3:00 AM, Turquette, Mike [off-list ref] wrote:
On Thu, Sep 6, 2012 at 5:19 PM, Mike Turquette [off-list ref] wrote:
quoted
I'll take this series into clk-next.
Oops, I forgot to ask about patch #3.  Which tree do you want that to
go through?
Just take it all through your tree if there are no major conflicts.
Acked-by etc.

Yours,
Linus Walleij

Re: [PATCH 1/4] clk: Provide option for clk_get_rate to issue hw for new rate

From: Ulf Hansson <hidden>
Date: 2012-09-07 12:30:03

On 7 September 2012 10:21, Linus Walleij [off-list ref] wrote:
On Fri, Sep 7, 2012 at 3:00 AM, Turquette, Mike [off-list ref] wrote:
quoted
On Thu, Sep 6, 2012 at 5:19 PM, Mike Turquette [off-list ref] wrote:
quoted
I'll take this series into clk-next.
Oops, I forgot to ask about patch #3.  Which tree do you want that to
go through?
Just take it all through your tree if there are no major conflicts.
Acked-by etc.
Agree, it makes it simpler to go through Mike's clock tree. At least
let's try it out.
I will likely have similar patch series with cross dependencies later
on, so this can be a good first test and hopefully it works.

Although, don't we need an ack by Samuel Ortiz for the patch on mfd?
"mfd: dbx500: Provide a more accurate smp_twd clock"

Or, is it enough with Linus ack since he is the maintainer of that
specific mfd file I have patched?

Kind regards
Ulf Hansson

Re: [PATCH 1/4] clk: Provide option for clk_get_rate to issue hw for new rate

From: Linus Walleij <hidden>
Date: 2012-09-07 21:34:08

On Fri, Sep 7, 2012 at 2:29 PM, Ulf Hansson [off-list ref] wrote:
On 7 September 2012 10:21, Linus Walleij [off-list ref] wrote:
quoted
On Fri, Sep 7, 2012 at 3:00 AM, Turquette, Mike [off-list ref] wrote:
quoted
On Thu, Sep 6, 2012 at 5:19 PM, Mike Turquette [off-list ref] wrote:
quoted
I'll take this series into clk-next.
Oops, I forgot to ask about patch #3.  Which tree do you want that to
go through?
Just take it all through your tree if there are no major conflicts.
Acked-by etc.
Agree, it makes it simpler to go through Mike's clock tree. At least
let's try it out.
I will likely have similar patch series with cross dependencies later
on, so this can be a good first test and hopefully it works.
That will probably hit the next merge window anyway - these
patches will go into v3.7 as it looks, then you can finalize the
next series for v3.8.
Although, don't we need an ack by Samuel Ortiz for the patch on mfd?
"mfd: dbx500: Provide a more accurate smp_twd clock"
Confused, isn't that patch part of the latter series, and not
part of what Mike merged?
Or, is it enough with Linus ack since he is the maintainer of that
specific mfd file I have patched?
That's up to Mike, but I hardly think Sam is going to get very
angry about this if we merge it.

Yours,
Linus Walleij

Re: [PATCH 3/4] mfd: dbx500: Provide a more accurate smp_twd clock

From: Samuel Ortiz <hidden>
Date: 2012-09-19 16:03:01

Hi Ulf,

On Fri, Aug 31, 2012 at 02:21:30PM +0200, Ulf Hansson wrote:
quoted hunk
From: Michel Jaouen <redacted>

The local timer clock is based on ARM subsystem clock. This patch
obtains a more exact value of that clock by reading PRCMU registers.
Using this increases the accuracy of the local timer events.

Signed-off-by: Ulf Hansson <redacted>
Signed-off-by: Rickard Andersson <redacted>
Signed-off-by: Michel Jaouen <redacted>
---
 drivers/mfd/db8500-prcmu.c       |   42 ++++++++++++++++++++++++++++++++++++++
 drivers/mfd/dbx500-prcmu-regs.h  |    4 +++-
 include/linux/mfd/dbx500-prcmu.h |    1 +
 3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
index 4f74529..e7f9539 100644
--- a/drivers/mfd/db8500-prcmu.c
+++ b/drivers/mfd/db8500-prcmu.c
@@ -418,6 +418,9 @@ static struct {
 
 static atomic_t ac_wake_req_state = ATOMIC_INIT(0);
 
+/* Functions definition */
+static void compute_armss_rate(void);
+
I'm not a big fan of forward declarations, but I still applied your patch to
my for-next branch. Thanks.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 3/4] mfd: dbx500: Provide a more accurate smp_twd clock

From: Mike Turquette <hidden>
Date: 2012-09-19 16:42:42

Quoting Samuel Ortiz (2012-09-19 09:02:51)
Hi Ulf,

On Fri, Aug 31, 2012 at 02:21:30PM +0200, Ulf Hansson wrote:
quoted
From: Michel Jaouen <redacted>

The local timer clock is based on ARM subsystem clock. This patch
obtains a more exact value of that clock by reading PRCMU registers.
Using this increases the accuracy of the local timer events.

Signed-off-by: Ulf Hansson <redacted>
Signed-off-by: Rickard Andersson <redacted>
Signed-off-by: Michel Jaouen <redacted>
---
 drivers/mfd/db8500-prcmu.c       |   42 ++++++++++++++++++++++++++++++++++++++
 drivers/mfd/dbx500-prcmu-regs.h  |    4 +++-
 include/linux/mfd/dbx500-prcmu.h |    1 +
 3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
index 4f74529..e7f9539 100644
--- a/drivers/mfd/db8500-prcmu.c
+++ b/drivers/mfd/db8500-prcmu.c
@@ -418,6 +418,9 @@ static struct {
 
 static atomic_t ac_wake_req_state = ATOMIC_INIT(0);
 
+/* Functions definition */
+static void compute_armss_rate(void);
+
I'm not a big fan of forward declarations, but I still applied your patch to
my for-next branch. Thanks.
Samuel,

Not sure if you were Cc'd on the rest of the patches in this series but
after some discussion (see patch #1) I took all four patches into
clk-next, including the MFD change.  This was done first week of
September.  At this point those patches have shifted into my stable
branch that I have promised not to rebase (for arm-soc dependencies).

Are you OK with me taking this patch through the clk-next branch?

Regards,
Mike
Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/

Re: [PATCH 3/4] mfd: dbx500: Provide a more accurate smp_twd clock

From: Samuel Ortiz <hidden>
Date: 2012-09-19 16:56:08

Hi Mike,

On Wed, Sep 19, 2012 at 09:42:31AM -0700, Mike Turquette wrote:
Quoting Samuel Ortiz (2012-09-19 09:02:51)
quoted
Hi Ulf,

On Fri, Aug 31, 2012 at 02:21:30PM +0200, Ulf Hansson wrote:
quoted
From: Michel Jaouen <redacted>

The local timer clock is based on ARM subsystem clock. This patch
obtains a more exact value of that clock by reading PRCMU registers.
Using this increases the accuracy of the local timer events.

Signed-off-by: Ulf Hansson <redacted>
Signed-off-by: Rickard Andersson <redacted>
Signed-off-by: Michel Jaouen <redacted>
---
 drivers/mfd/db8500-prcmu.c       |   42 ++++++++++++++++++++++++++++++++++++++
 drivers/mfd/dbx500-prcmu-regs.h  |    4 +++-
 include/linux/mfd/dbx500-prcmu.h |    1 +
 3 files changed, 46 insertions(+), 1 deletion(-)
diff --git a/drivers/mfd/db8500-prcmu.c b/drivers/mfd/db8500-prcmu.c
index 4f74529..e7f9539 100644
--- a/drivers/mfd/db8500-prcmu.c
+++ b/drivers/mfd/db8500-prcmu.c
@@ -418,6 +418,9 @@ static struct {
 
 static atomic_t ac_wake_req_state = ATOMIC_INIT(0);
 
+/* Functions definition */
+static void compute_armss_rate(void);
+
I'm not a big fan of forward declarations, but I still applied your patch to
my for-next branch. Thanks.
Samuel,

Not sure if you were Cc'd on the rest of the patches in this series but
after some discussion (see patch #1) I took all four patches into
clk-next, including the MFD change.  This was done first week of
September.  At this point those patches have shifted into my stable
branch that I have promised not to rebase (for arm-soc dependencies).

Are you OK with me taking this patch through the clk-next branch?
Yes, I'm fine with it.

Cheers,
Samuel.

-- 
Intel Open Source Technology Centre
http://oss.intel.com/
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help