Re: [RFC][PATCH] [I2C]pass scll, sclh through board file for Errata i585

2010-04-28 Thread Nishanth Menon

Krishnamoorthy, Balaji T had written, on 04/28/2010 07:34 AM, the following:

From: Tony Lindgren [mailto:t...@atomide.com]
* balaj...@ti.com  [100419 07:59]:

From: Balaji T K 

Errata ID: i535 - I2C1 to 3 SCL low period is shorter in FS mode
Due to IO cell influence, I2C1 to 3 SCL low period can be shorter than

expected.

As a result, I2C AC timing (SCL minimum low period) in FS mode may not meet
the timing configured by software.

I2C1 to 3, SCL low period is programmable and proper adjustments
to the SCLL/HSSCLL values can avoid the issue.

This patch provides flexibility to control tLOW, tHIGH for different boards.
scll, sclh values are to be provide in board data
for differents modes (standard, fast and high speed mode)
as the scl rate (I2C bus speed) can be changed by bootargs.

If scll, sclh values are not provided, scll and sclh values will be computed
for duty cycle tHIGH:tLOW of 1:2 (for HS, FS) and 1:1 for Standard as before




--- a/arch/arm/mach-omap2/board-3430sdp.c
+++ b/arch/arm/mach-omap2/board-3430sdp.c
@@ -594,6 +594,26 @@ static struct twl4030_platform_data sdp3430_twldata = {
.vpll2  = &sdp3430_vpll2,
 };

+static struct omap_i2c_scl_data __initdata sdp3430_i2c1_scl_data = {
+   .rate   = 2600,
+   .standard_scll  = 20,   /* For 100Khz */
+   .standard_sclh  = 20,   /* For 100Khz */


Scll and sclh calculation for other than omap1 and omap2420
from the given formula
fsscll = internal_clk / (dev->speed * 2);
fssclh = internal_clk / (dev->speed * 2);
where internal_clk is 4000KHz, dev-speed is 100Khz
and the assumption is 1:1 duty cycle, 50%tHIGH, 50%tLOW


ref: http://marc.info/?t=12354086592&r=1&w=2
tHigh and tLow are dependent of i2c bus capacitance as well.. TRM says:
The equations in Table 17-12 give the SCL timing values for 
SCLL/SCLH/HSSCLL/HSSCLH at HS I2C controller outputs. Actual tlow and 
thigh periods may vary, depending on the board (the load capacitance on 
the SCL signal). If necessary, any adjustments to the 
SCLL/SCLH/HSSCLL/HSSCLH values must be determined by measurements of 
actual SCL signal on the board...


so it is imperative that we provide some mechanism to provide
a) an autoconfiguration using the standard equation as in TRM
b) boards with different bus capacitance (less or more), should be able 
to define their own values.


This patch is in the direction to provide us that flexibility, which 
IMHO is necessary.





+   .fast_mode_scll = 16,   /* For 400Khz */
+   .fast_mode_sclh = 8,/* For 400Khz */


scl = internal_clk / dev->speed;
fsscll = scl - (scl / 3);
fssclh = (scl / 3);
internal_clk is 9600, dev->speed =400
and the assumption is 1:2 duty cycle, 33%tHIGH, 66%tLOW


+   .hs_phase1_scll = 32,   /* For 2600Khz */
+   .hs_phase1_sclh = 16,   /* For 2600Khz */


fsscll = scl - (scl / 3);
fssclh = (scl / 3);
internal_clk is 19200, dev->speed =400


+   .hs_phase2_scll = 24,   /* For 2600Khz */
+   .hs_phase2_sclh = 12,   /* For 2600Khz */


scl = fclk_rate / dev->speed;
hsscll = scl - (scl / 3);
hssclh = (scl / 3);
fclk_rate is 96000, dev->speed is 2600
and the assumption is 1:2 duty cycle, 33%tHIGH, 66%tLOW


+};

Can you please describe how you get these values for each board-*.c file?


Internal_clk is initialized to predefined value of 4000Khz, 9600, 19200
depending on i2c bus speed of 100, 400, or >400.
However fclk_rate varies between omap and needs to be determined by debug printk


--- a/arch/arm/plat-omap/i2c.c
+++ b/arch/arm/plat-omap/i2c.c
@@ -70,14 +70,14 @@ static struct resource i2c_resources[][2] = {
},  \
}

-static u32 i2c_rate[ARRAY_SIZE(i2c_resources)];
+static struct omap_i2c_platform_data i2c_pdata[ARRAY_SIZE(i2c_resources)];
 static struct platform_device omap_i2c_devices[] = {
-   I2C_DEV_BUILDER(1, i2c_resources[0], &i2c_rate[0]),
+   I2C_DEV_BUILDER(1, i2c_resources[0], &i2c_pdata[0]),
 #ifdefined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
-   I2C_DEV_BUILDER(2, i2c_resources[1], &i2c_rate[1]),
+   I2C_DEV_BUILDER(2, i2c_resources[1], &i2c_pdata[1]),
 #endif
 #ifdefined(CONFIG_ARCH_OMAP3)
-   I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_rate[2]),
+   I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_pdata[2]),
 #endif
 };

@@ -146,8 +146,8 @@ static int __init omap_i2c_bus_setup(char *str)
get_options(str, 3, ints);
if (ints[0] < 2 || ints[1] < 1 || ints[1] > ports)
return 0;
-   i2c_rate[ints[1] - 1] = ints[2];
-   i2c_rate[ints[1] - 1] |= OMAP_I2C_CMDLINE_SETUP;
+   i2c_pdata[ints[1] - 1].rate = ints[2];
+   i2c_pdata[ints[1] - 1].rate

RE: [RFC][PATCH] [I2C]pass scll, sclh through board file for Errata i585

2010-04-28 Thread Krishnamoorthy, Balaji T
> From: Tony Lindgren [mailto:t...@atomide.com]
> * balaj...@ti.com  [100419 07:59]:
> > From: Balaji T K 
> >
> > Errata ID: i535 - I2C1 to 3 SCL low period is shorter in FS mode
> > Due to IO cell influence, I2C1 to 3 SCL low period can be shorter than
> expected.
> > As a result, I2C AC timing (SCL minimum low period) in FS mode may not meet
> > the timing configured by software.
> >
> > I2C1 to 3, SCL low period is programmable and proper adjustments
> > to the SCLL/HSSCLL values can avoid the issue.
> >
> > This patch provides flexibility to control tLOW, tHIGH for different boards.
> > scll, sclh values are to be provide in board data
> > for differents modes (standard, fast and high speed mode)
> > as the scl rate (I2C bus speed) can be changed by bootargs.
> >
> > If scll, sclh values are not provided, scll and sclh values will be computed
> > for duty cycle tHIGH:tLOW of 1:2 (for HS, FS) and 1:1 for Standard as before
> 
> 
> 
> > --- a/arch/arm/mach-omap2/board-3430sdp.c
> > +++ b/arch/arm/mach-omap2/board-3430sdp.c
> > @@ -594,6 +594,26 @@ static struct twl4030_platform_data sdp3430_twldata = {
> > .vpll2  = &sdp3430_vpll2,
> >  };
> >
> > +static struct omap_i2c_scl_data __initdata sdp3430_i2c1_scl_data = {
> > +   .rate   = 2600,
> > +   .standard_scll  = 20,   /* For 100Khz */
> > +   .standard_sclh  = 20,   /* For 100Khz */

Scll and sclh calculation for other than omap1 and omap2420
from the given formula
fsscll = internal_clk / (dev->speed * 2);
fssclh = internal_clk / (dev->speed * 2);
where internal_clk is 4000KHz, dev-speed is 100Khz
and the assumption is 1:1 duty cycle, 50%tHIGH, 50%tLOW

> > +   .fast_mode_scll = 16,   /* For 400Khz */
> > +   .fast_mode_sclh = 8,/* For 400Khz */

scl = internal_clk / dev->speed;
fsscll = scl - (scl / 3);
fssclh = (scl / 3);
internal_clk is 9600, dev->speed =400
and the assumption is 1:2 duty cycle, 33%tHIGH, 66%tLOW

> > +   .hs_phase1_scll = 32,   /* For 2600Khz */
> > +   .hs_phase1_sclh = 16,   /* For 2600Khz */

fsscll = scl - (scl / 3);
fssclh = (scl / 3);
internal_clk is 19200, dev->speed =400

> > +   .hs_phase2_scll = 24,   /* For 2600Khz */
> > +   .hs_phase2_sclh = 12,   /* For 2600Khz */

scl = fclk_rate / dev->speed;
hsscll = scl - (scl / 3);
hssclh = (scl / 3);
fclk_rate is 96000, dev->speed is 2600
and the assumption is 1:2 duty cycle, 33%tHIGH, 66%tLOW

> > +};
> 
> Can you please describe how you get these values for each board-*.c file?

Internal_clk is initialized to predefined value of 4000Khz, 9600, 19200
depending on i2c bus speed of 100, 400, or >400.
However fclk_rate varies between omap and needs to be determined by debug printk

> 
> > --- a/arch/arm/plat-omap/i2c.c
> > +++ b/arch/arm/plat-omap/i2c.c
> > @@ -70,14 +70,14 @@ static struct resource i2c_resources[][2] = {
> > },  \
> > }
> >
> > -static u32 i2c_rate[ARRAY_SIZE(i2c_resources)];
> > +static struct omap_i2c_platform_data i2c_pdata[ARRAY_SIZE(i2c_resources)];
> >  static struct platform_device omap_i2c_devices[] = {
> > -   I2C_DEV_BUILDER(1, i2c_resources[0], &i2c_rate[0]),
> > +   I2C_DEV_BUILDER(1, i2c_resources[0], &i2c_pdata[0]),
> >  #ifdefined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
> > -   I2C_DEV_BUILDER(2, i2c_resources[1], &i2c_rate[1]),
> > +   I2C_DEV_BUILDER(2, i2c_resources[1], &i2c_pdata[1]),
> >  #endif
> >  #ifdefined(CONFIG_ARCH_OMAP3)
> > -   I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_rate[2]),
> > +   I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_pdata[2]),
> >  #endif
> >  };
> >
> > @@ -146,8 +146,8 @@ static int __init omap_i2c_bus_setup(char *str)
> > get_options(str, 3, ints);
> > if (ints[0] < 2 || ints[1] < 1 || ints[1] > ports)
> > return 0;
> > -   i2c_rate[ints[1] - 1] = ints[2];
> > -   i2c_rate[ints[1] - 1] |= OMAP_I2C_CMDLINE_SETUP;
> > +   i2c_pdata[ints[1] - 1].rate = ints[2];
> > +   i2c_pdata[ints[1] - 1].rate |= OMAP_I2C_CMDLINE_SETUP;
> >
> > return 1;
> >  }
> 
> FYI, the change from i2c_rate to i2c_pdata is needed also for
> "i2c-omap: add mpu wake up latency constraint in i2c" as that
> blocks some PM changes from going upstream. So once that's sorted
> out some minor rebasing of that patch is needed.

OK

> 
> > @@ -446,24 +453,47 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
> >
> > /* For first phase of HS mode */
> > scl = internal_clk / 400;
> > -   fsscll = scl - (scl / 3) - 7;
> > -   fssclh = (scl / 3) - 5;
> > +   if ((pdata->hs_phase1.scll > 7) &&
> > +   (pdata->hs_phase1.sclh > 

Re: [RFC][PATCH] [I2C]pass scll, sclh through board file for Errata i585

2010-04-26 Thread Tony Lindgren
* balaj...@ti.com  [100419 07:59]:
> From: Balaji T K 
> 
> Errata ID: i535 - I2C1 to 3 SCL low period is shorter in FS mode
> Due to IO cell influence, I2C1 to 3 SCL low period can be shorter than 
> expected. 
> As a result, I2C AC timing (SCL minimum low period) in FS mode may not meet 
> the timing configured by software.
> 
> I2C1 to 3, SCL low period is programmable and proper adjustments
> to the SCLL/HSSCLL values can avoid the issue.
> 
> This patch provides flexibility to control tLOW, tHIGH for different boards.
> scll, sclh values are to be provide in board data
> for differents modes (standard, fast and high speed mode)
> as the scl rate (I2C bus speed) can be changed by bootargs.
> 
> If scll, sclh values are not provided, scll and sclh values will be computed 
> for duty cycle tHIGH:tLOW of 1:2 (for HS, FS) and 1:1 for Standard as before



> --- a/arch/arm/mach-omap2/board-3430sdp.c
> +++ b/arch/arm/mach-omap2/board-3430sdp.c
> @@ -594,6 +594,26 @@ static struct twl4030_platform_data sdp3430_twldata = {
>   .vpll2  = &sdp3430_vpll2,
>  };
>  
> +static struct omap_i2c_scl_data __initdata sdp3430_i2c1_scl_data = {
> + .rate   = 2600,
> + .standard_scll  = 20,   /* For 100Khz */
> + .standard_sclh  = 20,   /* For 100Khz */
> + .fast_mode_scll = 16,   /* For 400Khz */
> + .fast_mode_sclh = 8,/* For 400Khz */
> + .hs_phase1_scll = 32,   /* For 2600Khz */
> + .hs_phase1_sclh = 16,   /* For 2600Khz */
> + .hs_phase2_scll = 24,   /* For 2600Khz */
> + .hs_phase2_sclh = 12,   /* For 2600Khz */
> +};

Can you please describe how you get these values for each board-*.c file?

> --- a/arch/arm/plat-omap/i2c.c
> +++ b/arch/arm/plat-omap/i2c.c
> @@ -70,14 +70,14 @@ static struct resource i2c_resources[][2] = {
>   },  \
>   }
>  
> -static u32 i2c_rate[ARRAY_SIZE(i2c_resources)];
> +static struct omap_i2c_platform_data i2c_pdata[ARRAY_SIZE(i2c_resources)];
>  static struct platform_device omap_i2c_devices[] = {
> - I2C_DEV_BUILDER(1, i2c_resources[0], &i2c_rate[0]),
> + I2C_DEV_BUILDER(1, i2c_resources[0], &i2c_pdata[0]),
>  #if  defined(CONFIG_ARCH_OMAP2) || defined(CONFIG_ARCH_OMAP3)
> - I2C_DEV_BUILDER(2, i2c_resources[1], &i2c_rate[1]),
> + I2C_DEV_BUILDER(2, i2c_resources[1], &i2c_pdata[1]),
>  #endif
>  #if  defined(CONFIG_ARCH_OMAP3)
> - I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_rate[2]),
> + I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_pdata[2]),
>  #endif
>  };
>  
> @@ -146,8 +146,8 @@ static int __init omap_i2c_bus_setup(char *str)
>   get_options(str, 3, ints);
>   if (ints[0] < 2 || ints[1] < 1 || ints[1] > ports)
>   return 0;
> - i2c_rate[ints[1] - 1] = ints[2];
> - i2c_rate[ints[1] - 1] |= OMAP_I2C_CMDLINE_SETUP;
> + i2c_pdata[ints[1] - 1].rate = ints[2];
> + i2c_pdata[ints[1] - 1].rate |= OMAP_I2C_CMDLINE_SETUP;
>  
>   return 1;
>  }

FYI, the change from i2c_rate to i2c_pdata is needed also for
"i2c-omap: add mpu wake up latency constraint in i2c" as that
blocks some PM changes from going upstream. So once that's sorted
out some minor rebasing of that patch is needed.

> @@ -446,24 +453,47 @@ static int omap_i2c_init(struct omap_i2c_dev *dev)
>  
>   /* For first phase of HS mode */
>   scl = internal_clk / 400;
> - fsscll = scl - (scl / 3) - 7;
> - fssclh = (scl / 3) - 5;
> + if ((pdata->hs_phase1.scll > 7) &&
> + (pdata->hs_phase1.sclh > 5)) {
> + fsscll = pdata->hs_phase1.scll - 7;
> + fssclh = pdata->hs_phase1.sclh - 5;
> + } else {
> + fsscll = scl - (scl / 3) - 7;
> + fssclh = (scl / 3) - 5;
> + }
>  
>   /* For second phase of HS mode */
>   scl = fclk_rate / dev->speed;
> - hsscll = scl - (scl / 3) - 7;
> - hssclh = (scl / 3) - 5;
> + if ((pdata->hs_phase2.scll > 7) &&
> + (pdata->hs_phase2.sclh > 5)) {
> + hsscll = pdata->hs_phase2.scll - 7;
> + hssclh = pdata->hs_phase2.sclh - 5;
> + } else {
> + hsscll = scl - (scl / 3) - 7;
> + hssclh = (scl / 3) - 5;
> + }
>   } else if (dev->speed > 100) {
>   unsigned long scl;
>  
>   /* Fast mode */
>   scl = internal_clk / dev->speed;
> - fsscll = scl - (scl / 3) - 7;
> - fssclh = (scl / 3) - 5;
> + if ((pdata->fast.scll > 7) && (pdata->fast.sclh > 5)) {
> +