Hi Ben, What's the status on this one? Have you had time to check it out? - Kalle > -----Original Message----- > From: ext Paul Walmsley [mailto:paul@xxxxxxxxx] > Sent: 30. marraskuuta 2009 9:24 > To: Jokiniemi Kalle (Nokia-D/Tampere) > Cc: ben-linux@xxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx; > linux-omap@xxxxxxxxxxxxxxx; Ujfalusi Peter (Nokia-D/Tampere); > Hogander Jouni (Nokia-D/Tampere); > khilman@xxxxxxxxxxxxxxxxxxx; kalle.jokiniemi@xxxxxxxxx; > m-sonasath@xxxxxx; jhnikula@xxxxxxxxx; nm@xxxxxx > Subject: RE: [PATCH] OMAP: I2C: Add mpu wake up latency > constraint in i2c > > 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-i2c" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
