[PATCH] OMAP: I2C: Add mpu wake up latency constraint in i2c

[PATCH] OMAP: I2C: Add mpu wake up latency constraint in i2c

[Kalle Jokiniemi]

From: ext Kalle Jokiniemi <kalle.jokiniemi-sMOQStClEysAvxtiuMwx3w@public.gmane.org>

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-l0cyMroinI0@public.gmane.org>
Cc: Jarkko Nikula <jhnikula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: Paul Walmsley <paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>
Cc: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org>
Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi-sMOQStClEysAvxtiuMwx3w@public.gmane.org>
---
 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

RE: [PATCH] OMAP: I2C: Add mpu wake up latency constraint in i2c

[kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w]

Hi Ben,

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. 

- Kalle


> -----Original Message-----
> From: linux-omap-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org 
> [mailto:linux-omap-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of 
> Jokiniemi Kalle (Nokia-D/Tampere)
> Sent: 25. marraskuuta 2009 20:04
> To: ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Ujfalusi Peter 
> (Nokia-D/Tampere); Hogander Jouni (Nokia-D/Tampere); 
> khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org; 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-sMOQStClEysAvxtiuMwx3w@public.gmane.org>
> 
> 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-l0cyMroinI0@public.gmane.org>
> Cc: Jarkko Nikula <jhnikula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Paul Walmsley <paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>
> Cc: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi-sMOQStClEysAvxtiuMwx3w@public.gmane.org>
> ---
>  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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at  
> http://vger.kernel.org/majordomo-info.html
> --
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] OMAP: I2C: Add mpu wake up latency constraint in i2c

[Paul Walmsley]

Hi Ben, Kalle,

On Thu, 26 Nov 2009, kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org 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-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>

Looks good to me.

- Paul


> 
> - Kalle
> 
> 
> > -----Original Message-----
> > From: linux-omap-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org 
> > [mailto:linux-omap-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of 
> > Jokiniemi Kalle (Nokia-D/Tampere)
> > Sent: 25. marraskuuta 2009 20:04
> > To: ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Ujfalusi Peter 
> > (Nokia-D/Tampere); Hogander Jouni (Nokia-D/Tampere); 
> > khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org; 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-sMOQStClEysAvxtiuMwx3w@public.gmane.org>
> > 
> > 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-l0cyMroinI0@public.gmane.org>
> > Cc: Jarkko Nikula <jhnikula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Cc: Paul Walmsley <paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>
> > Cc: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org>
> > Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi-sMOQStClEysAvxtiuMwx3w@public.gmane.org>
> > ---
> >  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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at  
> > http://vger.kernel.org/majordomo-info.html
> > 


- Paul

RE: [PATCH] OMAP: I2C: Add mpu wake up latency constraint in i2c

[kalle.jokiniemi]

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@pwsan.com] 
> Sent: 30. marraskuuta 2009 9:24
> To: Jokiniemi Kalle (Nokia-D/Tampere)
> Cc: ben-linux@fluff.org; linux-i2c@vger.kernel.org; 
> linux-omap@vger.kernel.org; Ujfalusi Peter (Nokia-D/Tampere); 
> Hogander Jouni (Nokia-D/Tampere); 
> khilman@deeprootsystems.com; kalle.jokiniemi@digia.com; 
> m-sonasath@ti.com; jhnikula@gmail.com; nm@ti.com
> Subject: RE: [PATCH] OMAP: I2C: Add mpu wake up latency 
> constraint in i2c
> 
> Hi Ben, Kalle,
> 
> On Thu, 26 Nov 2009, kalle.jokiniemi@nokia.com 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@pwsan.com>
> 
> Looks good to me.
> 
> - Paul
> 
> 
> > 
> > - Kalle
> > 
> > 
> > > -----Original Message-----
> > > From: linux-omap-owner@vger.kernel.org 
> > > [mailto:linux-omap-owner@vger.kernel.org] On Behalf Of Jokiniemi 
> > > Kalle (Nokia-D/Tampere)
> > > Sent: 25. marraskuuta 2009 20:04
> > > To: ben-linux@fluff.org; linux-i2c@vger.kernel.org
> > > Cc: linux-omap@vger.kernel.org; Ujfalusi Peter (Nokia-D/Tampere); 
> > > Hogander Jouni (Nokia-D/Tampere); 
> khilman@deeprootsystems.com; 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@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>
> > > ---
> > >  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@vger.kernel.org 
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > 
> 
> 
> - Paul
> 

RE: [PATCH] OMAP: I2C: Add mpu wake up latency constraint in i2c

[kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w]

Hi Jean,

> -----Original Message-----
> From: Jokiniemi Kalle (Nokia-D/Tampere) 
> Sent: 11. joulukuuta 2009 12:47
> To: 'ext Paul Walmsley'; ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org
> Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; 
> Ujfalusi Peter (Nokia-D/Tampere); Hogander Jouni 
> (Nokia-D/Tampere); khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org; 
> kalle.jokiniemi-sMOQStClEysAvxtiuMwx3w@public.gmane.org; m-sonasath-l0cyMroinI0@public.gmane.org; 
> jhnikula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; nm-l0cyMroinI0@public.gmane.org
> Subject: RE: [PATCH] OMAP: I2C: Add mpu wake up latency 
> constraint in i2c
> 
> 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-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org]
> > Sent: 30. marraskuuta 2009 9:24
> > To: Jokiniemi Kalle (Nokia-D/Tampere)
> > Cc: ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; 
> > linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Ujfalusi Peter 
> (Nokia-D/Tampere); Hogander 
> > Jouni (Nokia-D/Tampere); khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org; 
> > kalle.jokiniemi-sMOQStClEysAvxtiuMwx3w@public.gmane.org; m-sonasath-l0cyMroinI0@public.gmane.org; jhnikula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; 
> > nm-l0cyMroinI0@public.gmane.org
> > Subject: RE: [PATCH] OMAP: I2C: Add mpu wake up latency 
> constraint in 
> > i2c
> > 
> > Hi Ben, Kalle,
> > 
> > On Thu, 26 Nov 2009, kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org 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.


Ben is bit unresponsive about my patch. Would you Jean be the current linux-i2c maintainer? Could you take my patch in?

Regards,
Kalle Jokiniemi



> > > 
> > > 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-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>
> > 
> > Looks good to me.
> > 
> > - Paul
> > 
> > 
> > > 
> > > - Kalle
> > > 
> > > 
> > > > -----Original Message-----
> > > > From: linux-omap-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org 
> > > > [mailto:linux-omap-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org] On Behalf Of 
> Jokiniemi 
> > > > Kalle (Nokia-D/Tampere)
> > > > Sent: 25. marraskuuta 2009 20:04
> > > > To: ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > > Cc: linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Ujfalusi Peter 
> (Nokia-D/Tampere); 
> > > > Hogander Jouni (Nokia-D/Tampere);
> > khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org; 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-sMOQStClEysAvxtiuMwx3w@public.gmane.org>
> > > > 
> > > > 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-l0cyMroinI0@public.gmane.org>
> > > > Cc: Jarkko Nikula <jhnikula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > > > Cc: Paul Walmsley <paul-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org>
> > > > Cc: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org>
> > > > Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi-sMOQStClEysAvxtiuMwx3w@public.gmane.org>
> > > > ---
> > > >  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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org 
> > > > 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-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] OMAP: I2C: Add mpu wake up latency constraint in i2c

[Jean Delvare]

On Thu, 17 Dec 2009 16:18:24 +0100, kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org wrote:
> Hi Jean,
> 
> > -----Original Message-----
> > From: Jokiniemi Kalle (Nokia-D/Tampere) 
> > Sent: 11. joulukuuta 2009 12:47
> > To: 'ext Paul Walmsley'; ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org
> > Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; 
> > Ujfalusi Peter (Nokia-D/Tampere); Hogander Jouni 
> > (Nokia-D/Tampere); khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org; 
> > kalle.jokiniemi-sMOQStClEysAvxtiuMwx3w@public.gmane.org; m-sonasath-l0cyMroinI0@public.gmane.org; 
> > jhnikula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; nm-l0cyMroinI0@public.gmane.org
> > Subject: RE: [PATCH] OMAP: I2C: Add mpu wake up latency 
> > constraint in i2c
> > 
> > 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-DWxLp4Yu+b8AvxtiuMwx3w@public.gmane.org]
> > > Sent: 30. marraskuuta 2009 9:24
> > > To: Jokiniemi Kalle (Nokia-D/Tampere)
> > > Cc: ben-linux-elnMNo+KYs3YtjvyW6yDsg@public.gmane.org; linux-i2c-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; 
> > > linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Ujfalusi Peter 
> > (Nokia-D/Tampere); Hogander 
> > > Jouni (Nokia-D/Tampere); khilman-1D3HCaltpLuhEniVeURVKkEOCMrvLtNR@public.gmane.org; 
> > > kalle.jokiniemi-sMOQStClEysAvxtiuMwx3w@public.gmane.org; m-sonasath-l0cyMroinI0@public.gmane.org; jhnikula-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org; 
> > > nm-l0cyMroinI0@public.gmane.org
> > > Subject: RE: [PATCH] OMAP: I2C: Add mpu wake up latency 
> > constraint in 
> > > i2c
> > > 
> > > Hi Ben, Kalle,
> > > 
> > > On Thu, 26 Nov 2009, kalle.jokiniemi-xNZwKgViW5gAvxtiuMwx3w@public.gmane.org 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.
> 
> 
> Ben is bit unresponsive about my patch. Would you Jean be the current linux-i2c maintainer?

Only for the core and PC drivers. Not platform drivers.

> Could you take my patch in?

Not really. Either wait for Ben to respond, or push the patch through
the OMAP tree. If this really doesn't work, you might try Andrew Morton.

-- 
Jean Delvare

Re: [PATCH] OMAP: I2C: Add mpu wake up latency constraint in i2c

[Tony Lindgren]

* Jean Delvare <khali@linux-fr.org> [091217 08:35]:
> On Thu, 17 Dec 2009 16:18:24 +0100, kalle.jokiniemi@nokia.com wrote:
> > Hi Jean,
> > 
> > > -----Original Message-----
> > > From: Jokiniemi Kalle (Nokia-D/Tampere) 
> > > Sent: 11. joulukuuta 2009 12:47
> > > To: 'ext Paul Walmsley'; ben-linux@fluff.org
> > > Cc: linux-i2c@vger.kernel.org; linux-omap@vger.kernel.org; 
> > > Ujfalusi Peter (Nokia-D/Tampere); Hogander Jouni 
> > > (Nokia-D/Tampere); khilman@deeprootsystems.com; 
> > > kalle.jokiniemi@digia.com; m-sonasath@ti.com; 
> > > jhnikula@gmail.com; nm@ti.com
> > > Subject: RE: [PATCH] OMAP: I2C: Add mpu wake up latency 
> > > constraint in i2c
> > > 
> > > 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@pwsan.com]
> > > > Sent: 30. marraskuuta 2009 9:24
> > > > To: Jokiniemi Kalle (Nokia-D/Tampere)
> > > > Cc: ben-linux@fluff.org; linux-i2c@vger.kernel.org; 
> > > > linux-omap@vger.kernel.org; Ujfalusi Peter 
> > > (Nokia-D/Tampere); Hogander 
> > > > Jouni (Nokia-D/Tampere); khilman@deeprootsystems.com; 
> > > > kalle.jokiniemi@digia.com; m-sonasath@ti.com; jhnikula@gmail.com; 
> > > > nm@ti.com
> > > > Subject: RE: [PATCH] OMAP: I2C: Add mpu wake up latency 
> > > constraint in 
> > > > i2c
> > > > 
> > > > Hi Ben, Kalle,
> > > > 
> > > > On Thu, 26 Nov 2009, kalle.jokiniemi@nokia.com 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.
> > 
> > 
> > Ben is bit unresponsive about my patch. Would you Jean be the current linux-i2c maintainer?
> 
> Only for the core and PC drivers. Not platform drivers.
> 
> > Could you take my patch in?
> 
> Not really. Either wait for Ben to respond, or push the patch through
> the OMAP tree. If this really doesn't work, you might try Andrew Morton.

Ack from me too for Ben.

Acked-by: Tony Lindgren <tony@atomide.com>

Re: [PATCH] OMAP: I2C: Add mpu wake up latency constraint in i2c

[Kalle Jokiniemi]

On Fri, 2009-10-02 at 16:11 +0300, Jokiniemi Kalle wrote:
> 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.
> 
> Added a constraint that allows MPU to wake up in few
> hundred usecs, which is roughly the average i2c wait
> period.
> 
> The constraint function is passed as platform data from
> plat-omap/i2c.c. Currently there is implementation for
> omap3 constraint. Future omap platforms should implement
> their own functions, and pass them instead.

So this is the new version of setting mpu wakeup constraints in i2c
driver. Only build tested.

Kevin, could you run some tests on beagle or some other. I could not get
my zImage to work on RX-51 (tried rx51_defconfig and omap3_pm_defconfig
builds).

Applies on top of pm-branch.

- Kalle

> 
> Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
> ---
>  arch/arm/plat-omap/i2c.c      |   54 +++++++++++++++++++++++++++++++---------
>  drivers/i2c/busses/i2c-omap.c |   19 +++++++++++---
>  include/linux/i2c-omap.h      |    9 +++++++
>  3 files changed, 66 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..4ab7a92 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))
>  
> +/**
> + * omap3_i2c_set_wfc_mpu_wkup_lat - sets mpu wake up constraint
> + * @dev:	i2c bus device pointer
> + * @set:	set or clear constraints
> + *
> + * 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. For OMAP3 platform.
> + */
> +#ifdef CONFIG_ARCH_OMAP34XX
> +static void omap3_i2c_set_wfc_mpu_wkup_lat(struct device *dev, int set)
> +{
> +	omap_pm_set_max_mpu_wakeup_lat(dev, set ? 500 : -1);
> +}
> +#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 = omap3_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 75bf3ad..168623f 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,8 @@ struct omap_i2c_dev {
>  	struct clk		*fclk;		/* Functional clock */
>  	struct completion	cmd_complete;
>  	struct resource		*ioarea;
> +	void			(*set_mpu_wkup_lat)(struct device *dev,
> +						    int set);
>  	u32			speed;		/* Speed of bus in Khz */
>  	u16			cmd_err;
>  	u8			*buf;
> @@ -526,8 +529,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, 1);
>  	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, 0);
>  	dev->buf_len = 0;
>  	if (r < 0)
>  		return r;
> @@ -844,6 +851,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;
> @@ -873,10 +881,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;
> 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

[PATCH] OMAP: I2C: Add mpu wake up latency constraint in i2c

[Kalle Jokiniemi]

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.

Added a constraint that allows MPU to wake up in few
hundred usecs, which is roughly the average i2c wait
period.

The constraint function is passed as platform data from
plat-omap/i2c.c. Currently there is implementation for
omap3 constraint. Future omap platforms should implement
their own functions, and pass them instead.

Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@digia.com>
---
 arch/arm/plat-omap/i2c.c      |   54 +++++++++++++++++++++++++++++++---------
 drivers/i2c/busses/i2c-omap.c |   19 +++++++++++---
 include/linux/i2c-omap.h      |    9 +++++++
 3 files changed, 66 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..4ab7a92 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))
 
+/**
+ * omap3_i2c_set_wfc_mpu_wkup_lat - sets mpu wake up constraint
+ * @dev:	i2c bus device pointer
+ * @set:	set or clear constraints
+ *
+ * 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. For OMAP3 platform.
+ */
+#ifdef CONFIG_ARCH_OMAP34XX
+static void omap3_i2c_set_wfc_mpu_wkup_lat(struct device *dev, int set)
+{
+	omap_pm_set_max_mpu_wakeup_lat(dev, set ? 500 : -1);
+}
+#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 = omap3_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 75bf3ad..168623f 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,8 @@ struct omap_i2c_dev {
 	struct clk		*fclk;		/* Functional clock */
 	struct completion	cmd_complete;
 	struct resource		*ioarea;
+	void			(*set_mpu_wkup_lat)(struct device *dev,
+						    int set);
 	u32			speed;		/* Speed of bus in Khz */
 	u16			cmd_err;
 	u8			*buf;
@@ -526,8 +529,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, 1);
 	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, 0);
 	dev->buf_len = 0;
 	if (r < 0)
 		return r;
@@ -844,6 +851,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;
@@ -873,10 +881,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;
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.5.4.3