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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Ben, Kalle,

On Thu, 26 Nov 2009, kalle.jokiniemi@xxxxxxxxx wrote:

> Could you take this patch to i2c tree? The patch has been discussed in linux-omap list, and this version was the one that was agreed to pushed forward.
> 
> http://marc.info/?l=linux-omap&m=125907598412786&w=2
> 
> 
> I tried it on top of linus' tree and it applied fine. 

Reviewed-by: Paul Walmsley <paul@xxxxxxxxx>

Looks good to me.

- Paul


> 
> - Kalle
> 
> 
> > -----Original Message-----
> > From: linux-omap-owner@xxxxxxxxxxxxxxx 
> > [mailto:linux-omap-owner@xxxxxxxxxxxxxxx] On Behalf Of 
> > Jokiniemi Kalle (Nokia-D/Tampere)
> > Sent: 25. marraskuuta 2009 20:04
> > To: ben-linux@xxxxxxxxx; linux-i2c@xxxxxxxxxxxxxxx
> > Cc: linux-omap@xxxxxxxxxxxxxxx; Ujfalusi Peter 
> > (Nokia-D/Tampere); Hogander Jouni (Nokia-D/Tampere); 
> > khilman@xxxxxxxxxxxxxxxxxxx; ext Kalle Jokiniemi; Moiz 
> > Sonasath; Jarkko Nikula; Paul Walmsley; Nishanth Menon
> > Subject: [PATCH] OMAP: I2C: Add mpu wake up latency constraint in i2c
> > 
> > From: ext Kalle Jokiniemi <kalle.jokiniemi@xxxxxxxxx>
> > 
> > While waiting for completion of the i2c transfer, the MPU 
> > could hit OFF mode and cause several msecs of delay that made 
> > i2c transfers fail more often. The extra delays and 
> > subsequent re-trys cause i2c clocks to be active more often. 
> > This has also an negative effect on power consumption.
> > 
> > Created a mechanism for passing and using the constraint 
> > setting function in driver code. The used mpu wake up latency 
> > constraints are now set individually per bus, and they are 
> > calculated based on clock rate and fifo size.
> > 
> > Thanks to Jarkko Nikula, Moiz Sonasath, Paul Walmsley, and 
> > Nishanth Menon for tuning out the details of this patch.
> > 
> > Cc: Moiz Sonasath <m-sonasath@xxxxxx>
> > Cc: Jarkko Nikula <jhnikula@xxxxxxxxx>
> > Cc: Paul Walmsley <paul@xxxxxxxxx>
> > Cc: Nishanth Menon <nm@xxxxxx>
> > Signed-off-by: Kalle Jokiniemi <kalle.jokiniemi@xxxxxxxxx>
> > ---
> >  arch/arm/plat-omap/i2c.c      |   54 
> > +++++++++++++++++++++++++++++++---------
> >  drivers/i2c/busses/i2c-omap.c |   25 ++++++++++++++++---
> >  include/linux/i2c-omap.h      |    9 +++++++
> >  3 files changed, 72 insertions(+), 16 deletions(-)  create 
> > mode 100644 include/linux/i2c-omap.h
> > 
> > diff --git a/arch/arm/plat-omap/i2c.c 
> > b/arch/arm/plat-omap/i2c.c index 8b84839..3c122cd 100644
> > --- a/arch/arm/plat-omap/i2c.c
> > +++ b/arch/arm/plat-omap/i2c.c
> > @@ -26,8 +26,10 @@
> >  #include <linux/kernel.h>
> >  #include <linux/platform_device.h>
> >  #include <linux/i2c.h>
> > +#include <linux/i2c-omap.h>
> >  #include <mach/irqs.h>
> >  #include <mach/mux.h>
> > +#include <mach/omap-pm.h>
> >  
> >  #define OMAP_I2C_SIZE		0x3f
> >  #define OMAP1_I2C_BASE		0xfffb3800
> > @@ -69,14 +71,14 @@ static struct resource i2c_resources[][2] = {
> >  		},					\
> >  	}
> >  
> > -static u32 i2c_rate[ARRAY_SIZE(i2c_resources)];
> > +static struct omap_i2c_bus_platform_data 
> > +i2c_pdata[ARRAY_SIZE(i2c_resources)];
> >  static struct platform_device omap_i2c_devices[] = {
> > -	I2C_DEV_BUILDER(1, i2c_resources[0], &i2c_rate[0]),
> > +	I2C_DEV_BUILDER(1, i2c_resources[0], &i2c_pdata[0]),
> >  #if	defined(CONFIG_ARCH_OMAP24XX) || defined(CONFIG_ARCH_OMAP34XX)
> > -	I2C_DEV_BUILDER(2, i2c_resources[1], &i2c_rate[1]),
> > +	I2C_DEV_BUILDER(2, i2c_resources[1], &i2c_pdata[1]),
> >  #endif
> >  #if	defined(CONFIG_ARCH_OMAP34XX)
> > -	I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_rate[2]),
> > +	I2C_DEV_BUILDER(3, i2c_resources[2], &i2c_pdata[2]),
> >  #endif
> >  };
> >  
> > @@ -100,6 +102,31 @@ static const int omap34xx_pins[][2] = {};
> >  
> >  #define OMAP_I2C_CMDLINE_SETUP	(BIT(31))
> >  
> > +#ifdef CONFIG_ARCH_OMAP34XX
> > +/*
> > + * omap_i2c_set_wfc_mpu_wkup_lat - sets mpu wake up constraint
> > + * @dev:	i2c bus device pointer
> > + * @val:	latency constraint to set, -1 to disable constraint
> > + *
> > + * When waiting for completion of a i2c transfer, we need to 
> > set a wake 
> > +up
> > + * latency constraint for the MPU. This is to ensure quick enough 
> > +wakeup from
> > + * idle, when transfer completes.
> > + */
> > +static void omap_i2c_set_wfc_mpu_wkup_lat(struct device 
> > *dev, int val) 
> > +{
> > +	omap_pm_set_max_mpu_wakeup_lat(dev, val); } #endif
> > +
> > +static void __init omap_set_i2c_constraint_func(
> > +				struct omap_i2c_bus_platform_data *pd) {
> > +	if (cpu_is_omap34xx())
> > +		pd->set_mpu_wkup_lat = omap_i2c_set_wfc_mpu_wkup_lat;
> > +	else
> > +		pd->set_mpu_wkup_lat = NULL;
> > +}
> > +
> >  static void __init omap_i2c_mux_pins(int bus)  {
> >  	int scl, sda;
> > @@ -180,8 +207,8 @@ static int __init omap_i2c_bus_setup(char *str)
> >  	get_options(str, 3, ints);
> >  	if (ints[0] < 2 || ints[1] < 1 || ints[1] > ports)
> >  		return 0;
> > -	i2c_rate[ints[1] - 1] = ints[2];
> > -	i2c_rate[ints[1] - 1] |= OMAP_I2C_CMDLINE_SETUP;
> > +	i2c_pdata[ints[1] - 1].clkrate = ints[2];
> > +	i2c_pdata[ints[1] - 1].clkrate |= OMAP_I2C_CMDLINE_SETUP;
> >  
> >  	return 1;
> >  }
> > @@ -195,9 +222,10 @@ static int __init 
> > omap_register_i2c_bus_cmdline(void)
> >  {
> >  	int i, err = 0;
> >  
> > -	for (i = 0; i < ARRAY_SIZE(i2c_rate); i++)
> > -		if (i2c_rate[i] & OMAP_I2C_CMDLINE_SETUP) {
> > -			i2c_rate[i] &= ~OMAP_I2C_CMDLINE_SETUP;
> > +	for (i = 0; i < ARRAY_SIZE(i2c_pdata); i++)
> > +		if (i2c_pdata[i].clkrate & OMAP_I2C_CMDLINE_SETUP) {
> > +			i2c_pdata[i].clkrate &= ~OMAP_I2C_CMDLINE_SETUP;
> > +			omap_set_i2c_constraint_func(&i2c_pdata[i]);
> >  			err = omap_i2c_add_bus(i + 1);
> >  			if (err)
> >  				goto out;
> > @@ -231,9 +259,11 @@ int __init omap_register_i2c_bus(int 
> > bus_id, u32 clkrate,
> >  			return err;
> >  	}
> >  
> > -	if (!i2c_rate[bus_id - 1])
> > -		i2c_rate[bus_id - 1] = clkrate;
> > -	i2c_rate[bus_id - 1] &= ~OMAP_I2C_CMDLINE_SETUP;
> > +	if (!i2c_pdata[bus_id - 1].clkrate)
> > +		i2c_pdata[bus_id - 1].clkrate = clkrate;
> > +
> > +	omap_set_i2c_constraint_func(&i2c_pdata[bus_id - 1]);
> > +	i2c_pdata[bus_id - 1].clkrate &= ~OMAP_I2C_CMDLINE_SETUP;
> >  
> >  	return omap_i2c_add_bus(bus_id);
> >  }
> > diff --git a/drivers/i2c/busses/i2c-omap.c 
> > b/drivers/i2c/busses/i2c-omap.c index 827da08..ac0bfd2 100644
> > --- a/drivers/i2c/busses/i2c-omap.c
> > +++ b/drivers/i2c/busses/i2c-omap.c
> > @@ -37,6 +37,7 @@
> >  #include <linux/platform_device.h>
> >  #include <linux/clk.h>
> >  #include <linux/io.h>
> > +#include <linux/i2c-omap.h>
> >  
> >  /* I2C controller revisions */
> >  #define OMAP_I2C_REV_2			0x20
> > @@ -165,6 +166,9 @@ struct omap_i2c_dev {
> >  	struct clk		*fclk;		/* Functional clock */
> >  	struct completion	cmd_complete;
> >  	struct resource		*ioarea;
> > +	u32			latency;	/* maximum mpu 
> > wkup latency */
> > +	void			(*set_mpu_wkup_lat)(struct device *dev,
> > +						    int latency);
> >  	u32			speed;		/* Speed of bus 
> > in Khz */
> >  	u16			cmd_err;
> >  	u8			*buf;
> > @@ -508,8 +512,12 @@ static int omap_i2c_xfer_msg(struct 
> > i2c_adapter *adap,
> >  	 * REVISIT: We should abort the transfer on signals, 
> > but the bus goes
> >  	 * into arbitration and we're currently unable to 
> > recover from it.
> >  	 */
> > +	if (dev->set_mpu_wkup_lat != NULL)
> > +		dev->set_mpu_wkup_lat(dev->dev, dev->latency);
> >  	r = wait_for_completion_timeout(&dev->cmd_complete,
> >  					OMAP_I2C_TIMEOUT);
> > +	if (dev->set_mpu_wkup_lat != NULL)
> > +		dev->set_mpu_wkup_lat(dev->dev, -1);
> >  	dev->buf_len = 0;
> >  	if (r < 0)
> >  		return r;
> > @@ -826,6 +834,7 @@ omap_i2c_probe(struct platform_device *pdev)
> >  	struct omap_i2c_dev	*dev;
> >  	struct i2c_adapter	*adap;
> >  	struct resource		*mem, *irq, *ioarea;
> > +	struct omap_i2c_bus_platform_data *pdata = 
> > pdev->dev.platform_data;
> >  	irq_handler_t isr;
> >  	int r;
> >  	u32 speed = 0;
> > @@ -855,10 +864,13 @@ omap_i2c_probe(struct platform_device *pdev)
> >  		goto err_release_region;
> >  	}
> >  
> > -	if (pdev->dev.platform_data != NULL)
> > -		speed = *(u32 *)pdev->dev.platform_data;
> > -	else
> > -		speed = 100;	/* Defualt speed */
> > +	if (pdata != NULL) {
> > +		speed = pdata->clkrate;
> > +		dev->set_mpu_wkup_lat = pdata->set_mpu_wkup_lat;
> > +	} else {
> > +		speed = 100;	/* Default speed */
> > +		dev->set_mpu_wkup_lat = NULL;
> > +	}
> >  
> >  	dev->speed = speed;
> >  	dev->idle = 1;
> > @@ -893,6 +905,11 @@ omap_i2c_probe(struct platform_device *pdev)
> >  		 */
> >  		dev->fifo_size = (dev->fifo_size / 2);
> >  		dev->b_hw = 1; /* Enable hardware fixes */
> > +
> > +		/* calculate wakeup latency constraint for MPU */
> > +		if (dev->set_mpu_wkup_lat != NULL)
> > +			dev->latency = (1000000 * dev->fifo_size) /
> > +				       (1000 * speed / 8);
> >  	}
> >  
> >  	/* reset ASAP, clearing any IRQs */
> > diff --git a/include/linux/i2c-omap.h 
> > b/include/linux/i2c-omap.h new file mode 100644 index 0000000..1362fba
> > --- /dev/null
> > +++ b/include/linux/i2c-omap.h
> > @@ -0,0 +1,9 @@
> > +#ifndef __I2C_OMAP_H__
> > +#define __I2C_OMAP_H__
> > +
> > +struct omap_i2c_bus_platform_data {
> > +	u32		clkrate;
> > +	void		(*set_mpu_wkup_lat)(struct device *dev, 
> > int set);
> > +};
> > +
> > +#endif
> > --
> > 1.6.3.3
> > 
> > --
> > To unsubscribe from this list: send the line "unsubscribe 
> > linux-omap" in the body of a message to 
> > majordomo@xxxxxxxxxxxxxxx More majordomo info at  
> > http://vger.kernel.org/majordomo-info.html
> > 


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

[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux