Hi Ben, Kalle, On Thu, 26 Nov 2009, kalle.jokiniemi@xxxxxxxxx wrote: > Could you take this patch to i2c tree? The patch has been discussed in linux-omap list, and this version was the one that was agreed to pushed forward. > > http://marc.info/?l=linux-omap&m=125907598412786&w=2 > > > I tried it on top of linus' tree and it applied fine. Reviewed-by: Paul Walmsley <paul@xxxxxxxxx> Looks good to me. - Paul > > - Kalle > > > > -----Original Message----- > > From: linux-omap-owner@xxxxxxxxxxxxxxx > > [mailto:linux-omap-owner@xxxxxxxxxxxxxxx] On Behalf Of > > Jokiniemi Kalle (Nokia-D/Tampere) > > Sent: 25. marraskuuta 2009 20:04 > > To: ben-linux@xxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx > > Cc: linux-omap@xxxxxxxxxxxxxxx; Ujfalusi Peter > > (Nokia-D/Tampere); Hogander Jouni (Nokia-D/Tampere); > > khilman@xxxxxxxxxxxxxxxxxxx; ext Kalle Jokiniemi; Moiz > > Sonasath; Jarkko Nikula; Paul Walmsley; Nishanth Menon > > Subject: [PATCH] OMAP: I2C: Add mpu wake up latency constraint in i2c > > > > From: ext Kalle Jokiniemi <kalle.jokiniemi@xxxxxxxxx> > > > > 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 extra delays and > > subsequent re-trys cause i2c clocks to be active more often. > > This has also an negative effect on power consumption. > > > > Created a mechanism for passing and using the constraint > > setting function in driver code. The used mpu wake up latency > > constraints are now set individually per bus, and they are > > calculated based on clock rate and fifo size. > > > > Thanks to Jarkko Nikula, Moiz Sonasath, Paul Walmsley, and > > Nishanth Menon for tuning out the details of this patch. > > > > Cc: Moiz Sonasath <m-sonasath@xxxxxx> > > Cc: Jarkko Nikula <jhnikula@xxxxxxxxx> > > Cc: Paul Walmsley <paul@xxxxxxxxx> > > Cc: Nishanth Menon <nm@xxxxxx> > > Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@xxxxxxxxx> > > --- > > arch/arm/plat-omap/i2c.c | 54 > > +++++++++++++++++++++++++++++++--------- > > drivers/i2c/busses/i2c-omap.c | 25 ++++++++++++++++--- > > include/linux/i2c-omap.h | 9 +++++++ > > 3 files changed, 72 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..3c122cd 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,31 @@ static const int omap34xx_pins[][2] = {}; > > > > #define OMAP_I2C_CMDLINE_SETUP (BIT(31)) > > > > +#ifdef CONFIG_ARCH_OMAP34XX > > +/* > > + * omap_i2c_set_wfc_mpu_wkup_lat - sets mpu wake up constraint > > + * @dev: i2c bus device pointer > > + * @val: latency constraint to set, -1 to disable constraint > > + * > > + * 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. > > + */ > > +static void omap_i2c_set_wfc_mpu_wkup_lat(struct device > > *dev, int val) > > +{ > > + omap_pm_set_max_mpu_wakeup_lat(dev, val); } #endif > > + > > +static void __init omap_set_i2c_constraint_func( > > + struct omap_i2c_bus_platform_data *pd) { > > + if (cpu_is_omap34xx()) > > + pd->set_mpu_wkup_lat = omap_i2c_set_wfc_mpu_wkup_lat; > > + else > > + pd->set_mpu_wkup_lat = NULL; > > +} > > + > > static void __init omap_i2c_mux_pins(int bus) { > > int scl, sda; > > @@ -180,8 +207,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 +222,10 @@ 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; > > + omap_set_i2c_constraint_func(&i2c_pdata[i]); > > err = omap_i2c_add_bus(i + 1); > > if (err) > > goto out; > > @@ -231,9 +259,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; > > + > > + omap_set_i2c_constraint_func(&i2c_pdata[bus_id - 1]); > > + 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 827da08..ac0bfd2 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,9 @@ struct omap_i2c_dev { > > struct clk *fclk; /* Functional clock */ > > struct completion cmd_complete; > > struct resource *ioarea; > > + u32 latency; /* maximum mpu > > wkup latency */ > > + void (*set_mpu_wkup_lat)(struct device *dev, > > + int latency); > > u32 speed; /* Speed of bus > > in Khz */ > > u16 cmd_err; > > u8 *buf; > > @@ -508,8 +512,12 @@ 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. > > */ > > + if (dev->set_mpu_wkup_lat != NULL) > > + dev->set_mpu_wkup_lat(dev->dev, dev->latency); > > r = wait_for_completion_timeout(&dev->cmd_complete, > > OMAP_I2C_TIMEOUT); > > + if (dev->set_mpu_wkup_lat != NULL) > > + dev->set_mpu_wkup_lat(dev->dev, -1); > > dev->buf_len = 0; > > if (r < 0) > > return r; > > @@ -826,6 +834,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; > > @@ -855,10 +864,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; > > @@ -893,6 +905,11 @@ omap_i2c_probe(struct platform_device *pdev) > > */ > > dev->fifo_size = (dev->fifo_size / 2); > > dev->b_hw = 1; /* Enable hardware fixes */ > > + > > + /* calculate wakeup latency constraint for MPU */ > > + if (dev->set_mpu_wkup_lat != NULL) > > + dev->latency = (1000000 * dev->fifo_size) / > > + (1000 * speed / 8); > > } > > > > /* reset ASAP, clearing any IRQs */ > > 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.6.3.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 > > - Paul -- 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
