[PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups

[PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups

[Pais, Allen]

>From 4044fcc9c517e86fbea9f7d3b15d5cf75a767476 Mon Sep 17 00:00:00 2001
From: Allen Pais <allen.pais@ti.com>
Date: Thu, 21 Jan 2010 21:00:04 +0530
Subject: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups

This patch disables GAIA I2C1 adn I2C4(SR) internal pull-up, to
use only the external HW resistor >=470 Ohm for the assured
functionality in HS mode.

While testing the I2C in High Speed mode, it was discovered that
without a proper pull-up resistor, there is data corruption during
multi-byte transfer. RTC(time_set) test case was used for testing.

>From the analysis done, it was concluded that ideally we need a
pull-up of 1.6k Ohm(recomended) or atleast 470 Ohm or greater for
assured performance in HS mode.

Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>
Signed-off-by: Allen Pais <allen.pais@ti.com>
---
 drivers/mfd/twl-core.c  |   10 ++++++++++
 include/linux/i2c/twl.h |   13 +++++++++++++
 2 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 2a76065..7dcb5e9 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -965,6 +965,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 	int				status;
 	unsigned			i;
 	struct twl4030_platform_data	*pdata = client->dev.platform_data;
+	u8 temp;
 
 	if (!pdata) {
 		dev_dbg(&client->dev, "no platform data?\n");
@@ -1031,6 +1032,15 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
 		if (status < 0)
 			goto fail;
 	}
+	/* Disable GAIA I2C Pull-up on I2C1 and I2C4(SR) interface
+	 * program I2C_SCL_CTRL_PU(bit 0)=0, I2C_SDA_CTRL_PU (bit 2)=0,
+	 * SR_I2C_SCL_CTRL_PU(bit 4)=0 and SR_I2C_SDA_CTRL_PU(bit 6)=0.
+	 */
+
+	twl_i2c_read_u8(TWL4030_MODULE_INTBR, &temp, REG_GPPUPDCTR1);
+	temp &= ~(SR_I2C_SDA_CTRL_PU | SR_I2C_SCL_CTRL_PU | \
+				I2C_SDA_CTRL_PU | I2C_SCL_CTRL_PU);
+	twl_i2c_write_u8(TWL4030_MODULE_INTBR, temp, REG_GPPUPDCTR1);
 
 	status = add_children(pdata, id->driver_data);
 fail:
diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
index bf1c5be..2f4faf9 100644
--- a/include/linux/i2c/twl.h
+++ b/include/linux/i2c/twl.h
@@ -168,6 +168,19 @@ int twl_i2c_read(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes);
 int twl6030_interrupt_unmask(u8 bit_mask, u8 offset);
 int twl6030_interrupt_mask(u8 bit_mask, u8 offset);
 
+/*Interface Bit Register (INTBR) offsets
+ *(Use TWL_4030_MODULE_INTBR)
+ */
+
+#define REG_GPPUPDCTR1			0x0F
+
+/*I2C1 and I2C4(SR) SDA/SCL pull-up control bits */
+
+#define I2C_SCL_CTRL_PU			BIT(0)
+#define I2C_SDA_CTRL_PU			BIT(2)
+#define SR_I2C_SCL_CTRL_PU		BIT(4)
+#define SR_I2C_SDA_CTRL_PU		BIT(6)
+
 /*----------------------------------------------------------------------*/
 
 /*
-- 
1.6.3.3

Re: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups

[Tony Lindgren]

Hi,

* Pais, Allen <allen.pais@ti.com> [100121 02:31]:
> From 4044fcc9c517e86fbea9f7d3b15d5cf75a767476 Mon Sep 17 00:00:00 2001
> From: Allen Pais <allen.pais@ti.com>
> Date: Thu, 21 Jan 2010 21:00:04 +0530
> Subject: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups
> 
> This patch disables GAIA I2C1 adn I2C4(SR) internal pull-up, to
> use only the external HW resistor >=470 Ohm for the assured
> functionality in HS mode.
> 
> While testing the I2C in High Speed mode, it was discovered that
> without a proper pull-up resistor, there is data corruption during
> multi-byte transfer. RTC(time_set) test case was used for testing.
> 
> From the analysis done, it was concluded that ideally we need a
> pull-up of 1.6k Ohm(recomended) or atleast 470 Ohm or greater for
> assured performance in HS mode.

Does this apply to 3630 only, or also 34xx? Is this safe to do
always?

BTW, once ready it hould be sent to Samuel Ortiz with linux-omap
list Cc'd:

$ grep -A4 MFD MAINTAINERS 
MULTIFUNCTION DEVICES (MFD)
M:      Samuel Ortiz <sameo@linux.intel.com>
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/sameo/mfd-2.6.git
S:      Supported
F:      drivers/mfd/

Regards,

Tony

 
> Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>
> Signed-off-by: Allen Pais <allen.pais@ti.com>
> ---
>  drivers/mfd/twl-core.c  |   10 ++++++++++
>  include/linux/i2c/twl.h |   13 +++++++++++++
>  2 files changed, 23 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index 2a76065..7dcb5e9 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -965,6 +965,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  	int				status;
>  	unsigned			i;
>  	struct twl4030_platform_data	*pdata = client->dev.platform_data;
> +	u8 temp;
>  
>  	if (!pdata) {
>  		dev_dbg(&client->dev, "no platform data?\n");
> @@ -1031,6 +1032,15 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
>  		if (status < 0)
>  			goto fail;
>  	}
> +	/* Disable GAIA I2C Pull-up on I2C1 and I2C4(SR) interface
> +	 * program I2C_SCL_CTRL_PU(bit 0)=0, I2C_SDA_CTRL_PU (bit 2)=0,
> +	 * SR_I2C_SCL_CTRL_PU(bit 4)=0 and SR_I2C_SDA_CTRL_PU(bit 6)=0.
> +	 */
> +
> +	twl_i2c_read_u8(TWL4030_MODULE_INTBR, &temp, REG_GPPUPDCTR1);
> +	temp &= ~(SR_I2C_SDA_CTRL_PU | SR_I2C_SCL_CTRL_PU | \
> +				I2C_SDA_CTRL_PU | I2C_SCL_CTRL_PU);
> +	twl_i2c_write_u8(TWL4030_MODULE_INTBR, temp, REG_GPPUPDCTR1);
>  
>  	status = add_children(pdata, id->driver_data);
>  fail:
> diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
> index bf1c5be..2f4faf9 100644
> --- a/include/linux/i2c/twl.h
> +++ b/include/linux/i2c/twl.h
> @@ -168,6 +168,19 @@ int twl_i2c_read(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes);
>  int twl6030_interrupt_unmask(u8 bit_mask, u8 offset);
>  int twl6030_interrupt_mask(u8 bit_mask, u8 offset);
>  
> +/*Interface Bit Register (INTBR) offsets
> + *(Use TWL_4030_MODULE_INTBR)
> + */
> +
> +#define REG_GPPUPDCTR1			0x0F
> +
> +/*I2C1 and I2C4(SR) SDA/SCL pull-up control bits */
> +
> +#define I2C_SCL_CTRL_PU			BIT(0)
> +#define I2C_SDA_CTRL_PU			BIT(2)
> +#define SR_I2C_SCL_CTRL_PU		BIT(4)
> +#define SR_I2C_SDA_CTRL_PU		BIT(6)
> +
>  /*----------------------------------------------------------------------*/
>  
>  /*
> -- 
> 1.6.3.3
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups

[Pais, Allen]

________________________________________
From: Tony Lindgren [tony@atomide.com]
Sent: Monday, February 01, 2010 7:53 PM
To: Pais, Allen
Cc: linux-omap@vger.kernel.org
Subject: Re: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups

Hi,

* Pais, Allen <allen.pais@ti.com> [100121 02:31]:
> From 4044fcc9c517e86fbea9f7d3b15d5cf75a767476 Mon Sep 17 00:00:00 2001
> From: Allen Pais <allen.pais@ti.com>
> Date: Thu, 21 Jan 2010 21:00:04 +0530
> Subject: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups
>
> This patch disables GAIA I2C1 adn I2C4(SR) internal pull-up, to
> use only the external HW resistor >=470 Ohm for the assured
> functionality in HS mode.
>
> While testing the I2C in High Speed mode, it was discovered that
> without a proper pull-up resistor, there is data corruption during
> multi-byte transfer. RTC(time_set) test case was used for testing.
>
> From the analysis done, it was concluded that ideally we need a
> pull-up of 1.6k Ohm(recomended) or atleast 470 Ohm or greater for
> assured performance in HS mode.

Does this apply to 3630 only, or also 34xx? Is this safe to do
always?

[Allen] Yes, it does apply to 36xx only. 

BTW, once ready it hould be sent to Samuel Ortiz with linux-omap
list Cc'd:

[Allen] i'll have it sent to Samuel also.

Thanks,

- Allen

$ grep -A4 MFD MAINTAINERS
MULTIFUNCTION DEVICES (MFD)
M:      Samuel Ortiz <sameo@linux.intel.com>
T:      git git://git.kernel.org/pub/scm/linux/kernel/git/sameo/mfd-2.6.git
S:      Supported
F:      drivers/mfd/

Regards,

Tony


> Signed-off-by: Moiz Sonasath <m-sonasath@ti.com>
> Signed-off-by: Allen Pais <allen.pais@ti.com>
> ---
>  drivers/mfd/twl-core.c  |   10 ++++++++++
>  include/linux/i2c/twl.h |   13 +++++++++++++
>  2 files changed, 23 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index 2a76065..7dcb5e9 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -965,6 +965,7 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
>       int                             status;
>       unsigned                        i;
>       struct twl4030_platform_data    *pdata = client->dev.platform_data;
> +     u8 temp;
>
>       if (!pdata) {
>               dev_dbg(&client->dev, "no platform data?\n");
> @@ -1031,6 +1032,15 @@ twl_probe(struct i2c_client *client, const struct i2c_device_id *id)
>               if (status < 0)
>                       goto fail;
>       }
> +     /* Disable GAIA I2C Pull-up on I2C1 and I2C4(SR) interface
> +      * program I2C_SCL_CTRL_PU(bit 0)=0, I2C_SDA_CTRL_PU (bit 2)=0,
> +      * SR_I2C_SCL_CTRL_PU(bit 4)=0 and SR_I2C_SDA_CTRL_PU(bit 6)=0.
> +      */
> +
> +     twl_i2c_read_u8(TWL4030_MODULE_INTBR, &temp, REG_GPPUPDCTR1);
> +     temp &= ~(SR_I2C_SDA_CTRL_PU | SR_I2C_SCL_CTRL_PU | \
> +                             I2C_SDA_CTRL_PU | I2C_SCL_CTRL_PU);
> +     twl_i2c_write_u8(TWL4030_MODULE_INTBR, temp, REG_GPPUPDCTR1);
>
>       status = add_children(pdata, id->driver_data);
>  fail:
> diff --git a/include/linux/i2c/twl.h b/include/linux/i2c/twl.h
> index bf1c5be..2f4faf9 100644
> --- a/include/linux/i2c/twl.h
> +++ b/include/linux/i2c/twl.h
> @@ -168,6 +168,19 @@ int twl_i2c_read(u8 mod_no, u8 *value, u8 reg, unsigned num_bytes);
>  int twl6030_interrupt_unmask(u8 bit_mask, u8 offset);
>  int twl6030_interrupt_mask(u8 bit_mask, u8 offset);
>
> +/*Interface Bit Register (INTBR) offsets
> + *(Use TWL_4030_MODULE_INTBR)
> + */
> +
> +#define REG_GPPUPDCTR1                       0x0F
> +
> +/*I2C1 and I2C4(SR) SDA/SCL pull-up control bits */
> +
> +#define I2C_SCL_CTRL_PU                      BIT(0)
> +#define I2C_SDA_CTRL_PU                      BIT(2)
> +#define SR_I2C_SCL_CTRL_PU           BIT(4)
> +#define SR_I2C_SDA_CTRL_PU           BIT(6)
> +
>  /*----------------------------------------------------------------------*/
>
>  /*
> --
> 1.6.3.3
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups

[Tony Lindgren]

* Pais, Allen <allen.pais@ti.com> [100201 18:56]:
> 
> ________________________________________
> From: Tony Lindgren [tony@atomide.com]
> Sent: Monday, February 01, 2010 7:53 PM
> To: Pais, Allen
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups
> 
> Hi,
> 
> * Pais, Allen <allen.pais@ti.com> [100121 02:31]:
> > From 4044fcc9c517e86fbea9f7d3b15d5cf75a767476 Mon Sep 17 00:00:00 2001
> > From: Allen Pais <allen.pais@ti.com>
> > Date: Thu, 21 Jan 2010 21:00:04 +0530
> > Subject: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups
> >
> > This patch disables GAIA I2C1 adn I2C4(SR) internal pull-up, to
> > use only the external HW resistor >=470 Ohm for the assured
> > functionality in HS mode.
> >
> > While testing the I2C in High Speed mode, it was discovered that
> > without a proper pull-up resistor, there is data corruption during
> > multi-byte transfer. RTC(time_set) test case was used for testing.
> >
> > From the analysis done, it was concluded that ideally we need a
> > pull-up of 1.6k Ohm(recomended) or atleast 470 Ohm or greater for
> > assured performance in HS mode.
> 
> Does this apply to 3630 only, or also 34xx? Is this safe to do
> always?
> 
> [Allen] Yes, it does apply to 36xx only. 

Sounds like then this configuration should be passed from the
board-*.c file in platform_data as the external pulls depend
on the board.
 
> BTW, once ready it hould be sent to Samuel Ortiz with linux-omap
> list Cc'd:
> 
> [Allen] i'll have it sent to Samuel also.

Thanks, we can't merge it yet though, see above.

Regards,

Tony

RE: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups

[Sonasath, Moiz]

Tony/Allen/Paul,

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Tony Lindgren
> Sent: Tuesday, February 02, 2010 10:56 AM
> To: Pais, Allen
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups
> 
> * Pais, Allen <allen.pais@ti.com> [100201 18:56]:
> >
> > ________________________________________
> > From: Tony Lindgren [tony@atomide.com]
> > Sent: Monday, February 01, 2010 7:53 PM
> > To: Pais, Allen
> > Cc: linux-omap@vger.kernel.org
> > Subject: Re: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups
> >
> > Hi,
> >
> > * Pais, Allen <allen.pais@ti.com> [100121 02:31]:
> > > From 4044fcc9c517e86fbea9f7d3b15d5cf75a767476 Mon Sep 17 00:00:00 2001
> > > From: Allen Pais <allen.pais@ti.com>
> > > Date: Thu, 21 Jan 2010 21:00:04 +0530
> > > Subject: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups
> > >
> > > This patch disables GAIA I2C1 adn I2C4(SR) internal pull-up, to
> > > use only the external HW resistor >=470 Ohm for the assured
> > > functionality in HS mode.
> > >
> > > While testing the I2C in High Speed mode, it was discovered that
> > > without a proper pull-up resistor, there is data corruption during
> > > multi-byte transfer. RTC(time_set) test case was used for testing.
> > >
> > > From the analysis done, it was concluded that ideally we need a
> > > pull-up of 1.6k Ohm(recomended) or atleast 470 Ohm or greater for
> > > assured performance in HS mode.
> >
> > Does this apply to 3630 only, or also 34xx? Is this safe to do
> > always?
> >
> > [Allen] Yes, it does apply to 36xx only.

There is some confusion, this patch holds true for all OMAP34XX, 36XX as well as 44XX. The idea was to rely on only external Pull-up for I2C operation and disable any internal pull-up on any of the connected power IC's: Triton(TWL4030)/GAIA(TWL5030)/Phoenix(TWL6030)

I did verify, the register 'REG_GPPUPDCTR1' holds true for Triton(TWL4030)/GAIA(TWL5030), hence for OMAP34XX/36XX.
But there is a register change for this PU/PD control on Phoenix(TWL6030) which is used with OMAP44XX. So I might have to modify this patch a little as this code will apply for TWL4030/5030 and have to introduce new code to achieve the intended fix for TWL6030.

The second patch of the series: 'omap: 3630: Disable internal pull-ups'
applies only for OMAP36XX as it is a new feature introduced in OMAP3630.

I have to check if that feature is available on OMAP44XX as well, if so I will modify that patch to extend to OMAP44XX in future. 

Although
> 
> Sounds like then this configuration should be passed from the
> board-*.c file in platform_data as the external pulls depend
> on the board.
> 
> > BTW, once ready it hould be sent to Samuel Ortiz with linux-omap
> > list Cc'd:
> >
> > [Allen] i'll have it sent to Samuel also.
> 
> Thanks, we can't merge it yet though, see above.
> 
> Regards,
> 
> Tony
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Regards
Moiz Sonasath

RE: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups

[Pais, Allen]

________________________________________
From: Sonasath, Moiz
Sent: Tuesday, February 02, 2010 3:40 PM
To: Tony Lindgren; Pais, Allen
Cc: linux-omap@vger.kernel.org
Subject: RE: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups

Tony/Allen/Paul,

> -----Original Message-----
> From: linux-omap-owner@vger.kernel.org [mailto:linux-omap-
> owner@vger.kernel.org] On Behalf Of Tony Lindgren
> Sent: Tuesday, February 02, 2010 10:56 AM
> To: Pais, Allen
> Cc: linux-omap@vger.kernel.org
> Subject: Re: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups
>
> * Pais, Allen <allen.pais@ti.com> [100201 18:56]:
> >
> > ________________________________________
> > From: Tony Lindgren [tony@atomide.com]
> > Sent: Monday, February 01, 2010 7:53 PM
> > To: Pais, Allen
> > Cc: linux-omap@vger.kernel.org
> > Subject: Re: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups
> >
> > Hi,
> >
> > * Pais, Allen <allen.pais@ti.com> [100121 02:31]:
> > > From 4044fcc9c517e86fbea9f7d3b15d5cf75a767476 Mon Sep 17 00:00:00 2001
> > > From: Allen Pais <allen.pais@ti.com>
> > > Date: Thu, 21 Jan 2010 21:00:04 +0530
> > > Subject: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups
> > >
> > > This patch disables GAIA I2C1 adn I2C4(SR) internal pull-up, to
> > > use only the external HW resistor >=470 Ohm for the assured
> > > functionality in HS mode.
> > >
> > > While testing the I2C in High Speed mode, it was discovered that
> > > without a proper pull-up resistor, there is data corruption during
> > > multi-byte transfer. RTC(time_set) test case was used for testing.
> > >
> > > From the analysis done, it was concluded that ideally we need a
> > > pull-up of 1.6k Ohm(recomended) or atleast 470 Ohm or greater for
> > > assured performance in HS mode.
> >
> > Does this apply to 3630 only, or also 34xx? Is this safe to do
> > always?
> >
> > [Allen] Yes, it does apply to 36xx only.

There is some confusion, this patch holds true for all OMAP34XX, 36XX as well as 44XX. The idea was to rely on only external Pull-up for I2C operation and disable any internal pull-up on any of the connected power IC's: Triton(TWL4030)/GAIA(TWL5030)/Phoenix(TWL6030)

[Allen] sorry about the confusion. I mistook it for the other patch. 

I did verify, the register 'REG_GPPUPDCTR1' holds true for Triton(TWL4030)/GAIA(TWL5030), hence for OMAP34XX/36XX.
But there is a register change for this PU/PD control on Phoenix(TWL6030) which is used with OMAP44XX. So I might have to modify this patch a little as this code will apply for TWL4030/5030 and have to introduce new code to achieve the intended fix for TWL6030.

The second patch of the series: 'omap: 3630: Disable internal pull-ups'
applies only for OMAP36XX as it is a new feature introduced in OMAP3630.

I have to check if that feature is available on OMAP44XX as well, if so I will modify that patch to extend to OMAP44XX in future.

Although
>
> Sounds like then this configuration should be passed from the
> board-*.c file in platform_data as the external pulls depend
> on the board.
>
> > BTW, once ready it hould be sent to Samuel Ortiz with linux-omap
> > list Cc'd:
> >
> > [Allen] i'll have it sent to Samuel also.
>
> Thanks, we can't merge it yet though, see above.
>
> Regards,
>
> Tony
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Regards
Moiz Sonasath

Re: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups

[Paul Walmsley]

Hi Allen,

On Thu, 21 Jan 2010, Pais, Allen wrote:

> This patch disables GAIA I2C1 adn I2C4(SR) internal pull-up, to
> use only the external HW resistor >=470 Ohm for the assured
> functionality in HS mode.
> 
> While testing the I2C in High Speed mode, it was discovered that
> without a proper pull-up resistor, there is data corruption during
> multi-byte transfer. RTC(time_set) test case was used for testing.
> 
> >From the analysis done, it was concluded that ideally we need a
> pull-up of 1.6k Ohm(recomended) or atleast 470 Ohm or greater for
> assured performance in HS mode.

A few more questions.

1. Does this patch also apply to other TI PMICs, e.g., Triton2, Reno, etc?

2. It sounds like, from the patch description, that HS I2C mode is not 
reliable on boards without external pullups.  Shouldn't this patch (or one 
in the same series) provide some way for mach-omap2/board-*.c files to 
indicate this to the I2C code, such that HS I2C can be disallowed on those 
boards?  Similarly, for SmartReflex, which has an internal I2C controller 
but does not use the OMAP I2C driver code, shouldn't there be a similar 
mechanism for that code?


- Paul

RE: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups

[Sonasath, Moiz]

Paul,

> -----Original Message-----
> From: Paul Walmsley [mailto:paul@pwsan.com]
> Sent: Monday, February 01, 2010 9:37 PM
> To: Pais, Allen
> Cc: linux-omap@vger.kernel.org; Sonasath, Moiz; tony@atomide.com
> Subject: Re: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups
> 
> Hi Allen,
> 
> On Thu, 21 Jan 2010, Pais, Allen wrote:
> 
> > This patch disables GAIA I2C1 adn I2C4(SR) internal pull-up, to
> > use only the external HW resistor >=470 Ohm for the assured
> > functionality in HS mode.
> >
> > While testing the I2C in High Speed mode, it was discovered that
> > without a proper pull-up resistor, there is data corruption during
> > multi-byte transfer. RTC(time_set) test case was used for testing.
> >
> > >From the analysis done, it was concluded that ideally we need a
> > pull-up of 1.6k Ohm(recomended) or atleast 470 Ohm or greater for
> > assured performance in HS mode.
> 
> A few more questions.
> 
> 1. Does this patch also apply to other TI PMICs, e.g., Triton2, Reno, etc?

Thanks for raising this important question.

This patch does hold true for the following PM IC's:
Triton(TWL4030)/GAIA(TWL5030)/Phoenix(TWL6030)

I did verify, the register 'REG_GPPUPDCTR1' holds true for Triton(TWL4030)/GAIA(TWL5030), hence for OMAP34XX/36XX.
But there is a register change for this PU/PD control on Phoenix(TWL6030) which is used with OMAP44XX. So I might have to modify this patch a little as this code will apply for TWL4030/5030 and have to introduce new code to achieve the intended fix for TWL6030.

I don't think so it applies to Reno though, AFAIK its not included in the Triton family and not used with OMAP on any boards.

> 
> 2. It sounds like, from the patch description, that HS I2C mode is not
> reliable on boards without external pullups.  Shouldn't this patch (or one
> in the same series) provide some way for mach-omap2/board-*.c files to
> indicate this to the I2C code, such that HS I2C can be disallowed on those
> boards?  Similarly, for SmartReflex, which has an internal I2C controller
> but does not use the OMAP I2C driver code, shouldn't there be a similar
> mechanism for that code?

Its not that HS I2C mode is not reliable without external pull-ups but what was observed is that, for reliable operation a HW resitor >= 470 Ohm is needed as pull-up. Now if we keep the internal pull-ups on PIC's enabled they come in parallel to the external pull-up and might lead to lower equivalent resistor value (< required 470 Ohm). That was the reason we decided to just rely on the external resistor on OMAP SOM, and disable any internal pull-ups on Power IC as well as on OMAP (36XX/44XX).
 
> 
> 
> - Paul