| Message ID | 1434482445-1818-1-git-send-email-balbi@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jun 16, 2015 at 02:20:45PM -0500, Felipe Balbi wrote: > With this patch we try to be as close to 50% > duty cycle as possible. The reason for this > is that some devices present an erratic behavior > with certain duty cycles. > > One such example is TPS65218 PMIC which fails > to change voltages when running @ 400kHz and > duty cycle is lower than 34%. > > The idea of the patch is simple: > > calculate desired scl_period from requested scl > and use 50% for tLow and 50% for tHigh. > > tLow is calculated with a DIV_ROUND_UP() to make > sure it's slightly higher than tHigh and to make > sure that we end up within I2C specifications. > > Kudos to Nishanth Menon and Dave Gerlach for helping > debugging the TPS65218 problem found on AM437x SK. > > Signed-off-by: Felipe Balbi <balbi@ti.com> Aaro, can you test this on N900 ? > --- > drivers/i2c/busses/i2c-omap.c | 86 ++++++++++++++++++++++++++++--------------- > 1 file changed, 56 insertions(+), 30 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index 0e894193accf..034d2d1ff289 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -25,6 +25,7 @@ > */ > > #include <linux/module.h> > +#include <linux/kernel.h> > #include <linux/delay.h> > #include <linux/i2c.h> > #include <linux/err.h> > @@ -39,6 +40,8 @@ > #include <linux/i2c-omap.h> > #include <linux/pm_runtime.h> > > +#define NSECS_PER_SEC 1000000000 > + > /* I2C controller revisions */ > #define OMAP_I2C_OMAP1_REV_2 0x20 > > @@ -359,6 +362,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) > u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0; > unsigned long fclk_rate = 12000000; > unsigned long internal_clk = 0; > + unsigned long internal_clk_period = 0; > + unsigned long scl_period = 0; > struct clk *fclk; > > if (dev->rev >= OMAP_I2C_REV_ON_3430_3530) { > @@ -395,52 +400,73 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) > } > > if (!(dev->flags & OMAP_I2C_FLAG_SIMPLE_CLOCK)) { > - > /* > * HSI2C controller internal clk rate should be 19.2 Mhz for > - * HS and for all modes on 2430. On 34xx we can use lower rate > - * to get longer filter period for better noise suppression. > - * The filter is iclk (fclk for HS) period. > + * HS and for all modes on 2430. For all other devices and > + * speeds we will use a 12MHz internal clock. > */ > - if (dev->speed > 400 || > - dev->flags & OMAP_I2C_FLAG_FORCE_19200_INT_CLK) > - internal_clk = 19200; > - else if (dev->speed > 100) > - internal_clk = 9600; > - else > - internal_clk = 4000; > + if (dev->flags & OMAP_I2C_FLAG_FORCE_19200_INT_CLK || > + dev->speed > 400) { > + internal_clk = 1920000; > + internal_clk_period = NSECS_PER_SEC / > + internal_clk; /* ns */ > + } else { > + internal_clk = 12000000; > + internal_clk_period = NSECS_PER_SEC / > + internal_clk; /* ns */ > + } > + > fclk = clk_get(dev->dev, "fck"); > - fclk_rate = clk_get_rate(fclk) / 1000; > + fclk_rate = clk_get_rate(fclk); > clk_put(fclk); > > /* Compute prescaler divisor */ > psc = fclk_rate / internal_clk; > psc = psc - 1; > > + /* > + * Here's the tricky part, we want to make sure our duty cycle > + * is as close to 50% as possible. In order to achieve that, we > + * will first figure out what's the period on chosen scl is, > + * then divide that by two and calculate SCLL and SCLH based on > + * that. > + * > + * SCLL and SCLH equations are as folows: > + * > + * SCLL = (tLow / iclk_period) - 7; > + * SCLH = (tHigh / iclk_period) - 5; > + * > + * Where iclk_period is period of Internal Clock. > + * > + * tLow and tHigh will be basically half of scl_period where > + * possible as long as we can match I2C spec's minimum limits > + * for them. > + */ > + scl_period = NSECS_PER_SEC / dev->speed; > + > /* If configured for High Speed */ > if (dev->speed > 400) { > - unsigned long scl; > + unsigned long fs_period; > + > + /* > + * first phase of HS mode is up to > + * 400kHz so we will use that. > + */ > + fs_period = NSECS_PER_SEC / 400; > > /* For first phase of HS mode */ > - scl = internal_clk / 400; > - fsscll = scl - (scl / 3) - 7; > - fssclh = (scl / 3) - 5; > + fsscll = DIV_ROUND_UP(fs_period >> 1, > + internal_clk_period) - 7; > + fssclh = (fs_period >> 1) / internal_clk_period - 5; > > /* For second phase of HS mode */ > - scl = fclk_rate / dev->speed; > - 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; > - } else { > - /* Standard mode */ > - fsscll = internal_clk / (dev->speed * 2) - 7; > - fssclh = internal_clk / (dev->speed * 2) - 5; > + hsscll = DIV_ROUND_UP(scl_period >> 1, > + internal_clk_period) - 7; > + hssclh = (scl_period >> 1) / internal_clk_period - 5; > + } else { > + fsscll = DIV_ROUND_UP(scl_period >> 1, > + internal_clk_period) - 7; > + fssclh = (scl_period >> 1) / internal_clk_period - 5; > } > scll = (hsscll << OMAP_I2C_SCLL_HSSCLL) | fsscll; > sclh = (hssclh << OMAP_I2C_SCLH_HSSCLH) | fssclh; > -- > 2.4.3 >
Hello! On 16/06/15 21:20, ext Felipe Balbi wrote: > With this patch we try to be as close to 50% > duty cycle as possible. The reason for this > is that some devices present an erratic behavior > with certain duty cycles. > > One such example is TPS65218 PMIC which fails > to change voltages when running @ 400kHz and > duty cycle is lower than 34%. > > The idea of the patch is simple: > > calculate desired scl_period from requested scl > and use 50% for tLow and 50% for tHigh. > > tLow is calculated with a DIV_ROUND_UP() to make > sure it's slightly higher than tHigh and to make > sure that we end up within I2C specifications. > > Kudos to Nishanth Menon and Dave Gerlach for helping > debugging the TPS65218 problem found on AM437x SK. > > Signed-off-by: Felipe Balbi <balbi@ti.com> NAK. This is a direct violation of PHILIPS I2C-bus Specification v.2.1, section 15. Namely, you will have LOW period of SCL clock shorter than required 1.3uS. Therefore, I'm trying to fix the same currently in davinci: https://patchwork.ozlabs.org/patch/483024/ Probably your problem is in board design. I don't think this should be the reason for the spec-violating code in the upstream Kernel. You have your TI SDK for this. > --- > drivers/i2c/busses/i2c-omap.c | 86 ++++++++++++++++++++++++++++--------------- > 1 file changed, 56 insertions(+), 30 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index 0e894193accf..034d2d1ff289 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -25,6 +25,7 @@ > */ > > #include <linux/module.h> > +#include <linux/kernel.h> > #include <linux/delay.h> > #include <linux/i2c.h> > #include <linux/err.h> > @@ -39,6 +40,8 @@ > #include <linux/i2c-omap.h> > #include <linux/pm_runtime.h> > > +#define NSECS_PER_SEC 1000000000 > + > /* I2C controller revisions */ > #define OMAP_I2C_OMAP1_REV_2 0x20 > > @@ -359,6 +362,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) > u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0; > unsigned long fclk_rate = 12000000; > unsigned long internal_clk = 0; > + unsigned long internal_clk_period = 0; > + unsigned long scl_period = 0; > struct clk *fclk; > > if (dev->rev >= OMAP_I2C_REV_ON_3430_3530) { > @@ -395,52 +400,73 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) > } > > if (!(dev->flags & OMAP_I2C_FLAG_SIMPLE_CLOCK)) { > - > /* > * HSI2C controller internal clk rate should be 19.2 Mhz for > - * HS and for all modes on 2430. On 34xx we can use lower rate > - * to get longer filter period for better noise suppression. > - * The filter is iclk (fclk for HS) period. > + * HS and for all modes on 2430. For all other devices and > + * speeds we will use a 12MHz internal clock. > */ > - if (dev->speed > 400 || > - dev->flags & OMAP_I2C_FLAG_FORCE_19200_INT_CLK) > - internal_clk = 19200; > - else if (dev->speed > 100) > - internal_clk = 9600; > - else > - internal_clk = 4000; > + if (dev->flags & OMAP_I2C_FLAG_FORCE_19200_INT_CLK || > + dev->speed > 400) { > + internal_clk = 1920000; > + internal_clk_period = NSECS_PER_SEC / > + internal_clk; /* ns */ > + } else { > + internal_clk = 12000000; > + internal_clk_period = NSECS_PER_SEC / > + internal_clk; /* ns */ > + } > + > fclk = clk_get(dev->dev, "fck"); > - fclk_rate = clk_get_rate(fclk) / 1000; > + fclk_rate = clk_get_rate(fclk); > clk_put(fclk); > > /* Compute prescaler divisor */ > psc = fclk_rate / internal_clk; > psc = psc - 1; > > + /* > + * Here's the tricky part, we want to make sure our duty cycle > + * is as close to 50% as possible. In order to achieve that, we > + * will first figure out what's the period on chosen scl is, > + * then divide that by two and calculate SCLL and SCLH based on > + * that. > + * > + * SCLL and SCLH equations are as folows: > + * > + * SCLL = (tLow / iclk_period) - 7; > + * SCLH = (tHigh / iclk_period) - 5; > + * > + * Where iclk_period is period of Internal Clock. > + * > + * tLow and tHigh will be basically half of scl_period where > + * possible as long as we can match I2C spec's minimum limits > + * for them. > + */ > + scl_period = NSECS_PER_SEC / dev->speed; > + > /* If configured for High Speed */ > if (dev->speed > 400) { > - unsigned long scl; > + unsigned long fs_period; > + > + /* > + * first phase of HS mode is up to > + * 400kHz so we will use that. > + */ > + fs_period = NSECS_PER_SEC / 400; > > /* For first phase of HS mode */ > - scl = internal_clk / 400; > - fsscll = scl - (scl / 3) - 7; > - fssclh = (scl / 3) - 5; > + fsscll = DIV_ROUND_UP(fs_period >> 1, > + internal_clk_period) - 7; > + fssclh = (fs_period >> 1) / internal_clk_period - 5; > > /* For second phase of HS mode */ > - scl = fclk_rate / dev->speed; > - 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; > - } else { > - /* Standard mode */ > - fsscll = internal_clk / (dev->speed * 2) - 7; > - fssclh = internal_clk / (dev->speed * 2) - 5; > + hsscll = DIV_ROUND_UP(scl_period >> 1, > + internal_clk_period) - 7; > + hssclh = (scl_period >> 1) / internal_clk_period - 5; > + } else { > + fsscll = DIV_ROUND_UP(scl_period >> 1, > + internal_clk_period) - 7; > + fssclh = (scl_period >> 1) / internal_clk_period - 5; > } > scll = (hsscll << OMAP_I2C_SCLL_HSSCLL) | fsscll; > sclh = (hssclh << OMAP_I2C_SCLH_HSSCLH) | fssclh; >
Hi, On Wed, Jun 17, 2015 at 11:19:28AM +0200, Alexander Sverdlin wrote: > > With this patch we try to be as close to 50% > > duty cycle as possible. The reason for this > > is that some devices present an erratic behavior > > with certain duty cycles. > > > > One such example is TPS65218 PMIC which fails > > to change voltages when running @ 400kHz and > > duty cycle is lower than 34%. > > > > The idea of the patch is simple: > > > > calculate desired scl_period from requested scl > > and use 50% for tLow and 50% for tHigh. > > > > tLow is calculated with a DIV_ROUND_UP() to make > > sure it's slightly higher than tHigh and to make > > sure that we end up within I2C specifications. > > > > Kudos to Nishanth Menon and Dave Gerlach for helping > > debugging the TPS65218 problem found on AM437x SK. > > > > Signed-off-by: Felipe Balbi <balbi@ti.com> > > NAK. > This is a direct violation of PHILIPS I2C-bus Specification v.2.1, > section 15. > Namely, you will have LOW period of SCL clock shorter than required > 1.3uS. how is this out of spec ? http://i.imgur.com/jEDlZT7.png -Width = 1.4us, frequency 373.1kHz, duty cycle of 47.76% In any case, I have to send v2 anyway (found a bug which would show up on frequencies above 400kHz), so I'll resend this patch.
On Wed, Jun 17, 2015 at 01:00:52PM -0500, Felipe Balbi wrote: > Hi, > > On Wed, Jun 17, 2015 at 11:19:28AM +0200, Alexander Sverdlin wrote: > > > With this patch we try to be as close to 50% > > > duty cycle as possible. The reason for this > > > is that some devices present an erratic behavior > > > with certain duty cycles. > > > > > > One such example is TPS65218 PMIC which fails > > > to change voltages when running @ 400kHz and > > > duty cycle is lower than 34%. > > > > > > The idea of the patch is simple: > > > > > > calculate desired scl_period from requested scl > > > and use 50% for tLow and 50% for tHigh. > > > > > > tLow is calculated with a DIV_ROUND_UP() to make > > > sure it's slightly higher than tHigh and to make > > > sure that we end up within I2C specifications. > > > > > > Kudos to Nishanth Menon and Dave Gerlach for helping > > > debugging the TPS65218 problem found on AM437x SK. > > > > > > Signed-off-by: Felipe Balbi <balbi@ti.com> > > > > NAK. > > This is a direct violation of PHILIPS I2C-bus Specification v.2.1, > > section 15. > > Namely, you will have LOW period of SCL clock shorter than required > > 1.3uS. > > how is this out of spec ? > > http://i.imgur.com/jEDlZT7.png > > -Width = 1.4us, frequency 373.1kHz, duty cycle of 47.76% > > In any case, I have to send v2 anyway (found a bug which would show up > on frequencies above 400kHz), so I'll resend this patch. Also, let's assume we can generate exactly 400kHz with the same duty cycle. 400kHz has a period of 2500ns, so: 2500 * (1 - 0.4773) = 1306us How is that out of spec ?
Hello Felipe, On 17/06/15 20:00, Felipe Balbi wrote: >>> With this patch we try to be as close to 50% >>> > > duty cycle as possible. The reason for this >>> > > is that some devices present an erratic behavior >>> > > with certain duty cycles. >>> > > >>> > > One such example is TPS65218 PMIC which fails >>> > > to change voltages when running @ 400kHz and >>> > > duty cycle is lower than 34%. >>> > > >>> > > The idea of the patch is simple: >>> > > >>> > > calculate desired scl_period from requested scl >>> > > and use 50% for tLow and 50% for tHigh. >>> > > >>> > > tLow is calculated with a DIV_ROUND_UP() to make >>> > > sure it's slightly higher than tHigh and to make >>> > > sure that we end up within I2C specifications. >>> > > >>> > > Kudos to Nishanth Menon and Dave Gerlach for helping >>> > > debugging the TPS65218 problem found on AM437x SK. >>> > > >>> > > Signed-off-by: Felipe Balbi <balbi@ti.com> >> > >> > NAK. >> > This is a direct violation of PHILIPS I2C-bus Specification v.2.1, >> > section 15. >> > Namely, you will have LOW period of SCL clock shorter than required >> > 1.3uS. > how is this out of spec ? > > http://i.imgur.com/jEDlZT7.png > > -Width = 1.4us, frequency 373.1kHz, duty cycle of 47.76% > > In any case, I have to send v2 anyway (found a bug which would show up > on frequencies above 400kHz), so I'll resend this patch. If you really target 50% duty cycle and there will be no rounding/truncation error, you will end up with 1.25uS at 400kHz. I understand why you want to make HIGH phase longer, but 50% is a bad target at 400hHz. Probably more safe value? Alex.
Hi, On Wed, Jun 17, 2015 at 08:38:42PM +0200, Alexander Sverdlin wrote: > Hello Felipe, > > On 17/06/15 20:00, Felipe Balbi wrote: > >>> With this patch we try to be as close to 50% > >>> > > duty cycle as possible. The reason for this > >>> > > is that some devices present an erratic behavior > >>> > > with certain duty cycles. > >>> > > > >>> > > One such example is TPS65218 PMIC which fails > >>> > > to change voltages when running @ 400kHz and > >>> > > duty cycle is lower than 34%. > >>> > > > >>> > > The idea of the patch is simple: > >>> > > > >>> > > calculate desired scl_period from requested scl > >>> > > and use 50% for tLow and 50% for tHigh. > >>> > > > >>> > > tLow is calculated with a DIV_ROUND_UP() to make > >>> > > sure it's slightly higher than tHigh and to make > >>> > > sure that we end up within I2C specifications. > >>> > > > >>> > > Kudos to Nishanth Menon and Dave Gerlach for helping > >>> > > debugging the TPS65218 problem found on AM437x SK. > >>> > > > >>> > > Signed-off-by: Felipe Balbi <balbi@ti.com> > >> > > >> > NAK. > >> > This is a direct violation of PHILIPS I2C-bus Specification v.2.1, > >> > section 15. > >> > Namely, you will have LOW period of SCL clock shorter than required > >> > 1.3uS. > > how is this out of spec ? > > > > http://i.imgur.com/jEDlZT7.png > > > > -Width = 1.4us, frequency 373.1kHz, duty cycle of 47.76% > > > > In any case, I have to send v2 anyway (found a bug which would show up > > on frequencies above 400kHz), so I'll resend this patch. > > If you really target 50% duty cycle and there will be no > rounding/truncation error, you will end up with 1.25uS at 400kHz. I > understand why you want to make HIGH phase longer, but 50% is a bad > target at 400hHz. Probably more safe value? We can't generate exactly 400kHz anyway, and we're not getting exactly 50% duty cycle, it just brings it *closer* to that. I even mention the reason for the DIV_ROUND_UP() there. Let's just go through the math using TRM equation: @400kHz with 12MHz internal clock: tLow = (SCLL + 7) * iclk_period 1250 = (SCLL + 7) * 1/12MHz SCLL = DIV_ROUND_UP(1250, 83) - 7 SCLL = (1250 + 82)/83 - 7 SCLL = 9 Now if we do the reverse to find actual tLow: tLow = (9 + 7) * 83 tLow = 1328ns Likewise if we do it for tHigh: tHigh = (SCLH + 5) iclk_period SCLH = 1250 / 83 - 5 SCLH = 10 tHigh = 15 * 83 tHigh = 1245 tHigh + tLow (SCL period) = 2573ns. That gives us SCL of 388.65kHz. An error of mere 3% of what we really wanted to achieve. I'd say this is pretty darn good. Now you tell me, how is this *ever* going to be out of spec ? DIV_ROUND_UP() and the truncation at internal clock period calculation makes sure that tLow will be within spec. Instead of blindly NAKing the patch, you could've gone through this exercise yourself.
Hello! On 17/06/15 20:57, Felipe Balbi wrote: >>>>>>> duty cycle as possible. The reason for this >>>>>>> > >>> > > is that some devices present an erratic behavior >>>>>>> > >>> > > with certain duty cycles. >>>>>>> > >>> > > >>>>>>> > >>> > > One such example is TPS65218 PMIC which fails >>>>>>> > >>> > > to change voltages when running @ 400kHz and >>>>>>> > >>> > > duty cycle is lower than 34%. >>>>>>> > >>> > > >>>>>>> > >>> > > The idea of the patch is simple: >>>>>>> > >>> > > >>>>>>> > >>> > > calculate desired scl_period from requested scl >>>>>>> > >>> > > and use 50% for tLow and 50% for tHigh. >>>>>>> > >>> > > >>>>>>> > >>> > > tLow is calculated with a DIV_ROUND_UP() to make >>>>>>> > >>> > > sure it's slightly higher than tHigh and to make >>>>>>> > >>> > > sure that we end up within I2C specifications. >>>>>>> > >>> > > >>>>>>> > >>> > > Kudos to Nishanth Menon and Dave Gerlach for helping >>>>>>> > >>> > > debugging the TPS65218 problem found on AM437x SK. >>>>>>> > >>> > > >>>>>>> > >>> > > Signed-off-by: Felipe Balbi <balbi@ti.com> >>>>> > >> > >>>>> > >> > NAK. >>>>> > >> > This is a direct violation of PHILIPS I2C-bus Specification v.2.1, >>>>> > >> > section 15. >>>>> > >> > Namely, you will have LOW period of SCL clock shorter than required >>>>> > >> > 1.3uS. >>> > > how is this out of spec ? >>> > > >>> > > http://i.imgur.com/jEDlZT7.png >>> > > >>> > > -Width = 1.4us, frequency 373.1kHz, duty cycle of 47.76% >>> > > >>> > > In any case, I have to send v2 anyway (found a bug which would show up >>> > > on frequencies above 400kHz), so I'll resend this patch. >> > >> > If you really target 50% duty cycle and there will be no >> > rounding/truncation error, you will end up with 1.25uS at 400kHz. I >> > understand why you want to make HIGH phase longer, but 50% is a bad >> > target at 400hHz. Probably more safe value? > We can't generate exactly 400kHz anyway, and we're not getting exactly > 50% duty cycle, it just brings it *closer* to that. I even mention the > reason for the DIV_ROUND_UP() there. Let's just go through the math > using TRM equation: > > @400kHz with 12MHz internal clock: > > tLow = (SCLL + 7) * iclk_period > > 1250 = (SCLL + 7) * 1/12MHz > > SCLL = DIV_ROUND_UP(1250, 83) - 7 > SCLL = (1250 + 82)/83 - 7 > SCLL = 9 > > Now if we do the reverse to find actual tLow: > > tLow = (9 + 7) * 83 > tLow = 1328ns > > Likewise if we do it for tHigh: > > tHigh = (SCLH + 5) iclk_period > > SCLH = 1250 / 83 - 5 > SCLH = 10 > > tHigh = 15 * 83 > tHigh = 1245 > > tHigh + tLow (SCL period) = 2573ns. That gives us SCL of 388.65kHz. An The bus rate is actually 12MHz/(SCLH+5+SCLL+7), so it's ~387096Hz. Anyway, it's still possible to have maximum bus rate if SCLH+SCLL=12000000/400000-5-7 Let say SCLH=9, SCLL=9, still within the spec and exactly 400kHz But you need another equations and it's not about 50% duty cycle > error of mere 3% of what we really wanted to achieve. I'd say this is > pretty darn good. > > Now you tell me, how is this *ever* going to be out of spec ? > > DIV_ROUND_UP() and the truncation at internal clock period calculation > makes sure that tLow will be within spec. Instead of blindly NAKing the > patch, you could've gone through this exercise yourself.
diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index 0e894193accf..034d2d1ff289 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -25,6 +25,7 @@ */ #include <linux/module.h> +#include <linux/kernel.h> #include <linux/delay.h> #include <linux/i2c.h> #include <linux/err.h> @@ -39,6 +40,8 @@ #include <linux/i2c-omap.h> #include <linux/pm_runtime.h> +#define NSECS_PER_SEC 1000000000 + /* I2C controller revisions */ #define OMAP_I2C_OMAP1_REV_2 0x20 @@ -359,6 +362,8 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) u16 fsscll = 0, fssclh = 0, hsscll = 0, hssclh = 0; unsigned long fclk_rate = 12000000; unsigned long internal_clk = 0; + unsigned long internal_clk_period = 0; + unsigned long scl_period = 0; struct clk *fclk; if (dev->rev >= OMAP_I2C_REV_ON_3430_3530) { @@ -395,52 +400,73 @@ static int omap_i2c_init(struct omap_i2c_dev *dev) } if (!(dev->flags & OMAP_I2C_FLAG_SIMPLE_CLOCK)) { - /* * HSI2C controller internal clk rate should be 19.2 Mhz for - * HS and for all modes on 2430. On 34xx we can use lower rate - * to get longer filter period for better noise suppression. - * The filter is iclk (fclk for HS) period. + * HS and for all modes on 2430. For all other devices and + * speeds we will use a 12MHz internal clock. */ - if (dev->speed > 400 || - dev->flags & OMAP_I2C_FLAG_FORCE_19200_INT_CLK) - internal_clk = 19200; - else if (dev->speed > 100) - internal_clk = 9600; - else - internal_clk = 4000; + if (dev->flags & OMAP_I2C_FLAG_FORCE_19200_INT_CLK || + dev->speed > 400) { + internal_clk = 1920000; + internal_clk_period = NSECS_PER_SEC / + internal_clk; /* ns */ + } else { + internal_clk = 12000000; + internal_clk_period = NSECS_PER_SEC / + internal_clk; /* ns */ + } + fclk = clk_get(dev->dev, "fck"); - fclk_rate = clk_get_rate(fclk) / 1000; + fclk_rate = clk_get_rate(fclk); clk_put(fclk); /* Compute prescaler divisor */ psc = fclk_rate / internal_clk; psc = psc - 1; + /* + * Here's the tricky part, we want to make sure our duty cycle + * is as close to 50% as possible. In order to achieve that, we + * will first figure out what's the period on chosen scl is, + * then divide that by two and calculate SCLL and SCLH based on + * that. + * + * SCLL and SCLH equations are as folows: + * + * SCLL = (tLow / iclk_period) - 7; + * SCLH = (tHigh / iclk_period) - 5; + * + * Where iclk_period is period of Internal Clock. + * + * tLow and tHigh will be basically half of scl_period where + * possible as long as we can match I2C spec's minimum limits + * for them. + */ + scl_period = NSECS_PER_SEC / dev->speed; + /* If configured for High Speed */ if (dev->speed > 400) { - unsigned long scl; + unsigned long fs_period; + + /* + * first phase of HS mode is up to + * 400kHz so we will use that. + */ + fs_period = NSECS_PER_SEC / 400; /* For first phase of HS mode */ - scl = internal_clk / 400; - fsscll = scl - (scl / 3) - 7; - fssclh = (scl / 3) - 5; + fsscll = DIV_ROUND_UP(fs_period >> 1, + internal_clk_period) - 7; + fssclh = (fs_period >> 1) / internal_clk_period - 5; /* For second phase of HS mode */ - scl = fclk_rate / dev->speed; - 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; - } else { - /* Standard mode */ - fsscll = internal_clk / (dev->speed * 2) - 7; - fssclh = internal_clk / (dev->speed * 2) - 5; + hsscll = DIV_ROUND_UP(scl_period >> 1, + internal_clk_period) - 7; + hssclh = (scl_period >> 1) / internal_clk_period - 5; + } else { + fsscll = DIV_ROUND_UP(scl_period >> 1, + internal_clk_period) - 7; + fssclh = (scl_period >> 1) / internal_clk_period - 5; } scll = (hsscll << OMAP_I2C_SCLL_HSSCLL) | fsscll; sclh = (hssclh << OMAP_I2C_SCLH_HSSCLH) | fssclh;
With this patch we try to be as close to 50% duty cycle as possible. The reason for this is that some devices present an erratic behavior with certain duty cycles. One such example is TPS65218 PMIC which fails to change voltages when running @ 400kHz and duty cycle is lower than 34%. The idea of the patch is simple: calculate desired scl_period from requested scl and use 50% for tLow and 50% for tHigh. tLow is calculated with a DIV_ROUND_UP() to make sure it's slightly higher than tHigh and to make sure that we end up within I2C specifications. Kudos to Nishanth Menon and Dave Gerlach for helping debugging the TPS65218 problem found on AM437x SK. Signed-off-by: Felipe Balbi <balbi@ti.com> --- drivers/i2c/busses/i2c-omap.c | 86 ++++++++++++++++++++++++++++--------------- 1 file changed, 56 insertions(+), 30 deletions(-)