[PATCH RESEND] i2c: omap: improve duty cycle on SCL
Alexander Sverdlin
alexander.sverdlin at nokia.com
Wed Jun 17 02:19:28 PDT 2015
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 at 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;
>
--
Best regards,
Alexander Sverdlin.
More information about the linux-arm-kernel
mailing list