[PATCH 0/1] i2c PM change for omap-testing

[PATCH 0/1] i2c PM change for omap-testing

[Kevin Hilman]

This I2C PM patch has been in the PM branch for awhile.  This should
be queued in omap-testing with the other I2C changes targetted for
upstream.

Kevin


Kalle Jokiniemi (1):
  i2c-omap: add mpu wake up latency constraint in i2c

 arch/arm/plat-omap/i2c.c      |   55 ++++++++++++++++++++++++++++++++---------
 drivers/i2c/busses/i2c-omap.c |   24 +++++++++++++++---
 include/linux/i2c-omap.h      |    9 ++++++
 3 files changed, 72 insertions(+), 16 deletions(-)
 create mode 100644 include/linux/i2c-omap.h

[PATCH 1/1] i2c-omap: add mpu wake up latency constraint in i2c

[Kevin Hilman]

From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>

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@ti.com>
Cc: Jarkko Nikula <jhnikula@gmail.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Nishanth Menon <nm@ti.com>
Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
---
 arch/arm/plat-omap/i2c.c      |   55 ++++++++++++++++++++++++++++++++---------
 drivers/i2c/busses/i2c-omap.c |   24 +++++++++++++++---
 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 624e262..36011a1 100644
--- a/arch/arm/plat-omap/i2c.c
+++ b/arch/arm/plat-omap/i2c.c
@@ -26,9 +26,12 @@
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/i2c.h>
+#include <linux/i2c-omap.h>
+
 #include <mach/irqs.h>
 #include <plat/mux.h>
 #include <plat/i2c.h>
+#include <plat/omap-pm.h>
 
 #define OMAP_I2C_SIZE		0x3f
 #define OMAP1_I2C_BASE		0xfffb3800
@@ -70,19 +73,44 @@ 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_OMAP2) || defined(CONFIG_ARCH_OMAP3)
-	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_OMAP3)
-	I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_rate[2]),
+	I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_pdata[2]),
 #endif
 };
 
 #define OMAP_I2C_CMDLINE_SETUP	(BIT(31))
 
+#ifdef CONFIG_ARCH_OMAP3
+/*
+ * 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 int __init omap_i2c_nr_ports(void)
 {
 	int ports = 0;
@@ -146,8 +174,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;
 }
@@ -161,9 +189,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;
@@ -197,9 +226,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 f2019d2..7a80b00 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
@@ -174,6 +175,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;
@@ -602,8 +606,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;
@@ -926,6 +934,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;
@@ -955,10 +964,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;
@@ -1011,6 +1023,10 @@ 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.7.0.2

Re: [PATCH 1/1] i2c-omap: add mpu wake up latency constraint in i2c

[Paul Walmsley]

On Wed, 21 Apr 2010, Kevin Hilman wrote:

> From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> 
> 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@ti.com>
> Cc: Jarkko Nikula <jhnikula@gmail.com>
> Cc: Paul Walmsley <paul@pwsan.com>
> Cc: Nishanth Menon <nm@ti.com>
> Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>

Acked-by: Paul Walmsley <paul@pwsan.com>


- Paul

Re: [PATCH 1/1] i2c-omap: add mpu wake up latency constraint in i2c

[Tony Lindgren]

* Kevin Hilman <khilman@deeprootsystems.com> [100421 11:59]:
> From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> 
> 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.

<snip>

> +#ifdef CONFIG_ARCH_OMAP3
> +/*
> + * 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 int __init omap_i2c_nr_ports(void)
>  {
>  	int ports = 0;

<snip>

> @@ -161,9 +189,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;
> @@ -197,9 +226,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);
>  }

Looks like this all could be done in omap_i2c_add_bus a bit simpler.

Regards,

Tony

Re: [PATCH 1/1] i2c-omap: add mpu wake up latency constraint in i2c

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Kevin Hilman <khilman@deeprootsystems.com> [100421 11:59]:
>> From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
>> 
>> 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.
>

[...]

>>  
>> -	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);
>>  }
>
> Looks like this all could be done in omap_i2c_add_bus a bit simpler.
>

OK, here's updated version.

>From 319b439fac05d973dc5800fd061892b70f961708 Mon Sep 17 00:00:00 2001
From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
Date: Wed, 21 Oct 2009 14:51:21 +0300
Subject: [PATCH] i2c-omap: 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
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@ti.com>
Cc: Jarkko Nikula <jhnikula@gmail.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Nishanth Menon <nm@ti.com>
Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
---
Based on current omap-testing branch.

Changes from previous version, as requested by Tony:

Remove omap_set_i2c_constraint_func() in favor of conditionally
adding the flag in omap_i2c_add_bus() in order to keep all the
OMAP conditional checking in a single location.

 arch/arm/plat-omap/i2c.c      |   48 ++++++++++++++++++++++++++++++----------
 drivers/i2c/busses/i2c-omap.c |   24 +++++++++++++++++---
 include/linux/i2c-omap.h      |    9 +++++++
 3 files changed, 65 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 624e262..9ceeca8 100644
--- a/arch/arm/plat-omap/i2c.c
+++ b/arch/arm/plat-omap/i2c.c
@@ -26,9 +26,12 @@
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/i2c.h>
+#include <linux/i2c-omap.h>
+
 #include <mach/irqs.h>
 #include <plat/mux.h>
 #include <plat/i2c.h>
+#include <plat/omap-pm.h>
 
 #define OMAP_I2C_SIZE		0x3f
 #define OMAP1_I2C_BASE		0xfffb3800
@@ -70,19 +73,35 @@ 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_OMAP2) || defined(CONFIG_ARCH_OMAP3)
-	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_OMAP3)
-	I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_rate[2]),
+	I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_pdata[2]),
 #endif
 };
 
 #define OMAP_I2C_CMDLINE_SETUP	(BIT(31))
 
+#ifdef CONFIG_ARCH_OMAP3
+/*
+ * 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 int __init omap_i2c_nr_ports(void)
 {
 	int ports = 0;
@@ -100,10 +119,12 @@ static int __init omap_i2c_nr_ports(void)
 static int __init omap_i2c_add_bus(int bus_id)
 {
 	struct platform_device *pdev;
+	struct omap_i2c_bus_platform_data *pd;
 	struct resource *res;
 	resource_size_t base, irq;
 
 	pdev = &omap_i2c_devices[bus_id - 1];
+	pd = pdev->dev.platform_data;
 	if (bus_id == 1) {
 		res = pdev->resource;
 		if (cpu_class_is_omap1()) {
@@ -122,6 +143,8 @@ static int __init omap_i2c_add_bus(int bus_id)
 		omap1_i2c_mux_pins(bus_id);
 	if (cpu_class_is_omap2())
 		omap2_i2c_mux_pins(bus_id);
+	if (cpu_is_omap34xx())
+		pd->set_mpu_wkup_lat = omap_i2c_set_wfc_mpu_wkup_lat;
 
 	return platform_device_register(pdev);
 }
@@ -146,8 +169,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;
 }
@@ -161,9 +184,9 @@ 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;
 			err = omap_i2c_add_bus(i + 1);
 			if (err)
 				goto out;
@@ -197,9 +220,10 @@ 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].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 b81484d..f0d241f 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -38,6 +38,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/slab.h>
+#include <linux/i2c-omap.h>
 
 /* I2C controller revisions */
 #define OMAP_I2C_REV_2			0x20
@@ -175,6 +176,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;
@@ -603,8 +607,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;
@@ -927,6 +935,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;
@@ -956,10 +965,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;
@@ -1012,6 +1024,10 @@ 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.7.0.2

Re: [PATCH 1/1] i2c-omap: add mpu wake up latency constraint in i2c

[Tony Lindgren]

* Kevin Hilman <khilman@deeprootsystems.com> [100426 15:57]:
> Tony Lindgren <tony@atomide.com> writes:
>  
> +#ifdef CONFIG_ARCH_OMAP3
> +/*
> + * 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
> +

Can't you leave out the above too?

> +	struct omap_i2c_bus_platform_data *pd;
>  	struct resource *res;
>  	resource_size_t base, irq;
>  
>  	pdev = &omap_i2c_devices[bus_id - 1];
> +	pd = pdev->dev.platform_data;
>  	if (bus_id == 1) {
>  		res = pdev->resource;
>  		if (cpu_class_is_omap1()) {
> @@ -122,6 +143,8 @@ static int __init omap_i2c_add_bus(int bus_id)
>  		omap1_i2c_mux_pins(bus_id);
>  	if (cpu_class_is_omap2())
>  		omap2_i2c_mux_pins(bus_id);
> +	if (cpu_is_omap34xx())
> +		pd->set_mpu_wkup_lat = omap_i2c_set_wfc_mpu_wkup_lat;

And then just set
		pd->set_mpu_wkup_lat = omap_pm_set_max_mpu_wakeup_lat;

Here instead?

BTW, in your version looks like compile will break if CONFIG_ARCH_OMAP3
is not set.

Tony

Re: [PATCH 1/1] i2c-omap: add mpu wake up latency constraint in i2c

[Kevin Hilman]

Tony Lindgren <tony@atomide.com> writes:

> * Kevin Hilman <khilman@deeprootsystems.com> [100426 15:57]:
>> Tony Lindgren <tony@atomide.com> writes:
>>  
>> +#ifdef CONFIG_ARCH_OMAP3
>> +/*
>> + * 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
>> +
>
> Can't you leave out the above too?
>
>> +	struct omap_i2c_bus_platform_data *pd;
>>  	struct resource *res;
>>  	resource_size_t base, irq;
>>  
>>  	pdev = &omap_i2c_devices[bus_id - 1];
>> +	pd = pdev->dev.platform_data;
>>  	if (bus_id == 1) {
>>  		res = pdev->resource;
>>  		if (cpu_class_is_omap1()) {
>> @@ -122,6 +143,8 @@ static int __init omap_i2c_add_bus(int bus_id)
>>  		omap1_i2c_mux_pins(bus_id);
>>  	if (cpu_class_is_omap2())
>>  		omap2_i2c_mux_pins(bus_id);
>> +	if (cpu_is_omap34xx())
>> +		pd->set_mpu_wkup_lat = omap_i2c_set_wfc_mpu_wkup_lat;
>
> And then just set
> 		pd->set_mpu_wkup_lat = omap_pm_set_max_mpu_wakeup_lat;
>
> Here instead?

Sure.

> BTW, in your version looks like compile will break if CONFIG_ARCH_OMAP3
> is not set.

it shouldn't since if !CONFIG_ARCH_OMAP3, cpu_is_omap34xx() is zero
and that code will not be compiled.

Anyways, here's updated version which gets rid of both problems.

Kevin

>From 4dadd7a580c347d6d93ca3cdb7fbf6bd1bab9440 Mon Sep 17 00:00:00 2001
From: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
Date: Wed, 21 Oct 2009 14:51:21 +0300
Subject: [PATCH] i2c-omap: 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
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.

Updates by Kevin as requested by Tony:

- Remove omap_set_i2c_constraint_func() in favor of conditionally
  adding the flag in omap_i2c_add_bus() in order to keep all the OMAP
  conditional checking in a single location.
- Update set_mpu_wkup_lat prototypes to match OMAP PM layer so
  OMAP PM function can be used directly in pdata.

Cc: Moiz Sonasath <m-sonasath@ti.com>
Cc: Jarkko Nikula <jhnikula@gmail.com>
Cc: Paul Walmsley <paul@pwsan.com>
Cc: Nishanth Menon <nm@ti.com>
Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
Signed-off-by: Kevin Hilman <khilman@deeprootsystems.com>
---
 arch/arm/plat-omap/i2c.c      |   39 +++++++++++++++++++++++++++------------
 drivers/i2c/busses/i2c-omap.c |   24 ++++++++++++++++++++----
 include/linux/i2c-omap.h      |    9 +++++++++
 3 files changed, 56 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 624e262..7b7b12d 100644
--- a/arch/arm/plat-omap/i2c.c
+++ b/arch/arm/plat-omap/i2c.c
@@ -26,9 +26,12 @@
 #include <linux/kernel.h>
 #include <linux/platform_device.h>
 #include <linux/i2c.h>
+#include <linux/i2c-omap.h>
+
 #include <mach/irqs.h>
 #include <plat/mux.h>
 #include <plat/i2c.h>
+#include <plat/omap-pm.h>
 
 #define OMAP_I2C_SIZE		0x3f
 #define OMAP1_I2C_BASE		0xfffb3800
@@ -70,14 +73,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_OMAP2) || defined(CONFIG_ARCH_OMAP3)
-	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_OMAP3)
-	I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_rate[2]),
+	I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_pdata[2]),
 #endif
 };
 
@@ -100,10 +103,12 @@ static int __init omap_i2c_nr_ports(void)
 static int __init omap_i2c_add_bus(int bus_id)
 {
 	struct platform_device *pdev;
+	struct omap_i2c_bus_platform_data *pd;
 	struct resource *res;
 	resource_size_t base, irq;
 
 	pdev = &omap_i2c_devices[bus_id - 1];
+	pd = pdev->dev.platform_data;
 	if (bus_id == 1) {
 		res = pdev->resource;
 		if (cpu_class_is_omap1()) {
@@ -122,6 +127,15 @@ static int __init omap_i2c_add_bus(int bus_id)
 		omap1_i2c_mux_pins(bus_id);
 	if (cpu_class_is_omap2())
 		omap2_i2c_mux_pins(bus_id);
+	
+	/*
+	 * 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 (cpu_is_omap34xx())
+		pd->set_mpu_wkup_lat = omap_pm_set_max_mpu_wakeup_lat;
 
 	return platform_device_register(pdev);
 }
@@ -146,8 +160,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;
 }
@@ -161,9 +175,9 @@ 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;
 			err = omap_i2c_add_bus(i + 1);
 			if (err)
 				goto out;
@@ -197,9 +211,10 @@ 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].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 b81484d..a866e4e 100644
--- a/drivers/i2c/busses/i2c-omap.c
+++ b/drivers/i2c/busses/i2c-omap.c
@@ -38,6 +38,7 @@
 #include <linux/clk.h>
 #include <linux/io.h>
 #include <linux/slab.h>
+#include <linux/i2c-omap.h>
 
 /* I2C controller revisions */
 #define OMAP_I2C_REV_2			0x20
@@ -175,6 +176,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,
+						    long latency);
 	u32			speed;		/* Speed of bus in Khz */
 	u16			cmd_err;
 	u8			*buf;
@@ -603,8 +607,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;
@@ -927,6 +935,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;
@@ -956,10 +965,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;
@@ -1012,6 +1024,10 @@ 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..78ebf50
--- /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, long set);
+};
+
+#endif
-- 
1.7.0.2

Re: [PATCH 1/1] i2c-omap: add mpu wake up latency constraint in i2c

[Tony Lindgren]

* Kevin Hilman <khilman@deeprootsystems.com> [100426 16:27]:
> >> +	if (cpu_is_omap34xx())
> >> +		pd->set_mpu_wkup_lat = omap_i2c_set_wfc_mpu_wkup_lat;
> 
> > BTW, in your version looks like compile will break if CONFIG_ARCH_OMAP3
> > is not set.
> 
> it shouldn't since if !CONFIG_ARCH_OMAP3, cpu_is_omap34xx() is zero
> and that code will not be compiled.

Yes but look how there's no else after #ifdef CONFIG_ARCH_OMAP3..

The if (0) does not help if the code refers something that's
undefined, in this case omap_i2c_set_wfc_mpu_wkup_lat would be
undefined. So I'd assume you'll get something like when
CONFIG_ARCH_OMAP3 is not set:

error: ‘omap_i2c_set_wfc_mpu_wkup_lat’ undeclared (first use in this function)

One way to fix that would be to have #else statement with
#define omap_i2c_set_wfc_mpu_wkup_lat	NULL

Anyways, commenting on this just based on my virtual preprocessor,
I have not actually tried it :)
 
> Anyways, here's updated version which gets rid of both problems.

Thanks! Will add to i2c-omap-for-ben branch.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html