[PATCH 0/2] Restore Integrator/CP CLCD to fully working state

[PATCH 0/2] Restore Integrator/CP CLCD to fully working state

[Jonathan Austin]

Over time and with refactoring, the use of the Integrator/CP LCD seems to have
broken.

This series restores the ability to use the Integrator's CLCD and an external
VGA display.

Jonathan Austin (2):
  ARM: integrator_cp: Set LCD{0,1} enable lines when turning on CLCD
  clk: fixup argument order when setting VCO parameters

 arch/arm/mach-integrator/integrator_cp.c |    3 ++-
 drivers/clk/versatile/clk-icst.c         |    2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

-- 
1.7.9.5

[PATCH 1/2] ARM: integrator_cp: Set LCD{0, 1} enable lines when turning on CLCD

[Jonathan Austin]

This turns on the internal integrator LCD display(s). It seems that the code
to do this got lost in refactoring of the CLCD driver.

Cc: Linus Walleij <linus.walleij@linaro.org>
Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>
---
 arch/arm/mach-integrator/integrator_cp.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-integrator/integrator_cp.c b/arch/arm/mach-integrator/integrator_cp.c
index 8c60fcb..2f4c9248 100644
--- a/arch/arm/mach-integrator/integrator_cp.c
+++ b/arch/arm/mach-integrator/integrator_cp.c
@@ -199,7 +199,8 @@ static struct mmci_platform_data mmc_data = {
 static void cp_clcd_enable(struct clcd_fb *fb)
 {
 	struct fb_var_screeninfo *var = &fb->fb.var;
-	u32 val = CM_CTRL_STATIC1 | CM_CTRL_STATIC2;
+	u32 val = CM_CTRL_STATIC1 | CM_CTRL_STATIC2
+			| CM_CTRL_LCDEN0 | CM_CTRL_LCDEN1;
 
 	if (var->bits_per_pixel <= 8 ||
 	    (var->bits_per_pixel == 16 && var->green.length == 5))
-- 
1.7.9.5

[PATCH 2/2] clk: fixup argument order when setting VCO parameters

[Jonathan Austin]

The order of arguments in the call to vco_set() for the ICST clocks appears to
have been switched in error, which results in the VCO not being initialised
correctly. This in turn stops the integrated LCD on things like Integrator/CP
from working correctly.

This patch fixes the order and restores the expected functionality.

Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Mike Turquette <mturquette@linaro.org>
Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>
---
 drivers/clk/versatile/clk-icst.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/clk/versatile/clk-icst.c b/drivers/clk/versatile/clk-icst.c
index 67ccf4a..f5e4c21 100644
--- a/drivers/clk/versatile/clk-icst.c
+++ b/drivers/clk/versatile/clk-icst.c
@@ -107,7 +107,7 @@ static int icst_set_rate(struct clk_hw *hw, unsigned long rate,
 
 	vco = icst_hz_to_vco(icst->params, rate);
 	icst->rate = icst_hz(icst->params, vco);
-	vco_set(icst->vcoreg, icst->lockreg, vco);
+	vco_set(icst->lockreg, icst->vcoreg, vco);
 	return 0;
 }
 
-- 
1.7.9.5

[PATCH 2/2] clk: fixup argument order when setting VCO parameters

[Linus Walleij]

On Tue, Jul 23, 2013 at 5:42 PM, Jonathan Austin
<jonathan.austin@arm.com> wrote:

> The order of arguments in the call to vco_set() for the ICST clocks appears to
> have been switched in error, which results in the VCO not being initialised
> correctly. This in turn stops the integrated LCD on things like Integrator/CP
> from working correctly.
>
> This patch fixes the order and restores the expected functionality.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Mike Turquette <mturquette@linaro.org>
> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>

Good catch! Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
You should probably mark this for stable as well.

What I don't understand is why my CLCD VGA output has
been working all the time :-(

I thought this VCO was necessary for that as well,  but apparently
it works with the defaults.

Yours,
Linus Walleij

[PATCH 2/2] clk: fixup argument order when setting VCO parameters

[Mike Turquette]

Quoting Jonathan Austin (2013-07-23 08:42:18)
> The order of arguments in the call to vco_set() for the ICST clocks appears to
> have been switched in error, which results in the VCO not being initialised
> correctly. This in turn stops the integrated LCD on things like Integrator/CP
> from working correctly.
> 
> This patch fixes the order and restores the expected functionality.
> 
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Cc: Mike Turquette <mturquette@linaro.org>
> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>

Thanks for the fix. Will send out with the next batch.

Regards,
Mike

> ---
>  drivers/clk/versatile/clk-icst.c |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/versatile/clk-icst.c b/drivers/clk/versatile/clk-icst.c
> index 67ccf4a..f5e4c21 100644
> --- a/drivers/clk/versatile/clk-icst.c
> +++ b/drivers/clk/versatile/clk-icst.c
> @@ -107,7 +107,7 @@ static int icst_set_rate(struct clk_hw *hw, unsigned long rate,
>  
>         vco = icst_hz_to_vco(icst->params, rate);
>         icst->rate = icst_hz(icst->params, vco);
> -       vco_set(icst->vcoreg, icst->lockreg, vco);
> +       vco_set(icst->lockreg, icst->vcoreg, vco);
>         return 0;
>  }
>  
> -- 
> 1.7.9.5

[PATCH 2/2] clk: fixup argument order when setting VCO parameters

[Jonathan Austin]

On 24/07/13 20:32, Linus Walleij wrote:
> On Tue, Jul 23, 2013 at 5:42 PM, Jonathan Austin
> <jonathan.austin@arm.com> wrote:
>
>> The order of arguments in the call to vco_set() for the ICST clocks appears to
>> have been switched in error, which results in the VCO not being initialised
>> correctly. This in turn stops the integrated LCD on things like Integrator/CP
>> from working correctly.
>>
>> This patch fixes the order and restores the expected functionality.
>>
>> Cc: Linus Walleij <linus.walleij@linaro.org>
>> Cc: Mike Turquette <mturquette@linaro.org>
>> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>
>
> Good catch! Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> You should probably mark this for stable as well.
>

Mike, will you add Cc: stable when you take the fix or shall I resubmit 
the patch somehow else?

> What I don't understand is why my CLCD VGA output has
> been working all the time :-(
>

No, this confuses me too - perhaps the display you've got it plugged in 
to is really lenient about modes and just made the best of it?

> I thought this VCO was necessary for that as well,  but apparently
> it works with the defaults.

Certainly I can't get anything out of the display on my integrator or 
via the VGA port without this fix and the other one in the series.

Jonny
>
> Yours,
> Linus Walleij
>

[PATCH 2/2] clk: fixup argument order when setting VCO parameters

[Linus Walleij]

On Thu, Jul 25, 2013 at 11:30 AM, Jonathan Austin
<jonathan.austin@arm.com> wrote:

>> What I don't understand is why my CLCD VGA output has
>> been working all the time :-(
>>
>
> No, this confuses me too - perhaps the display you've got it plugged in to
> is really lenient about modes and just made the best of it?

I think it's because it is set up into some mode by the boot
monitor, as I get the boot monitor prompt on the VGA
display.

Then as the VCO params get written, it is never really unlocked
so the registers are untouched.

You may have a different boot monitor?

Yours,
Linus Walleij

[PATCH 2/2] clk: fixup argument order when setting VCO parameters

[Jonathan Austin]

On 25/07/13 10:33, Linus Walleij wrote:
> On Thu, Jul 25, 2013 at 11:30 AM, Jonathan Austin
> <jonathan.austin@arm.com> wrote:
>
>>> What I don't understand is why my CLCD VGA output has
>>> been working all the time :-(
>>>
>>
>> No, this confuses me too - perhaps the display you've got it plugged in to
>> is really lenient about modes and just made the best of it?
>
> I think it's because it is set up into some mode by the boot
> monitor, as I get the boot monitor prompt on the VGA
> display.
>
> Then as the VCO params get written, it is never really unlocked
> so the registers are untouched.
>
> You may have a different boot monitor?
>

Ahh, that seems highly likely. I don't ever see boot-monitor (boot 
straight in to uboot) but I don't see anything on the display until the 
kernel boots, so I don't think BM is setting up the VCO for me.

Jonny

[PATCH 2/2] clk: fixup argument order when setting VCO parameters

[Linus Walleij]

On Thu, Jul 25, 2013 at 11:36 AM, Jonathan Austin
<jonathan.austin@arm.com> wrote:
> On 25/07/13 10:33, Linus Walleij wrote:

>> Then as the VCO params get written, it is never really unlocked
>> so the registers are untouched.
>>
>> You may have a different boot monitor?
>>
>
> Ahh, that seems highly likely. I don't ever see boot-monitor (boot straight
> in to uboot) but I don't see anything on the display until the kernel boots,
> so I don't think BM is setting up the VCO for me.

True, if you boot directly into U-boot the boot monitor is
never executed and the mode is never set up.

Yours,
Linus Walleij

[PATCH 2/2] clk: fixup argument order when setting VCO parameters

[Mike Turquette]

Quoting Jonathan Austin (2013-07-25 02:30:29)
> On 24/07/13 20:32, Linus Walleij wrote:
> > On Tue, Jul 23, 2013 at 5:42 PM, Jonathan Austin
> > <jonathan.austin@arm.com> wrote:
> >
> >> The order of arguments in the call to vco_set() for the ICST clocks appears to
> >> have been switched in error, which results in the VCO not being initialised
> >> correctly. This in turn stops the integrated LCD on things like Integrator/CP
> >> from working correctly.
> >>
> >> This patch fixes the order and restores the expected functionality.
> >>
> >> Cc: Linus Walleij <linus.walleij@linaro.org>
> >> Cc: Mike Turquette <mturquette@linaro.org>
> >> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>
> >
> > Good catch! Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> > You should probably mark this for stable as well.
> >
> 
> Mike, will you add Cc: stable when you take the fix or shall I resubmit 
> the patch somehow else?

I'll add it.

Regards,
Mike

[PATCH 2/2] clk: fixup argument order when setting VCO parameters

[Jonathan Austin]

On 25/07/13 18:44, Mike Turquette wrote:
> Quoting Jonathan Austin (2013-07-25 02:30:29)
>> On 24/07/13 20:32, Linus Walleij wrote:
>>> On Tue, Jul 23, 2013 at 5:42 PM, Jonathan Austin
>>> <jonathan.austin@arm.com> wrote:
>>>
>>>> The order of arguments in the call to vco_set() for the ICST clocks appears to
>>>> have been switched in error, which results in the VCO not being initialised
>>>> correctly. This in turn stops the integrated LCD on things like Integrator/CP
>>>> from working correctly.
>>>>
>>>> This patch fixes the order and restores the expected functionality.
>>>>
>>>> Cc: Linus Walleij <linus.walleij@linaro.org>
>>>> Cc: Mike Turquette <mturquette@linaro.org>
>>>> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>
>>>
>>> Good catch! Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
>>> You should probably mark this for stable as well.
>>>
>>
>> Mike, will you add Cc: stable when you take the fix or shall I resubmit
>> the patch somehow else?
>
> I'll add it.
>

Thanks very much.

Linus or Mike: Should I put 1/2 in this series in to Russell's tree, or 
should that go in elsewise?

Jonny

[PATCH 1/2] ARM: integrator_cp: Set LCD{0,1} enable lines when turning on CLCD

[Jonathan Austin]

Hi Linus,

On 23/07/13 16:42, Jonathan Austin wrote:
> This turns on the internal integrator LCD display(s). It seems that the code
> to do this got lost in refactoring of the CLCD driver.
>

Assuming this is okay, should this go through you or the patch system?

Jonny
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>
> ---
>   arch/arm/mach-integrator/integrator_cp.c |    3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/mach-integrator/integrator_cp.c b/arch/arm/mach-integrator/integrator_cp.c
> index 8c60fcb..2f4c9248 100644
> --- a/arch/arm/mach-integrator/integrator_cp.c
> +++ b/arch/arm/mach-integrator/integrator_cp.c
> @@ -199,7 +199,8 @@ static struct mmci_platform_data mmc_data = {
>   static void cp_clcd_enable(struct clcd_fb *fb)
>   {
>   	struct fb_var_screeninfo *var = &fb->fb.var;
> -	u32 val = CM_CTRL_STATIC1 | CM_CTRL_STATIC2;
> +	u32 val = CM_CTRL_STATIC1 | CM_CTRL_STATIC2
> +			| CM_CTRL_LCDEN0 | CM_CTRL_LCDEN1;
>
>   	if (var->bits_per_pixel <= 8 ||
>   	    (var->bits_per_pixel == 16 && var->green.length == 5))
>

[PATCH 1/2] ARM: integrator_cp: Set LCD{0,1} enable lines when turning on CLCD

[Linus Walleij]

On Tue, Jul 23, 2013 at 5:42 PM, Jonathan Austin
<jonathan.austin@arm.com> wrote:

> This turns on the internal integrator LCD display(s). It seems that the code
> to do this got lost in refactoring of the CLCD driver.
>
> Cc: Linus Walleij <linus.walleij@linaro.org>
> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>

I wonder if these bits should really be set if you have only VGA?
I don't think it hurts but I'm a bit picky with the details...

Anyway since it's a regression:
Acked-by: Linus Walleij <linus.walleij@linaro.org>

You can send it to arm at kernel.org as a fix and ask that the
ARM SoC maintainers apply it directly, also add
Cc: stable at vger.kernel.org so the fix is applied backward.

Yours,
Linus Walleij

[PATCH 1/2] ARM: integrator_cp: Set LCD{0, 1} enable lines when turning on CLCD

[Jonathan Austin]

This turns on the internal integrator LCD display(s). It seems that the code
to do this got lost in refactoring of the CLCD driver.

Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>
Acked-by: Linus Walleij <linus.walleij@linaro.org>
Cc: stable at vger.kernel.org

---
...resending this patch to arm at kernel.org as suggested by Linus W with this Ack

 arch/arm/mach-integrator/integrator_cp.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/arm/mach-integrator/integrator_cp.c b/arch/arm/mach-integrator/integrator_cp.c
index 8c60fcb..2f4c9248 100644
--- a/arch/arm/mach-integrator/integrator_cp.c
+++ b/arch/arm/mach-integrator/integrator_cp.c
@@ -199,7 +199,8 @@ static struct mmci_platform_data mmc_data = {
 static void cp_clcd_enable(struct clcd_fb *fb)
 {
 	struct fb_var_screeninfo *var = &fb->fb.var;
-	u32 val = CM_CTRL_STATIC1 | CM_CTRL_STATIC2;
+	u32 val = CM_CTRL_STATIC1 | CM_CTRL_STATIC2
+			| CM_CTRL_LCDEN0 | CM_CTRL_LCDEN1;
 
 	if (var->bits_per_pixel <= 8 ||
 	    (var->bits_per_pixel == 16 && var->green.length == 5))
-- 
1.7.9.5

[PATCH 1/2] ARM: integrator_cp: Set LCD{0,1} enable lines when turning on CLCD

[Olof Johansson]

On Thu, Aug 29, 2013 at 06:41:11PM +0100, Jonathan Austin wrote:
> This turns on the internal integrator LCD display(s). It seems that the code
> to do this got lost in refactoring of the CLCD driver.
> 
> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>
> Acked-by: Linus Walleij <linus.walleij@linaro.org>
> Cc: stable at vger.kernel.org
> 
> ---
> ...resending this patch to arm at kernel.org as suggested by Linus W with this Ack

Applied to fixes.


-Olof

[PATCH 1/2] ARM: integrator_cp: Set LCD{0,1} enable lines when turning on CLCD

[Jonathan Austin]

Hi Olof,

On 10/09/13 01:35, Olof Johansson wrote:
> On Thu, Aug 29, 2013 at 06:41:11PM +0100, Jonathan Austin wrote:
>> This turns on the internal integrator LCD display(s). It seems that the code
>> to do this got lost in refactoring of the CLCD driver.
>>
>> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>
>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>> Cc: stable at vger.kernel.org
>>
>> ---
>> ...resending this patch to arm at kernel.org as suggested by Linus W with this Ack
>
> Applied to fixes.
>

How will this go in? I poked in your arm-soc fixes branches and couldn't 
see it there. Did I miss it somewhere or did it fall out?

Jonny

>
> -Olof
>

[PATCH 1/2] ARM: integrator_cp: Set LCD{0,1} enable lines when turning on CLCD

[Olof Johansson]

On Mon, Nov 11, 2013 at 10:40 AM, Jonathan Austin
<jonathan.austin@arm.com> wrote:
> Hi Olof,
>
>
> On 10/09/13 01:35, Olof Johansson wrote:
>>
>> On Thu, Aug 29, 2013 at 06:41:11PM +0100, Jonathan Austin wrote:
>>>
>>> This turns on the internal integrator LCD display(s). It seems that the
>>> code
>>> to do this got lost in refactoring of the CLCD driver.
>>>
>>> Signed-off-by: Jonathan Austin <jonathan.austin@arm.com>
>>> Acked-by: Linus Walleij <linus.walleij@linaro.org>
>>> Cc: stable at vger.kernel.org
>>>
>>> ---
>>> ...resending this patch to arm at kernel.org as suggested by Linus W with
>>> this Ack
>>
>>
>> Applied to fixes.
>>
>
> How will this go in? I poked in your arm-soc fixes branches and couldn't see
> it there. Did I miss it somewhere or did it fall out?


Hmm, odd. Yes, it did fall out. I'll re-apply it now for the next set
of patches we're sending up. Sorry about that.


-Olof