[PATCH] clk: versatile/impd1: switch order of CLCD clocks

[PATCH] clk: versatile/impd1: switch order of CLCD clocks

[Linus Walleij]

The CLCD clocks are registered in the wrong order: first the
APB clock, then the pixelclock. This should be the other way
around so the driver gets the pixelclock, not the APB clock,
when it asks for the first clock for the block.

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/clk/versatile/clk-impd1.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/versatile/clk-impd1.c b/drivers/clk/versatile/clk-impd1.c
index 1cc1330dc570..5e81408e91d6 100644
--- a/drivers/clk/versatile/clk-impd1.c
+++ b/drivers/clk/versatile/clk-impd1.c
@@ -107,8 +107,9 @@ void integrator_impd1_clk_init(void __iomem *base, unsigned int id)
 	clk = icst_clk_register(NULL, &impd1_icst1_desc, imc->vco1name, NULL,
 				base);
 	imc->vco1clk = clk;
-	imc->clks[0] = clkdev_alloc(pclk, "apb_pclk", "lm%x:01000", id);
-	imc->clks[1] = clkdev_alloc(clk, NULL, "lm%x:01000", id);
+	/* CLCD pixel clock */
+	imc->clks[0] = clkdev_alloc(clk, NULL, "lm%x:01000", id);
+	imc->clks[1] = clkdev_alloc(pclk, "apb_pclk", "lm%x:01000", id);
 
 	/* VCO2 is also called "CLK2" */
 	imc->vco2name = kasprintf(GFP_KERNEL, "lm%x-vco2", id);
-- 
2.4.3

Re: [PATCH] clk: versatile/impd1: switch order of CLCD clocks

[Stephen Boyd]

On 07/29, Linus Walleij wrote:
> The CLCD clocks are registered in the wrong order: first the
> APB clock, then the pixelclock. This should be the other way
> around so the driver gets the pixelclock, not the APB clock,
> when it asks for the first clock for the block.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---

Is this because the consumer driver is calling clk_get() with a
NULL connection id for the pixelclock and then a more specific
connection id for the apb clk? Where's the consumer driver?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] clk: versatile/impd1: switch order of CLCD clocks

[Linus Walleij]

On Fri, Jul 31, 2015 at 8:50 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 07/29, Linus Walleij wrote:
>> The CLCD clocks are registered in the wrong order: first the
>> APB clock, then the pixelclock. This should be the other way
>> around so the driver gets the pixelclock, not the APB clock,
>> when it asks for the first clock for the block.
>>
>> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
>> ---
>
> Is this because the consumer driver is calling clk_get() with a
> NULL connection id for the pixelclock and then a more specific
> connection id for the apb clk?

Yes. Like most PrimeCells do.

> Where's the consumer driver?

The NULL (pixel) clock is taken by:
drivers/video/fbdev/amba-clcd.c

The APB "apb_pclk" clock is taken by:
drivers/amba/bus.c

Yours,
Linus Walleij

Re: [PATCH] clk: versatile/impd1: switch order of CLCD clocks

[Stephen Boyd]

On 07/31, Linus Walleij wrote:
> On Fri, Jul 31, 2015 at 8:50 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 07/29, Linus Walleij wrote:
> >> The CLCD clocks are registered in the wrong order: first the
> >> APB clock, then the pixelclock. This should be the other way
> >> around so the driver gets the pixelclock, not the APB clock,
> >> when it asks for the first clock for the block.
> >>
> >> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> >> ---
> >
> > Is this because the consumer driver is calling clk_get() with a
> > NULL connection id for the pixelclock and then a more specific
> > connection id for the apb clk?
> 
> Yes. Like most PrimeCells do.
> 
> > Where's the consumer driver?
> 
> The NULL (pixel) clock is taken by:
> drivers/video/fbdev/amba-clcd.c
> 
> The APB "apb_pclk" clock is taken by:
> drivers/amba/bus.c
> 

Ok. I'm still confused and concerned that putting the clocks into
the list in a certain order fixes anything. I thought the purpose
of a NULL clkdev connection ID was to make it so that we didn't
match the apb_pclk lookup. Instead we find the second lookup for
the device with the wildcard connection id and use that. 

If this fix is valid, did you want a fixes tag on this so it goes
back to stable?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

Re: [PATCH] clk: versatile/impd1: switch order of CLCD clocks

[Linus Walleij]

On Wed, Aug 12, 2015 at 3:25 AM, Stephen Boyd <sboyd@codeaurora.org> wrote:

> Ok. I'm still confused and concerned that putting the clocks into
> the list in a certain order fixes anything.

Oh maybe it doesn't I could've been mistaken here.
clk_find() should indeed find the right clock.

> If this fix is valid, did you want a fixes tag on this so it goes
> back to stable?

Nah you can drop it, sorry for the fuzz.

Yours,
Linus Walleij