| Message ID | alpine.DEB.2.00.1211061619461.20573@utopia.booyaka.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Nov 6, 2012 at 5:31 PM, Paul Walmsley <paul@pwsan.com> wrote: > > This reverts commit 3db11feffc1ad2ab9dea27789e6b5b3032827adc. > This commit causes I2C timeouts to appear on several OMAP3430/3530-based > boards: > > http://marc.info/?l=linux-arm-kernel&m=135071372426971&w=2 > http://marc.info/?l=linux-arm-kernel&m=135067558415214&w=2 > http://marc.info/?l=linux-arm-kernel&m=135216013608196&w=2 > > and appears to have been sent for merging before one of its prerequisites > was merged: > > http://marc.info/?l=linux-arm-kernel&m=135219411617621&w=2 Indeed. Acked-by: Jean Pihet <j-pihet@ti.com> > > Signed-off-by: Paul Walmsley <paul@pwsan.com> > Cc: Jean Pihet <jean.pihet@newoldbits.com> > Cc: Kevin Hilman <khilman@ti.com> > Cc: Aaro Koskinen <aaro.koskinen@iki.fi> > Cc: Felipe Balbi <balbi@ti.com> > --- > > Intended for 3.7-rc. > > arch/arm/plat-omap/i2c.c | 21 +++++++++++++++++++++ > drivers/i2c/busses/i2c-omap.c | 32 ++++++++++++++------------------ > include/linux/i2c-omap.h | 1 + > 3 files changed, 36 insertions(+), 18 deletions(-) > > diff --git a/arch/arm/plat-omap/i2c.c b/arch/arm/plat-omap/i2c.c > index a5683a8..6013831 100644 > --- a/arch/arm/plat-omap/i2c.c > +++ b/arch/arm/plat-omap/i2c.c > @@ -26,12 +26,14 @@ > #include <linux/kernel.h> > #include <linux/platform_device.h> > #include <linux/i2c.h> > +#include <linux/i2c-omap.h> > #include <linux/slab.h> > #include <linux/err.h> > #include <linux/clk.h> > > #include <mach/irqs.h> > #include <plat/i2c.h> > +#include <plat/omap-pm.h> > #include <plat/omap_device.h> > > #define OMAP_I2C_SIZE 0x3f > @@ -127,6 +129,16 @@ static inline int omap1_i2c_add_bus(int bus_id) > > > #ifdef CONFIG_ARCH_OMAP2PLUS > +/* > + * XXX This function is a temporary compatibility wrapper - only > + * needed until the I2C driver can be converted to call > + * omap_pm_set_max_dev_wakeup_lat() and handle a return code. > + */ > +static void omap_pm_set_max_mpu_wakeup_lat_compat(struct device *dev, long t) > +{ > + omap_pm_set_max_mpu_wakeup_lat(dev, t); > +} > + > static inline int omap2_i2c_add_bus(int bus_id) > { > int l; > @@ -158,6 +170,15 @@ static inline int omap2_i2c_add_bus(int bus_id) > dev_attr = (struct omap_i2c_dev_attr *)oh->dev_attr; > pdata->flags = dev_attr->flags; > > + /* > + * 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. > + * Only omap3 has support for constraints > + */ > + if (cpu_is_omap34xx()) > + pdata->set_mpu_wkup_lat = omap_pm_set_max_mpu_wakeup_lat_compat; > pdev = omap_device_build(name, bus_id, oh, pdata, > sizeof(struct omap_i2c_bus_platform_data), > NULL, 0, 0); > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index db31eae..0b02543 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -43,7 +43,6 @@ > #include <linux/slab.h> > #include <linux/i2c-omap.h> > #include <linux/pm_runtime.h> > -#include <linux/pm_qos.h> > > /* I2C controller revisions */ > #define OMAP_I2C_OMAP1_REV_2 0x20 > @@ -187,8 +186,9 @@ struct omap_i2c_dev { > int reg_shift; /* bit shift for I2C register addresses */ > struct completion cmd_complete; > struct resource *ioarea; > - u32 latency; /* maximum MPU wkup latency */ > - struct pm_qos_request pm_qos_request; > + u32 latency; /* maximum mpu wkup latency */ > + void (*set_mpu_wkup_lat)(struct device *dev, > + long latency); > u32 speed; /* Speed of bus in kHz */ > u32 dtrev; /* extra revision from DT */ > u32 flags; > @@ -494,7 +494,9 @@ static void omap_i2c_resize_fifo(struct omap_i2c_dev *dev, u8 size, bool is_rx) > dev->b_hw = 1; /* Enable hardware fixes */ > > /* calculate wakeup latency constraint for MPU */ > - dev->latency = (1000000 * dev->threshold) / (1000 * dev->speed / 8); > + if (dev->set_mpu_wkup_lat != NULL) > + dev->latency = (1000000 * dev->threshold) / > + (1000 * dev->speed / 8); > } > > /* > @@ -629,16 +631,8 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) > if (r < 0) > goto out; > > - /* > - * 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. > - */ > - if (dev->latency) > - pm_qos_add_request(&dev->pm_qos_request, > - PM_QOS_CPU_DMA_LATENCY, > - dev->latency); > + if (dev->set_mpu_wkup_lat != NULL) > + dev->set_mpu_wkup_lat(dev->dev, dev->latency); > > for (i = 0; i < num; i++) { > r = omap_i2c_xfer_msg(adap, &msgs[i], (i == (num - 1))); > @@ -646,8 +640,8 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) > break; > } > > - if (dev->latency) > - pm_qos_remove_request(&dev->pm_qos_request); > + if (dev->set_mpu_wkup_lat != NULL) > + dev->set_mpu_wkup_lat(dev->dev, -1); > > if (r == 0) > r = num; > @@ -1104,6 +1098,7 @@ omap_i2c_probe(struct platform_device *pdev) > } else if (pdata != NULL) { > dev->speed = pdata->clkrate; > dev->flags = pdata->flags; > + dev->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat; > dev->dtrev = pdata->rev; > } > > @@ -1159,8 +1154,9 @@ omap_i2c_probe(struct platform_device *pdev) > dev->b_hw = 1; /* Enable hardware fixes */ > > /* calculate wakeup latency constraint for MPU */ > - dev->latency = (1000000 * dev->fifo_size) / > - (1000 * dev->speed / 8); > + if (dev->set_mpu_wkup_lat != NULL) > + dev->latency = (1000000 * dev->fifo_size) / > + (1000 * dev->speed / 8); > } > > /* reset ASAP, clearing any IRQs */ > diff --git a/include/linux/i2c-omap.h b/include/linux/i2c-omap.h > index df804ba..92a0dc7 100644 > --- a/include/linux/i2c-omap.h > +++ b/include/linux/i2c-omap.h > @@ -34,6 +34,7 @@ struct omap_i2c_bus_platform_data { > u32 clkrate; > u32 rev; > u32 flags; > + void (*set_mpu_wkup_lat)(struct device *dev, long set); > }; > > #endif > -- > 1.7.10.4
On Tue, Nov 6, 2012 at 10:01 PM, Paul Walmsley <paul@pwsan.com> wrote: > > This reverts commit 3db11feffc1ad2ab9dea27789e6b5b3032827adc. > This commit causes I2C timeouts to appear on several OMAP3430/3530-based > boards: > > http://marc.info/?l=linux-arm-kernel&m=135071372426971&w=2 > http://marc.info/?l=linux-arm-kernel&m=135067558415214&w=2 > http://marc.info/?l=linux-arm-kernel&m=135216013608196&w=2 > > and appears to have been sent for merging before one of its prerequisites > was merged: > > http://marc.info/?l=linux-arm-kernel&m=135219411617621&w=2 > Not a comment however was curious does merging the dependency. make the issue go away?
On Tue, Nov 06, 2012 at 04:31:32PM +0000, Paul Walmsley wrote: > > This reverts commit 3db11feffc1ad2ab9dea27789e6b5b3032827adc. Here giving the patch name in parens would have really made sense. Will fix that. > This commit causes I2C timeouts to appear on several OMAP3430/3530-based > boards: > > http://marc.info/?l=linux-arm-kernel&m=135071372426971&w=2 > http://marc.info/?l=linux-arm-kernel&m=135067558415214&w=2 > http://marc.info/?l=linux-arm-kernel&m=135216013608196&w=2 > > and appears to have been sent for merging before one of its prerequisites > was merged: > > http://marc.info/?l=linux-arm-kernel&m=135219411617621&w=2 Hmm, any ideas how to avoid such things in the future? > Signed-off-by: Paul Walmsley <paul@pwsan.com> Applied to for-current, thanks!
Hi Wolfram, On Wed, Nov 14, 2012 at 11:51 AM, Wolfram Sang <w.sang@pengutronix.de> wrote: > On Tue, Nov 06, 2012 at 04:31:32PM +0000, Paul Walmsley wrote: >> >> This reverts commit 3db11feffc1ad2ab9dea27789e6b5b3032827adc. > > Here giving the patch name in parens would have really made sense. > Will fix that. The title is "ARM: OMAP: convert I2C driver to PM QoS for MPU latency constraints". > >> This commit causes I2C timeouts to appear on several OMAP3430/3530-based >> boards: >> >> http://marc.info/?l=linux-arm-kernel&m=135071372426971&w=2 >> http://marc.info/?l=linux-arm-kernel&m=135067558415214&w=2 >> http://marc.info/?l=linux-arm-kernel&m=135216013608196&w=2 There is no formal piecof evidence that the commit is the cause of it. >> >> and appears to have been sent for merging before one of its prerequisites >> was merged: However this is correct. My fault ;-| >> >> http://marc.info/?l=linux-arm-kernel&m=135219411617621&w=2 > > Hmm, any ideas how to avoid such things in the future? The only way is to figure out the dependencies with other features. In this case I overlooked them and assumed some other features were already merged in. Will take care next time. > >> Signed-off-by: Paul Walmsley <paul@pwsan.com> > > Applied to for-current, thanks! Thanks! Jean > > -- > Pengutronix e.K. | Wolfram Sang | > Industrial Linux Solutions | http://www.pengutronix.de/ |
diff --git a/arch/arm/plat-omap/i2c.c b/arch/arm/plat-omap/i2c.c index a5683a8..6013831 100644 --- a/arch/arm/plat-omap/i2c.c +++ b/arch/arm/plat-omap/i2c.c @@ -26,12 +26,14 @@ #include <linux/kernel.h> #include <linux/platform_device.h> #include <linux/i2c.h> +#include <linux/i2c-omap.h> #include <linux/slab.h> #include <linux/err.h> #include <linux/clk.h> #include <mach/irqs.h> #include <plat/i2c.h> +#include <plat/omap-pm.h> #include <plat/omap_device.h> #define OMAP_I2C_SIZE 0x3f @@ -127,6 +129,16 @@ static inline int omap1_i2c_add_bus(int bus_id) #ifdef CONFIG_ARCH_OMAP2PLUS +/* + * XXX This function is a temporary compatibility wrapper - only + * needed until the I2C driver can be converted to call + * omap_pm_set_max_dev_wakeup_lat() and handle a return code. + */ +static void omap_pm_set_max_mpu_wakeup_lat_compat(struct device *dev, long t) +{ + omap_pm_set_max_mpu_wakeup_lat(dev, t); +} + static inline int omap2_i2c_add_bus(int bus_id) { int l; @@ -158,6 +170,15 @@ static inline int omap2_i2c_add_bus(int bus_id) dev_attr = (struct omap_i2c_dev_attr *)oh->dev_attr; pdata->flags = dev_attr->flags; + /* + * 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. + * Only omap3 has support for constraints + */ + if (cpu_is_omap34xx()) + pdata->set_mpu_wkup_lat = omap_pm_set_max_mpu_wakeup_lat_compat; pdev = omap_device_build(name, bus_id, oh, pdata, sizeof(struct omap_i2c_bus_platform_data), NULL, 0, 0); diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index db31eae..0b02543 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -43,7 +43,6 @@ #include <linux/slab.h> #include <linux/i2c-omap.h> #include <linux/pm_runtime.h> -#include <linux/pm_qos.h> /* I2C controller revisions */ #define OMAP_I2C_OMAP1_REV_2 0x20 @@ -187,8 +186,9 @@ struct omap_i2c_dev { int reg_shift; /* bit shift for I2C register addresses */ struct completion cmd_complete; struct resource *ioarea; - u32 latency; /* maximum MPU wkup latency */ - struct pm_qos_request pm_qos_request; + u32 latency; /* maximum mpu wkup latency */ + void (*set_mpu_wkup_lat)(struct device *dev, + long latency); u32 speed; /* Speed of bus in kHz */ u32 dtrev; /* extra revision from DT */ u32 flags; @@ -494,7 +494,9 @@ static void omap_i2c_resize_fifo(struct omap_i2c_dev *dev, u8 size, bool is_rx) dev->b_hw = 1; /* Enable hardware fixes */ /* calculate wakeup latency constraint for MPU */ - dev->latency = (1000000 * dev->threshold) / (1000 * dev->speed / 8); + if (dev->set_mpu_wkup_lat != NULL) + dev->latency = (1000000 * dev->threshold) / + (1000 * dev->speed / 8); } /* @@ -629,16 +631,8 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) if (r < 0) goto out; - /* - * 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. - */ - if (dev->latency) - pm_qos_add_request(&dev->pm_qos_request, - PM_QOS_CPU_DMA_LATENCY, - dev->latency); + if (dev->set_mpu_wkup_lat != NULL) + dev->set_mpu_wkup_lat(dev->dev, dev->latency); for (i = 0; i < num; i++) { r = omap_i2c_xfer_msg(adap, &msgs[i], (i == (num - 1))); @@ -646,8 +640,8 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) break; } - if (dev->latency) - pm_qos_remove_request(&dev->pm_qos_request); + if (dev->set_mpu_wkup_lat != NULL) + dev->set_mpu_wkup_lat(dev->dev, -1); if (r == 0) r = num; @@ -1104,6 +1098,7 @@ omap_i2c_probe(struct platform_device *pdev) } else if (pdata != NULL) { dev->speed = pdata->clkrate; dev->flags = pdata->flags; + dev->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat; dev->dtrev = pdata->rev; } @@ -1159,8 +1154,9 @@ omap_i2c_probe(struct platform_device *pdev) dev->b_hw = 1; /* Enable hardware fixes */ /* calculate wakeup latency constraint for MPU */ - dev->latency = (1000000 * dev->fifo_size) / - (1000 * dev->speed / 8); + if (dev->set_mpu_wkup_lat != NULL) + dev->latency = (1000000 * dev->fifo_size) / + (1000 * dev->speed / 8); } /* reset ASAP, clearing any IRQs */ diff --git a/include/linux/i2c-omap.h b/include/linux/i2c-omap.h index df804ba..92a0dc7 100644 --- a/include/linux/i2c-omap.h +++ b/include/linux/i2c-omap.h @@ -34,6 +34,7 @@ struct omap_i2c_bus_platform_data { u32 clkrate; u32 rev; u32 flags; + void (*set_mpu_wkup_lat)(struct device *dev, long set); }; #endif
This reverts commit 3db11feffc1ad2ab9dea27789e6b5b3032827adc. This commit causes I2C timeouts to appear on several OMAP3430/3530-based boards: http://marc.info/?l=linux-arm-kernel&m=135071372426971&w=2 http://marc.info/?l=linux-arm-kernel&m=135067558415214&w=2 http://marc.info/?l=linux-arm-kernel&m=135216013608196&w=2 and appears to have been sent for merging before one of its prerequisites was merged: http://marc.info/?l=linux-arm-kernel&m=135219411617621&w=2 Signed-off-by: Paul Walmsley <paul@pwsan.com> Cc: Jean Pihet <jean.pihet@newoldbits.com> Cc: Kevin Hilman <khilman@ti.com> Cc: Aaro Koskinen <aaro.koskinen@iki.fi> Cc: Felipe Balbi <balbi@ti.com> --- Intended for 3.7-rc. arch/arm/plat-omap/i2c.c | 21 +++++++++++++++++++++ drivers/i2c/busses/i2c-omap.c | 32 ++++++++++++++------------------ include/linux/i2c-omap.h | 1 + 3 files changed, 36 insertions(+), 18 deletions(-)