2012-10-30 18:17:27

by Pantelis Antoniou

[permalink] [raw]
Subject: [PATCH] omap2-clk: Add missing lcdc clock definition

Looks like the lcdc clock definition got dropped.
It is required for the LCD controller to work. Reintroduce.

Signed-off-by: Pantelis Antoniou <[email protected]>
---
arch/arm/mach-omap2/clock33xx_data.c | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/arch/arm/mach-omap2/clock33xx_data.c b/arch/arm/mach-omap2/clock33xx_data.c
index 1a45d6b..b7b7995 100644
--- a/arch/arm/mach-omap2/clock33xx_data.c
+++ b/arch/arm/mach-omap2/clock33xx_data.c
@@ -867,6 +867,16 @@ static struct clk lcd_gclk = {
.recalc = &followparent_recalc,
};

+static struct clk lcdc_fck = {
+ .name = "lcdc_fck",
+ .clkdm_name = "lcdc_clkdm",
+ .parent = &lcd_gclk,
+ .enable_reg = AM33XX_CM_PER_LCDC_CLKCTRL,
+ .enable_bit = AM33XX_MODULEMODE_SWCTRL,
+ .ops = &clkops_omap2_dflt,
+ .recalc = &followparent_recalc,
+};
+
static struct clk mmc_clk = {
.name = "mmc_clk",
.clkdm_name = "l4ls_clkdm",
@@ -1075,6 +1085,7 @@ static struct omap_clk am33xx_clks[] = {
CLK(NULL, "clkout2_ck", &clkout2_ck, CK_AM33XX),
CLK(NULL, "timer_32k_ck", &clkdiv32k_ick, CK_AM33XX),
CLK(NULL, "timer_sys_ck", &sys_clkin_ck, CK_AM33XX),
+ CLK("da8xx_lcdc.0", NULL, &lcdc_fck, CK_AM33XX),
};

int __init am33xx_clk_init(void)
--
1.7.12


2012-10-30 18:18:44

by Tony Lindgren

[permalink] [raw]
Subject: Re: [PATCH] omap2-clk: Add missing lcdc clock definition

* Pantelis Antoniou <[email protected]> [121030 11:04]:
> Looks like the lcdc clock definition got dropped.
> It is required for the LCD controller to work. Reintroduce.

This looks like a regression, can you also add the commit
causing it?

Regards,

Tony

> Signed-off-by: Pantelis Antoniou <[email protected]>
> ---
> arch/arm/mach-omap2/clock33xx_data.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/arch/arm/mach-omap2/clock33xx_data.c b/arch/arm/mach-omap2/clock33xx_data.c
> index 1a45d6b..b7b7995 100644
> --- a/arch/arm/mach-omap2/clock33xx_data.c
> +++ b/arch/arm/mach-omap2/clock33xx_data.c
> @@ -867,6 +867,16 @@ static struct clk lcd_gclk = {
> .recalc = &followparent_recalc,
> };
>
> +static struct clk lcdc_fck = {
> + .name = "lcdc_fck",
> + .clkdm_name = "lcdc_clkdm",
> + .parent = &lcd_gclk,
> + .enable_reg = AM33XX_CM_PER_LCDC_CLKCTRL,
> + .enable_bit = AM33XX_MODULEMODE_SWCTRL,
> + .ops = &clkops_omap2_dflt,
> + .recalc = &followparent_recalc,
> +};
> +
> static struct clk mmc_clk = {
> .name = "mmc_clk",
> .clkdm_name = "l4ls_clkdm",
> @@ -1075,6 +1085,7 @@ static struct omap_clk am33xx_clks[] = {
> CLK(NULL, "clkout2_ck", &clkout2_ck, CK_AM33XX),
> CLK(NULL, "timer_32k_ck", &clkdiv32k_ick, CK_AM33XX),
> CLK(NULL, "timer_sys_ck", &sys_clkin_ck, CK_AM33XX),
> + CLK("da8xx_lcdc.0", NULL, &lcdc_fck, CK_AM33XX),
> };
>
> int __init am33xx_clk_init(void)
> --
> 1.7.12
>

2012-10-30 23:26:43

by Paul Walmsley

[permalink] [raw]
Subject: Re: [PATCH] omap2-clk: Add missing lcdc clock definition

+ Vaibhav Hiremath

On Tue, 30 Oct 2012, Tony Lindgren wrote:

> * Pantelis Antoniou <[email protected]> [121030 11:04]:
> > Looks like the lcdc clock definition got dropped.
> > It is required for the LCD controller to work. Reintroduce.
>
> This looks like a regression, can you also add the commit
> causing it?

Looks like probably a new "feature," in that this clock didn't exist in
the original check-in. Would be good to get Vaibhav's opinion on this;
also the common clock patches will need to be updated.


- Paul

2012-10-31 05:09:41

by Hiremath, Vaibhav

[permalink] [raw]
Subject: RE: [PATCH] omap2-clk: Add missing lcdc clock definition

On Wed, Oct 31, 2012 at 04:56:40, Paul Walmsley wrote:
> + Vaibhav Hiremath
>
> On Tue, 30 Oct 2012, Tony Lindgren wrote:
>
> > * Pantelis Antoniou <[email protected]> [121030 11:04]:
> > > Looks like the lcdc clock definition got dropped.
> > > It is required for the LCD controller to work. Reintroduce.
> >
> > This looks like a regression, can you also add the commit
> > causing it?
>
> Looks like probably a new "feature," in that this clock didn't exist in
> the original check-in. Would be good to get Vaibhav's opinion on this;
> also the common clock patches will need to be updated.
>

Thanks Paul for looping me in, something went wrong with my l-o subscription,
so I didn't receive these Patches.

As far as lck clock node is concerned, we had deliberately dropped all leaf-
node clocks from the clock tree, please refer to the description mentioned
in -
http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/101987.html


>From LCDC driver perspective, driver is using,

fb_clk = clk_get(&device->dev, NULL);

This I feel needs to be corrected for valid name as per Spec (mostly I would
vote for "fck") and then every platform should make sure that it returns
valid clock-node for it.

Change in Driver would be,

fb_clk = clk_get(&device->dev, "fck");


Thanks,
Vaibhav

>
> - Paul
>

2012-10-31 05:49:50

by Paul Walmsley

[permalink] [raw]
Subject: RE: [PATCH] omap2-clk: Add missing lcdc clock definition

On Wed, 31 Oct 2012, Hiremath, Vaibhav wrote:

> As far as lck clock node is concerned, we had deliberately dropped all leaf-
> node clocks from the clock tree, please refer to the description mentioned
> in -
> http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/101987.html

Ach, should have remembered that :-( Indeed there is an LCDC hwmod:

static struct omap_hwmod am33xx_lcdc_hwmod = {
.name = "lcdc",
.class = &am33xx_lcdc_hwmod_class,
.clkdm_name = "lcdc_clkdm",
.mpu_irqs = am33xx_lcdc_irqs,
.flags = HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY,
.main_clk = "lcd_gclk",
.prcm = {
.omap4 = {
.clkctrl_offs = AM33XX_CM_PER_LCDC_CLKCTRL_OFFSET,
.modulemode = MODULEMODE_SWCTRL,
},
},
};

> >From LCDC driver perspective, driver is using,
>
> fb_clk = clk_get(&device->dev, NULL);
>
> This I feel needs to be corrected for valid name as per Spec (mostly I would
> vote for "fck") and then every platform should make sure that it returns
> valid clock-node for it.
>
> Change in Driver would be,
>
> fb_clk = clk_get(&device->dev, "fck");

Indeed.


- Paul

2012-10-31 06:38:42

by Hiremath, Vaibhav

[permalink] [raw]
Subject: RE: [PATCH] omap2-clk: Add missing lcdc clock definition

On Wed, Oct 31, 2012 at 11:19:44, Paul Walmsley wrote:
> On Wed, 31 Oct 2012, Hiremath, Vaibhav wrote:
>
> > As far as lck clock node is concerned, we had deliberately dropped all leaf-
> > node clocks from the clock tree, please refer to the description mentioned
> > in -
> > http://lists.infradead.org/pipermail/linux-arm-kernel/2012-May/101987.html
>
> Ach, should have remembered that :-( Indeed there is an LCDC hwmod:
>
> static struct omap_hwmod am33xx_lcdc_hwmod = {
> .name = "lcdc",
> .class = &am33xx_lcdc_hwmod_class,
> .clkdm_name = "lcdc_clkdm",
> .mpu_irqs = am33xx_lcdc_irqs,
> .flags = HWMOD_SWSUP_SIDLE | HWMOD_SWSUP_MSTANDBY,
> .main_clk = "lcd_gclk",
> .prcm = {
> .omap4 = {
> .clkctrl_offs = AM33XX_CM_PER_LCDC_CLKCTRL_OFFSET,
> .modulemode = MODULEMODE_SWCTRL,
> },
> },
> };
>
> > >From LCDC driver perspective, driver is using,
> >
> > fb_clk = clk_get(&device->dev, NULL);
> >
> > This I feel needs to be corrected for valid name as per Spec (mostly I would
> > vote for "fck") and then every platform should make sure that it returns
> > valid clock-node for it.
> >
> > Change in Driver would be,
> >
> > fb_clk = clk_get(&device->dev, "fck");
>

Ok, thanks. Let me submit patch for this.

Thanks,
Vaibhav

> Indeed.
>
>
> - Paul
>