Skip to contentSkip to conversation

Revert "ARM: OMAP: convert I2C driver to PM QoS for MPU latency constraints"

PATCH New Paul Walmsley 4 participants 5 messages Nov 6, 2012 – Nov 14, 2012

Subsystems (6)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Revert "ARM: OMAP: convert I2C driver to PM QoS for MPU latency constraints"

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

Cc Jean Pihet <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Aaro Koskinen <[email protected]>
Cc Felipe Balbi <[email protected]>

---

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(-)
arch/arm/plat-omap/i2c.c +21 -0
index a5683a8..6013831 100644
@@ -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);
drivers/i2c/busses/i2c-omap.c +14 -18
index db31eae..0b02543 100644
@@ -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 */
include/linux/i2c-omap.h +1 -0
index df804ba..92a0dc7 100644
@@ -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
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH 1/1 Revert "ARM: OMAP: convert I2C driver to PM QoS for MPU latency constraints"

Quoted 5 lines · Paul Walmsley

This reverts commit 3db11feffc1ad2ab9dea27789e6b5b3032827adc. This commit causes I2C timeouts to appear on several OMAP3430/3530-based boards:

Quoted 8 lines

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.

Quoted 166 lines
Cc Jean Pihet <[email protected]>
Cc Kevin Hilman <[email protected]>
Cc Aaro Koskinen <[email protected]>
Cc Felipe Balbi <[email protected]>

---

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(-)
arch/arm/plat-omap/i2c.c +21 -0
index a5683a8..6013831 100644
@@ -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);
drivers/i2c/busses/i2c-omap.c +14 -18
index db31eae..0b02543 100644
@@ -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 */
include/linux/i2c-omap.h +1 -1
index df804ba..92a0dc7 100644
@@ -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

#2 lore raw

PATCH 1/1 Revert "ARM: OMAP: convert I2C driver to PM QoS for MPU latency constraints"

In reply to Paul Walmsley
Quoted 5 lines · Paul Walmsley

This reverts commit 3db11feffc1ad2ab9dea27789e6b5b3032827adc. This commit causes I2C timeouts to appear on several OMAP3430/3530-based boards:

Quoted 9 lines

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?

Applied
#3 lore raw

PATCH 1/1 Revert "ARM: OMAP: convert I2C driver to PM QoS for MPU latency constraints"

In reply to Paul Walmsley
Quoted 2 lines · Paul Walmsley

This reverts commit 3db11feffc1ad2ab9dea27789e6b5b3032827adc.

Here giving the patch name in parens would have really made sense. Will fix that.

Quoted 11 lines

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?

Quoted 1 line

Applied to for-current, thanks!

Signature · 9 lines
Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20121114/51cce6bc/attachment.sig>
#4 lore raw

PATCH 1/1 Revert "ARM: OMAP: convert I2C driver to PM QoS for MPU latency constraints"

Hi Wolfram,

Quoted 6 lines · Wolfram Sang
Quoted 2 lines · Paul Walmsley

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".

Quoted 7 lines
Quoted 7 lines

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.

Quoted 3 lines
Quoted 3 lines

and appears to have been sent for merging before one of its prerequisites was merged:

However this is correct. My fault ;-|

Quoted 4 lines
Quoted 2 lines

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.

Quoted 4 lines
Quoted 2 lines

Applied to for-current, thanks!

Thanks!
Jean

Quoted 4 lines

--

Pengutronix e.K.                           | Wolfram Sang                |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |