Restore Integrator/CP CLCD to fully working state

PATCH New Jonathan Austin 3 participants 13 messages Jul 23, 2013 – Aug 28, 2013

Subsystems (4)

Arm (32-bit) Arm sub-architectures Common clk Arm integrator, versatile and realview support

lore

Share & tools

lore

Jonathan Austin

author

2013-07-23 15:42 UTC #0 lore raw

cover Restore Integrator/CP CLCD to fully working state

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(-)

Signature · 1 line

1.7.9.5

Jonathan Austin

author

2013-07-23 15:42 UTC #1 lore raw

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

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 <[email protected]>

Signed-off-by Jonathan Austin <[email protected]>

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

arch/arm/mach-integrator/integrator_cp.c +2 -1

index 8c60fcb..2f4c9248 100644
@@ -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))

Signature · 1 line

1.7.9.5

Jonathan Austin

author

2013-08-15 15:41 UTC #2 lore raw

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

Hi Linus,

Quoted 3 lines · 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.

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

Jonny

Quoted 21 lines

Cc Linus Walleij <[email protected]>

Signed-off-by Jonathan Austin <[email protected]>

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

arch/arm/mach-integrator/integrator_cp.c +2 -1

index 8c60fcb..2f4c9248 100644
@@ -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))

Linus Walleij

2013-08-28 07:29 UTC #3 lore raw

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

In reply to Jonathan Austin

Quoted 5 lines · 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 <[email protected]>
Signed-off-by Jonathan Austin <[email protected]>

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 <[email protected]>

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

Jonathan Austin

author

2013-07-23 15:42 UTC #4 lore raw

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

In reply to 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 <[email protected]>

Cc Mike Turquette <[email protected]>

Signed-off-by Jonathan Austin <[email protected]>

---
 drivers/clk/versatile/clk-icst.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/clk/versatile/clk-icst.c +1 -1

index 67ccf4a..f5e4c21 100644
@@ -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;
 }

Signature · 1 line

1.7.9.5

Linus Walleij

2013-07-24 19:32 UTC #5 lore raw

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

Quoted 10 lines · 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 <[email protected]>
Cc Mike Turquette <[email protected]>
Signed-off-by Jonathan Austin <[email protected]>

Good catch! Reviewed-by: Linus Walleij <[email protected]> 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

Jonathan Austin

author

2013-07-25 09:30 UTC #6 lore raw

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

Quoted 8 lines · Linus Walleij

Quoted 5 lines · 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.

Quoted 9 lines

Quoted 5 lines
This patch fixes the order and restores the expected functionality.
Cc Linus Walleij <[email protected]>
Cc Mike Turquette <[email protected]>
Signed-off-by Jonathan Austin <[email protected]>
Good catch! Reviewed-by: Linus Walleij <[email protected]> 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?

Quoted 3 lines

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?

Quoted 2 lines

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

Quoted 4 lines

Yours,
Linus Walleij

Linus Walleij

2013-07-25 09:33 UTC #7 lore raw

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

Quoted 6 lines · Jonathan Austin

Quoted 3 lines
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

Jonathan Austin

author

2013-07-25 09:36 UTC #8 lore raw

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

Quoted 10 lines · Linus Walleij

Quoted 6 lines · Jonathan Austin
Quoted 3 lines
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?

Quoted 9 lines

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

Linus Walleij

2013-07-25 14:01 UTC #9 lore raw

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

Quoted 11 lines · Jonathan Austin

Quoted 5 lines · Linus Walleij
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

Mike Turquette

2013-07-25 17:44 UTC #10 lore raw

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

In reply to Jonathan Austin

Quoting Jonathan Austin (2013-07-25 02:30:29)

Quoted 15 lines · Linus Walleij

Quoted 14 lines
Quoted 10 lines · 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 <[email protected]>
Cc Mike Turquette <[email protected]>
Signed-off-by Jonathan Austin <[email protected]>

Quoted 6 lines

Quoted 3 lines
Good catch! Reviewed-by: Linus Walleij <[email protected]> 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

Jonathan Austin

author

2013-07-29 11:01 UTC #11 lore raw

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

Quoted 16 lines · Mike Turquette

Quoting Jonathan Austin (2013-07-25 02:30:29)
Quoted 15 lines · Linus Walleij
Quoted 14 lines
Quoted 10 lines · 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 <[email protected]>
Cc Mike Turquette <[email protected]>
Signed-off-by Jonathan Austin <[email protected]>

Quoted 9 lines

Quoted 6 lines
Quoted 3 lines
Good catch! Reviewed-by: Linus Walleij <[email protected]> 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

Mike Turquette

2013-07-24 23:26 UTC #12 lore raw

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

In reply to Jonathan Austin

Quoting Jonathan Austin (2013-07-23 08:42:18)

Quoted 10 lines

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 <[email protected]>
Cc Mike Turquette <[email protected]>
Signed-off-by Jonathan Austin <[email protected]>

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

Regards,
Mike

Quoted 19 lines

---
 drivers/clk/versatile/clk-icst.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/clk/versatile/clk-icst.c +1 -1

index 67ccf4a..f5e4c21 100644
@@ -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;
 }

Signature · 1 line

1.7.9.5