[v9,01/10] clk: fix initial state of critical clock's parents
diff mbox

Message ID 20160810210922.GD2996@codeaurora.org
State New
Headers show

Commit Message

Stephen Boyd Aug. 10, 2016, 9:09 p.m. UTC
(Including lists)

On 08/09, James Liao wrote:
> On Wed, 2016-08-03 at 13:46 +0800, James Liao wrote:
>>
>> Hi Mike,
>>
>> Do you have new patches to fix new clock parents? If not, I think we can
>> use my patch first. Is it okay?
>>
>
> Hi Stephen,
>
> Do you have comments for the bug fixing? I prefer to use my patch (clk:
> fix initial state of critical clock's parents). How do you think?
>

How about we recalc accuracies and rates in addition to the patch
from Mike? That will fix everything?

---8<----
From: Michael Turquette <mturquette@baylibre.com>
Subject: [PATCH] clk: migrate ref counts when orphans are reunited

It's always nice to see families reunited, and this is equally true when
talking about parent clocks and their children. However, if the orphan
clk had a positive prepare_count or enable_count, then we would not
migrate those counts up the parent chain correctly.

This has manifested with the recent critical clocks feature, which often
enables clocks very early, before their parents have been registered.

Fixed by replacing the call to clk_core_reparent with calls to
__clk_set_parent_{before,after}.

Cc: James Liao <jamesjj.liao@mediatek.com>
Cc: Erin Lo <erin.lo@mediatek.com>
Signed-off-by: Michael Turquette <mturquette@baylibre.com>
[sboyd@codeaurora.org: Recalc accuracies and rates too]
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 drivers/clk/clk.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

Comments

James Liao Aug. 12, 2016, 8:17 a.m. UTC | #1
On Wed, 2016-08-10 at 14:09 -0700, Stephen Boyd wrote:
> (Including lists)
> 
> On 08/09, James Liao wrote:
> > On Wed, 2016-08-03 at 13:46 +0800, James Liao wrote:
> >>
> >> Hi Mike,
> >>
> >> Do you have new patches to fix new clock parents? If not, I think we can
> >> use my patch first. Is it okay?
> >>
> >
> > Hi Stephen,
> >
> > Do you have comments for the bug fixing? I prefer to use my patch (clk:
> > fix initial state of critical clock's parents). How do you think?
> >
> 
> How about we recalc accuracies and rates in addition to the patch
> from Mike? That will fix everything?

Hi Stephen,

It works!

I'll send a new series of MT2701 clock support in few days. Should I
include this patch in my series? Or you'll merge it into clk-next
directly?


Best regards,

James

> ---8<----
> From: Michael Turquette <mturquette@baylibre.com>
> Subject: [PATCH] clk: migrate ref counts when orphans are reunited
> 
> It's always nice to see families reunited, and this is equally true when
> talking about parent clocks and their children. However, if the orphan
> clk had a positive prepare_count or enable_count, then we would not
> migrate those counts up the parent chain correctly.
> 
> This has manifested with the recent critical clocks feature, which often
> enables clocks very early, before their parents have been registered.
> 
> Fixed by replacing the call to clk_core_reparent with calls to
> __clk_set_parent_{before,after}.
> 
> Cc: James Liao <jamesjj.liao@mediatek.com>
> Cc: Erin Lo <erin.lo@mediatek.com>
> Signed-off-by: Michael Turquette <mturquette@baylibre.com>
> [sboyd@codeaurora.org: Recalc accuracies and rates too]
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  drivers/clk/clk.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
> index 820a939fb6bb..dc3fff2bf839 100644
> --- a/drivers/clk/clk.c
> +++ b/drivers/clk/clk.c
> @@ -2449,8 +2449,16 @@ static int __clk_core_init(struct clk_core *core)
>  	hlist_for_each_entry_safe(orphan, tmp2, &clk_orphan_list, child_node) {
>  		struct clk_core *parent = __clk_init_parent(orphan);
>  
> -		if (parent)
> -			clk_core_reparent(orphan, parent);
> +		/*
> +		 * we could call __clk_set_parent, but that would result in a
> +		 * reducant call to the .set_rate op, if it exists
> +		 */
> +		if (parent) {
> +			__clk_set_parent_before(orphan, parent);
> +			__clk_set_parent_after(orphan, parent, NULL);
> +			__clk_recalc_accuracies(orphan);
> +			__clk_recalc_rates(orphan, 0);
> +		}
>  	}
>  
>  	/*
Stephen Boyd Aug. 13, 2016, 12:39 a.m. UTC | #2
On 08/12, James Liao wrote:
> On Wed, 2016-08-10 at 14:09 -0700, Stephen Boyd wrote:
> > (Including lists)
> > 
> > On 08/09, James Liao wrote:
> > > On Wed, 2016-08-03 at 13:46 +0800, James Liao wrote:
> > >>
> > >> Hi Mike,
> > >>
> > >> Do you have new patches to fix new clock parents? If not, I think we can
> > >> use my patch first. Is it okay?
> > >>
> > >
> > > Hi Stephen,
> > >
> > > Do you have comments for the bug fixing? I prefer to use my patch (clk:
> > > fix initial state of critical clock's parents). How do you think?
> > >
> > 
> > How about we recalc accuracies and rates in addition to the patch
> > from Mike? That will fix everything?
> 
> Hi Stephen,
> 
> It works!
> 
> I'll send a new series of MT2701 clock support in few days. Should I
> include this patch in my series? Or you'll merge it into clk-next
> directly?
> 

Thanks. I can take that as a tested-by? I can merge it into
clk-next directly, but do we need to put the mt2701 patches on a
separate branch to merge into arm-soc? If so we'll need to put
this patch first to avoid bisection failures.
James Liao Aug. 15, 2016, 2:47 a.m. UTC | #3
Hi Stephen,

On Fri, 2016-08-12 at 17:39 -0700, Stephen Boyd wrote:
> On 08/12, James Liao wrote:
> > On Wed, 2016-08-10 at 14:09 -0700, Stephen Boyd wrote:
> > > (Including lists)
> > > 
> > > On 08/09, James Liao wrote:
> > > > On Wed, 2016-08-03 at 13:46 +0800, James Liao wrote:
> > > >>
> > > >> Hi Mike,
> > > >>
> > > >> Do you have new patches to fix new clock parents? If not, I think we can
> > > >> use my patch first. Is it okay?
> > > >>
> > > >
> > > > Hi Stephen,
> > > >
> > > > Do you have comments for the bug fixing? I prefer to use my patch (clk:
> > > > fix initial state of critical clock's parents). How do you think?
> > > >
> > > 
> > > How about we recalc accuracies and rates in addition to the patch
> > > from Mike? That will fix everything?
> > 
> > Hi Stephen,
> > 
> > It works!
> > 
> > I'll send a new series of MT2701 clock support in few days. Should I
> > include this patch in my series? Or you'll merge it into clk-next
> > directly?
> > 
> 
> Thanks. I can take that as a tested-by? I can merge it into

Yes, please feel free to add:

Tested-by: James Liao <jamesjj.liao@mediatek.com>

> clk-next directly, but do we need to put the mt2701 patches on a
> separate branch to merge into arm-soc? If so we'll need to put
> this patch first to avoid bisection failures.

I prefer to merge clk driver into mainline first. Patches that depend on
mt2701 clks such as dtsi can base on next kernel release.


Best regards,

James

Patch
diff mbox

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 820a939fb6bb..dc3fff2bf839 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -2449,8 +2449,16 @@  static int __clk_core_init(struct clk_core *core)
 	hlist_for_each_entry_safe(orphan, tmp2, &clk_orphan_list, child_node) {
 		struct clk_core *parent = __clk_init_parent(orphan);
 
-		if (parent)
-			clk_core_reparent(orphan, parent);
+		/*
+		 * we could call __clk_set_parent, but that would result in a
+		 * reducant call to the .set_rate op, if it exists
+		 */
+		if (parent) {
+			__clk_set_parent_before(orphan, parent);
+			__clk_set_parent_after(orphan, parent, NULL);
+			__clk_recalc_accuracies(orphan);
+			__clk_recalc_rates(orphan, 0);
+		}
 	}
 
 	/*