RE: [PATCH 1/2] omap: Disable GAIA I2C1/I2C4 internal pull-ups
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-ow...@vger.kernel.org [mailto:linux-omap- > ow...@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 [100201 18:56]: > > > > > > From: Tony Lindgren [t...@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 [100121 02:31]: > > > From 4044fcc9c517e86fbea9f7d3b15d5cf75a767476 Mon Sep 17 00:00:00 2001 > > > From: Allen Pais > > > 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 majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Regards Moiz Sonasath -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@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
Paul, > -Original Message- > From: Paul Walmsley [mailto:p...@pwsan.com] > Sent: Monday, February 01, 2010 9:37 PM > To: Pais, Allen > Cc: linux-omap@vger.kernel.org; Sonasath, Moiz; t...@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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@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/Allen/Paul, > -Original Message- > From: linux-omap-ow...@vger.kernel.org [mailto:linux-omap- > ow...@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 [100201 18:56]: > > > > > > From: Tony Lindgren [t...@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 [100121 02:31]: > > > From 4044fcc9c517e86fbea9f7d3b15d5cf75a767476 Mon Sep 17 00:00:00 2001 > > > From: Allen Pais > > > 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 majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html Regards Moiz Sonasath -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@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 [100201 18:56]: > > > From: Tony Lindgren [t...@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 [100121 02:31]: > > From 4044fcc9c517e86fbea9f7d3b15d5cf75a767476 Mon Sep 17 00:00:00 2001 > > From: Allen Pais > > 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@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
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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@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
From: Tony Lindgren [t...@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 [100121 02:31]:
> From 4044fcc9c517e86fbea9f7d3b15d5cf75a767476 Mon Sep 17 00:00:00 2001
> From: Allen Pais
> 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
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
> Signed-off-by: Allen Pais
> ---
> 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;
> unsignedi;
> 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 majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@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
Hi,
* Pais, Allen [100121 02:31]:
> From 4044fcc9c517e86fbea9f7d3b15d5cf75a767476 Mon Sep 17 00:00:00 2001
> From: Allen Pais
> 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
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
> Signed-off-by: Allen Pais
> ---
> 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;
> unsignedi;
> 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 majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
