ARMv7: OMAP: Add init function for TWL4030 GBPR1 register

PATCH New Jonathan Solnit 3 participants 6 messages Feb 29, 2012 – Mar 5, 2012

Share & tools

Jonathan Solnit

author

2012-02-29 20:52 UTC #0 lore raw

PATCH 1/1 ARMv7: OMAP: Add init function for TWL4030 GBPR1 register

The OMAP ROM code modifies the GBPR1 register, which can cause unintended consequences. This patch adds a function to restore GBPR1 to its default value.

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

---
 drivers/power/twl4030.c |    8 ++++++++
 include/twl4030.h       |   17 +++++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

drivers/power/twl4030.c +8 -0

index 4a4ddeb..ddaa7e1 100644
@@ -103,3 +103,11 @@ void twl4030_power_mmc_init(void)
 				TWL4030_PM_RECEIVER_VMMC1_DEV_GRP,
 				TWL4030_PM_RECEIVER_DEV_GRP_P1);
 }
+
+void twl4030_madc_clk_init(void)
+{
+	/* Restore the default MADC clk */
+	twl4030_i2c_write_u8(TWL4030_CHIP_INTBR,
+				TWL4030_INTBR_GPBR1_MADC_HFCLK_EN | TWL4030_INTBR_GPBR1_DFLT_MADC_CLK_EN,
+				TWL4030_INTBR_GPBR1);
+}

include/twl4030.h +17 -0

index 9cd32ab..60bfa79 100644
@@ -482,6 +482,18 @@
 #define TWL4030_USB_PHY_CLK_CTRL			0xFE
 #define TWL4030_USB_PHY_CLK_CTRL_STS			0xFF
 
+/* General Purpose */
+#define TWL4030_INTBR_GPBR1						0x91
+
+#define TWL4030_INTBR_GPBR1_MADC_HFCLK_EN		(0x1 << 7)
+#define TWL4030_INTBR_GPBR1_MADC_3MHZ_EN		(0x1 << 6)
+#define TWL4030_INTBR_GPBR1_VBAT_MON_EN			(0x1 << 5)
+#define TWL4030_INTBR_GPBR1_DFLT_MADC_CLK_EN	(0x1 << 4)
+#define TWL4030_INTBR_GPBR1_PWM1_EN				(0x1 << 3)
+#define TWL4030_INTBR_GPBR1_PWM0_EN				(0x1 << 2)
+#define TWL4030_INTBR_GPBR1_PWM1_CLK_EN			(0x1 << 1)
+#define TWL4030_INTBR_GPBR1_PWM0_CLK_EN			(0x1 << 0)
+
 /*
  * Convience functions to read and write from TWL4030
  *

@@ -530,4 +542,9 @@ void twl4030_led_init(unsigned char ledon_mask);
  */
 int twl4030_usb_ulpi_init(void);
 
+/*
+ * MADC Clock
+ */
+void twl4030_madc_clk_init(void);
+
 #endif /* TWL4030_H */

Signature · 1 line

1.7.1

Igor Grinberg

2012-03-01 08:41 UTC #1 lore raw

PATCH 1/1 ARMv7: OMAP: Add init function for TWL4030 GBPR1 register

Hi Jonathan,

Quoted 1 line · Jonathan Solnit

The OMAP ROM code modifies the GBPR1 register, which can cause

s/GBPR1/GPBR1/

Quoted 1 line

unintended consequences.

What do you mean by this?
Can you please elaborate, what issues do you see? Also, why does the OMAP ROM code needs to touch the GPBR1?

Quoted 58 lines

This patch adds a function to restore GBPR1 to
its default value.

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

---
 drivers/power/twl4030.c |    8 ++++++++
 include/twl4030.h       |   17 +++++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

drivers/power/twl4030.c +8 -0

index 4a4ddeb..ddaa7e1 100644
@@ -103,3 +103,11 @@ void twl4030_power_mmc_init(void)
 				TWL4030_PM_RECEIVER_VMMC1_DEV_GRP,
 				TWL4030_PM_RECEIVER_DEV_GRP_P1);
 }
+
+void twl4030_madc_clk_init(void)
+{
+	/* Restore the default MADC clk */
+	twl4030_i2c_write_u8(TWL4030_CHIP_INTBR,
+				TWL4030_INTBR_GPBR1_MADC_HFCLK_EN | TWL4030_INTBR_GPBR1_DFLT_MADC_CLK_EN,
+				TWL4030_INTBR_GPBR1);
+}

include/twl4030.h +17 -0

index 9cd32ab..60bfa79 100644
@@ -482,6 +482,18 @@
 #define TWL4030_USB_PHY_CLK_CTRL			0xFE
 #define TWL4030_USB_PHY_CLK_CTRL_STS			0xFF
 
+/* General Purpose */
+#define TWL4030_INTBR_GPBR1						0x91
+
+#define TWL4030_INTBR_GPBR1_MADC_HFCLK_EN		(0x1 << 7)
+#define TWL4030_INTBR_GPBR1_MADC_3MHZ_EN		(0x1 << 6)
+#define TWL4030_INTBR_GPBR1_VBAT_MON_EN			(0x1 << 5)
+#define TWL4030_INTBR_GPBR1_DFLT_MADC_CLK_EN	(0x1 << 4)
+#define TWL4030_INTBR_GPBR1_PWM1_EN				(0x1 << 3)
+#define TWL4030_INTBR_GPBR1_PWM0_EN				(0x1 << 2)
+#define TWL4030_INTBR_GPBR1_PWM1_CLK_EN			(0x1 << 1)
+#define TWL4030_INTBR_GPBR1_PWM0_CLK_EN			(0x1 << 0)
+
 /*
  * Convience functions to read and write from TWL4030
  *

@@ -530,4 +542,9 @@ void twl4030_led_init(unsigned char ledon_mask);
  */
 int twl4030_usb_ulpi_init(void);
 
+/*
+ * MADC Clock
+ */
+void twl4030_madc_clk_init(void);
+
 #endif /* TWL4030_H */

Signature · 2 lines

Regards,
Igor.

Jonathan Solnit

author

2012-03-01 17:47 UTC #2 lore raw

PATCH 1/1 ARMv7: OMAP: Add init function for TWL4030 GBPR1 register

Hi Igor.

Quoted 5 lines · Igor Grinberg

Hi Jonathan,
Quoted 1 line · Jonathan Solnit
The OMAP ROM code modifies the GBPR1 register, which can cause

Quoted 9 lines

s/GBPR1/GPBR1/
Quoted 1 line
unintended consequences.
What do you mean by this?
Can you please elaborate, what issues do you see? Also, why does the OMAP ROM code needs to touch the GPBR1?

For my board, when booting the OMAP3 from MMC1, GPBR1 comes up as 0x00 instead of 0x90. The improper clock configuration causes timeouts whenever I try to use the MADC. In Linux, the error looks like this:

user.err kernel: twl4030_madc twl4030_madc: conversion timeout!

As for why ROM touches GPBR1, only TI can answer that. What I've found is that this is a known issue and re-initializing the register at start-up is the recommended solution:

http://e2e.ti.com/support/power_management/pmu/f/43/t/762.aspx

Thanks,
Jonathan

Igor Grinberg

2012-03-04 07:45 UTC #3 lore raw

PATCH 1/1 ARMv7: OMAP: Add init function for TWL4030 GBPR1 register

Quoted 13 lines · Jonathan Solnit

Hi Igor.
On Thu, Mar 1, 2012 at 12:41 AM, Igor Grinberg <grinberg at compulab.co.il <mailto:[email protected]>> wrote:
Hi Jonathan,
On 02/29/12 22:52, Jonathan Solnit wrote:
> The OMAP ROM code modifies the GBPR1 register, which can cause
s/GBPR1/GPBR1/
> unintended consequences.

Quoted 8 lines

What do you mean by this?
Can you please elaborate, what issues do you see? Also, why does the OMAP ROM code needs to touch the GPBR1?
For my board, when booting the OMAP3 from MMC1, GPBR1 comes up as 0x00 instead of 0x90. The improper clock configuration causes timeouts whenever I try to use the MADC. In Linux, the error looks like this:
user.err kernel: twl4030_madc twl4030_madc: conversion timeout!

Right, but isn't this has been already fixed in kernel:
3d6271f (mfd: Turn on the twl4030-madc MADC clock)

$ git describe 3d6271f
v3.1-13-g3d6271f

Quoted 4 lines

As for why ROM touches GPBR1, only TI can answer that. What I've found is that this is a known issue and re-initializing the register at start-up is the recommended solution:
http://e2e.ti.com/support/power_management/pmu/f/43/t/762.aspx

We would really appreciate if someone from TI (Tom?) can confirm that this is a bug in ROM code. Nevertheless, I don't think U-Boot uses MADC feature (am I right?),
so IMO, kernel has to be fixed (I think already fixed) to not rely
on ROM/U-Boot setup of that MADC clock.

Signature · 2 lines

Regards,
Igor.

Jonathan Solnit

author

2012-03-05 19:00 UTC #4 lore raw

PATCH 1/1 ARMv7: OMAP: Add init function for TWL4030 GBPR1 register

Quoted 9 lines

We would really appreciate if someone from TI (Tom?) can confirm that this is a bug in ROM code.
Nevertheless, I don't think U-Boot uses MADC feature (am I right?), so IMO, kernel has to be fixed (I think already fixed) to not rely on ROM/U-Boot setup of that MADC clock.

I can say for sure that in my application I'm not using the MADC clock until booting Linux.

Since there's already something in the kernel addressing the issue (at least setting bit 7, I think I need bit 4 set as well), I'm okay with withdrawing this patch.

Quoted 1 line

Quoted 3 lines

Regards,
Igor.

Tom Rini

2012-03-05 19:07 UTC #5 lore raw

PATCH 1/1 ARMv7: OMAP: Add init function for TWL4030 GBPR1 register

In reply to Igor Grinberg

Quoted 21 lines · Igor Grinberg

Quoted 20 lines · Jonathan Solnit
Hi Igor.
On Thu, Mar 1, 2012 at 12:41 AM, Igor Grinberg <grinberg at compulab.co.il <mailto:[email protected]>> wrote:
? ? Hi Jonathan,
? ? On 02/29/12 22:52, Jonathan Solnit wrote:
? ? > The OMAP ROM code modifies the GBPR1 register, which can cause
? ? s/GBPR1/GPBR1/
? ? > unintended consequences.
? ? What do you mean by this?
? ? Can you please elaborate, what issues do you see? ? ? Also, why does the OMAP ROM code needs to touch the GPBR1?
For my board, when booting the OMAP3 from MMC1, GPBR1 comes up as 0x00 instead of 0x90. ?The improper clock configuration causes timeouts whenever I try to use the MADC. ?In Linux, the error looks like this:

Quoted 7 lines

Quoted 1 line
user.err kernel: twl4030_madc twl4030_madc: conversion timeout!
Right, but isn't this has been already fixed in kernel:
3d6271f (mfd: Turn on the twl4030-madc MADC clock)
$ git describe 3d6271f
v3.1-13-g3d6271f

OK, good (also good given Jonathan's follow-up). In this case, I'm going to go with just having this changed in the kernel unless someone steps up with a case where this is a problem for U-Boot itself.

Quoted 5 lines

Quoted 3 lines
As for why ROM touches GPBR1, only TI can answer that. ?What I've found is that this is a known issue and re-initializing the register at start-up is the recommended solution:
http://e2e.ti.com/support/power_management/pmu/f/43/t/762.aspx
We would really appreciate if someone from TI (Tom?) can confirm that this is a bug in ROM code.

My suspicion is that the "re-init in SW" note is the final answer to the problem, from the ROM PoV.

Signature · 1 line

Tom