Re: [PATCH] clk: respect the clock dependencies in of_clk_init

[Sebastian Hesselbarth]

On 02/07/2014 03:49 PM, Gregory CLEMENT wrote:
> On 07/02/2014 15:43, Ezequiel Garcia wrote:
> > On Fri, Feb 07, 2014 at 09:24:30AM -0500, Jason Cooper wrote:
> > > On Fri, Feb 07, 2014 at 10:06:08AM -0300, Emilio LÃpez wrote:
> > >
> > > [snip a great explanation]
> > >
> > > Guys, can I get some Tested-by's on this?
> >
> > In case someone missed Emilio's comment about it, I gave his oneliner
> > a test on A370 Reference Design. It worked just as well as Sebastian's.
>
> Well ok it's working but this patch is not better than Sebastian, it is
> even worth. I don't think it is a good idea at all to totally ignore the

Tstststs.. Gregory please re-read the above slooooowly.. and think about
where you'd put me in.

Hint: "this patch", "not better than", "even worse". ;)

> information given by the device tree.

Actually, I have no strong opinion about how we fix the issue now. 
Emilio's patch is short and very suitable for a fix. I'll test later
this weekend.

For the long run, I definitely prefer probe ordering within clk frame
work or even better some early_device stuff.

> > Tested-by: Ezequiel Garcia <ezequiel.garcia@xxxxxxxxxxxxxxxxxx>
> >
> > > > -----8<------
> > > >
> > > >  From ffdb49506e3ce92090c15e1f9b37f4d465097ac1 Mon Sep 17 00:00:00 2001
> > > > From: =?UTF-8?q?Emilio=20L=C3=B3pez?= <emilio@xxxxxxxxxxxxx>
> > > > Date: Thu, 6 Feb 2014 18:07:07 -0300
> > > > Subject: [PATCH] clk: mvebu: fix name dependency during registration time
> > > >
> > > > Currently, mvebu_clk_gating_setup has a silly dependency on clock
> > > > registration order just to gather the parent clock name. This is

Of course, the dependency is not silly but we are just ignoring that
some clocks actually have a different parent clock. As long as we are
not interested in the frequency and they all depend on a fixed-clock
it makes no difference. Also, on some gates, we do some tweaking, e.g.
on Dove there is a clock gate for the GBE ip that we loop back to the
GBE PHY gate.. some day I may be so bored to get it straight.

> > > > completely unnecesary, as it supports using an already provided name
> > > > via the clk_gating_soc_desc structs, and we can therefore solve this
> > > > issue with a 69+/- line patch. But, given that the parent name is
> > > > always "tclk" as default-hardcoded on mvebu_coreclk_setup(), we can
> > > > just default-hardcode it here too and get away with solving this
> > > > problem with a one-liner.

I agree with the default hard-coded "tclk" for now. But in general,
clocking can become very nasty and rather than coding the whole fscking
clock tree like the imx clock tree beast does, it would be much more
readable to be able separate the clock drivers into different parts.

Sebastian

> > > > ---
> > > >   drivers/clk/mvebu/common.c | 2 +-
> > > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/clk/mvebu/common.c b/drivers/clk/mvebu/common.c
> > > > index 25ceccf..6c63b43 100644
> > > > --- a/drivers/clk/mvebu/common.c
> > > > +++ b/drivers/clk/mvebu/common.c
> > > > @@ -121,7 +121,7 @@ void __init mvebu_clk_gating_setup(struct
> > > > device_node *np,
> > > >   	struct clk_gating_ctrl *ctrl;
> > > >   	struct clk *clk;
> > > >   	void __iomem *base;
> > > > -	const char *default_parent = NULL;
> > > > +	const char *default_parent = "tclk";
> > > >   	int n;
> > > >
> > > >   	base = of_iomap(np, 0);
> > > > --
> > > > 1.8.5.3


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/