Jokiniemi >-----Original Message----- >From: linux-omap-owner@xxxxxxxxxxxxxxx [mailto:linux-omap-owner@xxxxxxxxxxxxxxx] On Behalf Of Kalle >Jokiniemi >Sent: Thursday, September 17, 2009 11:29 AM >To: khilman@xxxxxxxxxxxxxxxxxxx >Cc: jhnikula@xxxxxxxxx; linux-omap@xxxxxxxxxxxxxxx; Kalle Jokiniemi >Subject: [PATCH 1/1] OMAP: I2C: Add mpu wake up latency constraint in i2c > >While waiting for completion of the i2c transfer, the >MPU could hit OFF mode and cause several msecs of >delay that made i2c transfers fail more often. The How many bytes i2c read were you doing, that failed? What OPP are you in when you observe these failures? VDD1:OPP? >extra delays and subsequent re-trys cause i2c clocks >to be active more often. This has also an negative >effect on power consumption. > >Added a constraint that allows MPU to wake up in few >hundred usecs, which is roughly the average i2c wait >period. How did you arrive at the number 500us for the wakeup latency? On Zoom2 with Synaptic touch screen controller on I2C-2, at vdd1:opp1 we need to put a wakeup latency of 400us else we see data corruption. Our case is doing a write of 1 byte followed by read of 16 bytes. > >The constraint function is passed as platform data from >plat-omap/i2c.c and applied only on OMAP34XX devices. > >Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@xxxxxxxxx> >--- > arch/arm/plat-omap/i2c.c | 49 +++++++++++++++++++++++++++++++---------- > drivers/i2c/busses/i2c-omap.c | 17 +++++++++++--- > include/linux/i2c-omap.h | 9 +++++++ > 3 files changed, 59 insertions(+), 16 deletions(-) > create mode 100644 include/linux/i2c-omap.h > >diff --git a/arch/arm/plat-omap/i2c.c b/arch/arm/plat-omap/i2c.c >index 8b84839..d43d0ad 100644 >--- a/arch/arm/plat-omap/i2c.c >+++ b/arch/arm/plat-omap/i2c.c >@@ -26,8 +26,10 @@ > #include <linux/kernel.h> > #include <linux/platform_device.h> > #include <linux/i2c.h> >+#include <linux/i2c-omap.h> > #include <mach/irqs.h> > #include <mach/mux.h> >+#include <mach/omap-pm.h> > > #define OMAP_I2C_SIZE 0x3f > #define OMAP1_I2C_BASE 0xfffb3800 >@@ -69,14 +71,14 @@ static struct resource i2c_resources[][2] = { > }, \ > } > >-static u32 i2c_rate[ARRAY_SIZE(i2c_resources)]; >+static struct omap_i2c_bus_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_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX) >- 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_OMAP34XX) >- I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_rate[2]), >+ I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_pdata[2]), > #endif > }; > >@@ -100,6 +102,25 @@ static const int omap34xx_pins[][2] = {}; > > #define OMAP_I2C_CMDLINE_SETUP (BIT(31)) > >+/** >+ * omap_i2c_set_wfc_mpu_wkup_lat - sets mpu wake up constraint >+ * @dev: i2c bus device pointer >+ * @set: set or clear constraints >+ * >+ * When waiting for completion of a i2c transfer, we need to set a wake up >+ * latency constraint for the MPU. This is to ensure quick enough wakeup >+ * from idle, when transfer completes. >+ */ >+#ifdef CONFIG_ARCH_OMAP34XX >+static void omap_i2c_set_wfc_mpu_wkup_lat(struct device *dev, int set) >+{ >+ omap_pm_set_max_mpu_wakeup_lat(dev, set ? 500 : -1); >+} >+#else >+static inline void omap_i2c_set_wfc_mpu_wkup_lat(struct device *dev, int set) >+{; } >+#endif >+ > static void __init omap_i2c_mux_pins(int bus) > { > int scl, sda; >@@ -180,8 +201,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].clkrate = ints[2]; >+ i2c_pdata[ints[1] - 1].clkrate |= OMAP_I2C_CMDLINE_SETUP; > > return 1; > } >@@ -195,9 +216,11 @@ static int __init omap_register_i2c_bus_cmdline(void) > { > int i, err = 0; > >- for (i = 0; i < ARRAY_SIZE(i2c_rate); i++) >- if (i2c_rate[i] & OMAP_I2C_CMDLINE_SETUP) { >- i2c_rate[i] &= ~OMAP_I2C_CMDLINE_SETUP; >+ for (i = 0; i < ARRAY_SIZE(i2c_pdata); i++) >+ if (i2c_pdata[i].clkrate & OMAP_I2C_CMDLINE_SETUP) { >+ i2c_pdata[i].clkrate &= ~OMAP_I2C_CMDLINE_SETUP; >+ i2c_pdata[i].set_mpu_wkup_lat = >+ omap_i2c_set_wfc_mpu_wkup_lat; > err = omap_i2c_add_bus(i + 1); > if (err) > goto out; >@@ -231,9 +254,11 @@ int __init omap_register_i2c_bus(int bus_id, u32 clkrate, > return err; > } > >- if (!i2c_rate[bus_id - 1]) >- i2c_rate[bus_id - 1] = clkrate; >- i2c_rate[bus_id - 1] &= ~OMAP_I2C_CMDLINE_SETUP; >+ if (!i2c_pdata[bus_id - 1].clkrate) >+ i2c_pdata[bus_id - 1].clkrate = clkrate; >+ >+ i2c_pdata[bus_id - 1].set_mpu_wkup_lat = omap_i2c_set_wfc_mpu_wkup_lat; >+ i2c_pdata[bus_id - 1].clkrate &= ~OMAP_I2C_CMDLINE_SETUP; > > return omap_i2c_add_bus(bus_id); > } >diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c >index 75bf3ad..da6d276 100644 >--- a/drivers/i2c/busses/i2c-omap.c >+++ b/drivers/i2c/busses/i2c-omap.c >@@ -37,6 +37,7 @@ > #include <linux/platform_device.h> > #include <linux/clk.h> > #include <linux/io.h> >+#include <linux/i2c-omap.h> > > /* I2C controller revisions */ > #define OMAP_I2C_REV_2 0x20 >@@ -165,6 +166,8 @@ struct omap_i2c_dev { > struct clk *fclk; /* Functional clock */ > struct completion cmd_complete; > struct resource *ioarea; >+ void (*set_mpu_wkup_lat)(struct device *dev, >+ int set); > u32 speed; /* Speed of bus in Khz */ > u16 cmd_err; > u8 *buf; >@@ -526,8 +529,10 @@ static int omap_i2c_xfer_msg(struct i2c_adapter *adap, > * REVISIT: We should abort the transfer on signals, but the bus goes > * into arbitration and we're currently unable to recover from it. > */ >+ dev->set_mpu_wkup_lat(dev->dev, 1); > r = wait_for_completion_timeout(&dev->cmd_complete, > OMAP_I2C_TIMEOUT); >+ dev->set_mpu_wkup_lat(dev->dev, 0); > dev->buf_len = 0; > if (r < 0) > return r; >@@ -844,6 +849,7 @@ omap_i2c_probe(struct platform_device *pdev) > struct omap_i2c_dev *dev; > struct i2c_adapter *adap; > struct resource *mem, *irq, *ioarea; >+ struct omap_i2c_bus_platform_data *pdata = pdev->dev.platform_data; > irq_handler_t isr; > int r; > u32 speed = 0; >@@ -873,10 +879,13 @@ omap_i2c_probe(struct platform_device *pdev) > goto err_release_region; > } > >- if (pdev->dev.platform_data != NULL) >- speed = *(u32 *)pdev->dev.platform_data; >- else >- speed = 100; /* Defualt speed */ >+ if (pdata != NULL) { >+ speed = pdata->clkrate; >+ dev->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat; >+ } else { >+ speed = 100; /* Default speed */ >+ dev->set_mpu_wkup_lat = NULL; >+ } > > dev->speed = speed; > dev->idle = 1; >diff --git a/include/linux/i2c-omap.h b/include/linux/i2c-omap.h >new file mode 100644 >index 0000000..1362fba >--- /dev/null >+++ b/include/linux/i2c-omap.h >@@ -0,0 +1,9 @@ >+#ifndef __I2C_OMAP_H__ >+#define __I2C_OMAP_H__ >+ >+struct omap_i2c_bus_platform_data { >+ u32 clkrate; >+ void (*set_mpu_wkup_lat)(struct device *dev, int set); >+}; >+ >+#endif >-- >1.5.4.3 > >-- >To unsubscribe from this list: send the line "unsubscribe linux-omap" in >the body of a message to majordomo@xxxxxxxxxxxxxxx >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 majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
