public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] ARMv7: OMAP: Add init function for TWL4030 GBPR1 register
@ 2012-02-29 20:52 Jonathan Solnit
  2012-03-01  8:41 ` Igor Grinberg
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Solnit @ 2012-02-29 20:52 UTC (permalink / raw)
  To: u-boot

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 <jsolnit@gmail.com>
---
 drivers/power/twl4030.c |    8 ++++++++
 include/twl4030.h       |   17 +++++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/drivers/power/twl4030.c b/drivers/power/twl4030.c
index 4a4ddeb..ddaa7e1 100644
--- a/drivers/power/twl4030.c
+++ b/drivers/power/twl4030.c
@@ -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);
+}
diff --git a/include/twl4030.h b/include/twl4030.h
index 9cd32ab..60bfa79 100644
--- a/include/twl4030.h
+++ b/include/twl4030.h
@@ -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 */
-- 
1.7.1

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] ARMv7: OMAP: Add init function for TWL4030 GBPR1 register
  2012-02-29 20:52 [U-Boot] [PATCH] ARMv7: OMAP: Add init function for TWL4030 GBPR1 register Jonathan Solnit
@ 2012-03-01  8:41 ` Igor Grinberg
  2012-03-01 17:47   ` Jonathan Solnit
  0 siblings, 1 reply; 6+ messages in thread
From: Igor Grinberg @ 2012-03-01  8:41 UTC (permalink / raw)
  To: u-boot

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?

> This patch adds a function to restore GBPR1 to
> its default value.
> 
> Signed-off-by: Jonathan Solnit <jsolnit@gmail.com>
> ---
>  drivers/power/twl4030.c |    8 ++++++++
>  include/twl4030.h       |   17 +++++++++++++++++
>  2 files changed, 25 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/power/twl4030.c b/drivers/power/twl4030.c
> index 4a4ddeb..ddaa7e1 100644
> --- a/drivers/power/twl4030.c
> +++ b/drivers/power/twl4030.c
> @@ -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);
> +}
> diff --git a/include/twl4030.h b/include/twl4030.h
> index 9cd32ab..60bfa79 100644
> --- a/include/twl4030.h
> +++ b/include/twl4030.h
> @@ -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 */

-- 
Regards,
Igor.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] ARMv7: OMAP: Add init function for TWL4030 GBPR1 register
  2012-03-01  8:41 ` Igor Grinberg
@ 2012-03-01 17:47   ` Jonathan Solnit
  2012-03-04  7:45     ` Igor Grinberg
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan Solnit @ 2012-03-01 17:47 UTC (permalink / raw)
  To: u-boot

Hi Igor.

On Thu, Mar 1, 2012 at 12:41 AM, Igor Grinberg <grinberg@compulab.co.il>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:

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] ARMv7: OMAP: Add init function for TWL4030 GBPR1 register
  2012-03-01 17:47   ` Jonathan Solnit
@ 2012-03-04  7:45     ` Igor Grinberg
  2012-03-05 19:00       ` Jonathan Solnit
  2012-03-05 19:07       ` Tom Rini
  0 siblings, 2 replies; 6+ messages in thread
From: Igor Grinberg @ 2012-03-04  7:45 UTC (permalink / raw)
  To: u-boot

On 03/01/12 19:47, Jonathan Solnit wrote:
> Hi Igor.
> 
> On Thu, Mar 1, 2012 at 12:41 AM, Igor Grinberg <grinberg at compulab.co.il <mailto:grinberg@compulab.co.il>> 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:
> 
> 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

> 
> 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.


-- 
Regards,
Igor.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] ARMv7: OMAP: Add init function for TWL4030 GBPR1 register
  2012-03-04  7:45     ` Igor Grinberg
@ 2012-03-05 19:00       ` Jonathan Solnit
  2012-03-05 19:07       ` Tom Rini
  1 sibling, 0 replies; 6+ messages in thread
From: Jonathan Solnit @ 2012-03-05 19:00 UTC (permalink / raw)
  To: u-boot

>
>
> 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.


>

--
> Regards,
> Igor.
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH] ARMv7: OMAP: Add init function for TWL4030 GBPR1 register
  2012-03-04  7:45     ` Igor Grinberg
  2012-03-05 19:00       ` Jonathan Solnit
@ 2012-03-05 19:07       ` Tom Rini
  1 sibling, 0 replies; 6+ messages in thread
From: Tom Rini @ 2012-03-05 19:07 UTC (permalink / raw)
  To: u-boot

On Sun, Mar 4, 2012 at 12:45 AM, Igor Grinberg <grinberg@compulab.co.il> wrote:
> On 03/01/12 19:47, Jonathan Solnit wrote:
>> Hi Igor.
>>
>> On Thu, Mar 1, 2012 at 12:41 AM, Igor Grinberg <grinberg at compulab.co.il <mailto:grinberg@compulab.co.il>> 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:
>>
>> 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.

>> 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.

-- 
Tom

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-03-05 19:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-02-29 20:52 [U-Boot] [PATCH] ARMv7: OMAP: Add init function for TWL4030 GBPR1 register Jonathan Solnit
2012-03-01  8:41 ` Igor Grinberg
2012-03-01 17:47   ` Jonathan Solnit
2012-03-04  7:45     ` Igor Grinberg
2012-03-05 19:00       ` Jonathan Solnit
2012-03-05 19:07       ` Tom Rini

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox