Hi, On Wed, Aug 03, 2011 at 11:09:20AM -0700, Kevin Hilman wrote: > Current usage of runtime PM is not quite correct. The actual > idle/unidle of the I2C hardware should not happen until the runtime PM > callbacks are called. Therefore, change omap_i2c_[un]idle() functions > to only be called from the runtime PM callbacks (when usage count > transitions to/from zero.) > > Also, the runtime PM core does usage counting and replaces > functionality currently managed by the dev->idle flag. Remove usage > of dev->idle in favor of using runtime PM, and checking status using > pm_runtime_suspended(). > > Signed-off-by: Kevin Hilman <[email protected]> > --- > drivers/i2c/busses/i2c-omap.c | 58 ++++++++++++++++++++++++++-------------- > 1 files changed, 38 insertions(+), 20 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index 12d0cbc..1b5325b 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -194,7 +194,6 @@ struct omap_i2c_dev { > */ > u8 rev; > unsigned b_hw:1; /* bad h/w fixes */ > - unsigned idle:1; > u16 iestate; /* Saved interrupt register */ > u16 pscstate; > u16 scllstate; > @@ -269,12 +268,8 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev) > { > struct omap_i2c_bus_platform_data *pdata; > > - WARN_ON(!dev->idle); > - > pdata = dev->dev->platform_data; > > - pm_runtime_get_sync(dev->dev); > - > if (pdata->flags & OMAP_I2C_FLAG_RESET_REGS_POSTIDLE) { > omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, 0); > omap_i2c_write_reg(dev, OMAP_I2C_PSC_REG, dev->pscstate); > @@ -285,7 +280,6 @@ static void omap_i2c_unidle(struct omap_i2c_dev *dev) > omap_i2c_write_reg(dev, OMAP_I2C_WE_REG, dev->westate); > omap_i2c_write_reg(dev, OMAP_I2C_CON_REG, OMAP_I2C_CON_EN); > } > - dev->idle = 0; > > /* > * Don't write to this register if the IE state is 0 as it can > @@ -300,8 +294,6 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev) > struct omap_i2c_bus_platform_data *pdata; > u16 iv; > > - WARN_ON(dev->idle); > - > pdata = dev->dev->platform_data; > > dev->iestate = omap_i2c_read_reg(dev, OMAP_I2C_IE_REG); > @@ -315,12 +307,9 @@ static void omap_i2c_idle(struct omap_i2c_dev *dev) > } else { > omap_i2c_write_reg(dev, OMAP_I2C_STAT_REG, dev->iestate); > > - /* Flush posted write before the dev->idle store occurs */ > + /* Flush posted write */ > omap_i2c_read_reg(dev, OMAP_I2C_STAT_REG); > } > - dev->idle = 1; > - > - pm_runtime_put_sync(dev->dev); > } > > static int omap_i2c_init(struct omap_i2c_dev *dev) > @@ -644,7 +633,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg > msgs[], int num) > int i; > int r; > > - omap_i2c_unidle(dev); > + pm_runtime_get_sync(dev->dev); > > r = omap_i2c_wait_for_bb(dev); > if (r < 0) > @@ -667,7 +656,7 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg > msgs[], int num) > > omap_i2c_wait_for_bb(dev); > out: > - omap_i2c_idle(dev); > + pm_runtime_put_sync(dev->dev);
I wonder if these pm_runtime_put need to be synchronous ? Could we just
call pm_runtime_put() instead ? Ditto to all other.
> @@ -1140,6 +1128,36 @@ omap_i2c_remove(struct platform_device *pdev)
> return 0;
> }
>
> +#ifdef CONFIG_PM_RUNTIME
> +static int omap_i2c_runtime_suspend(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct omap_i2c_dev *_dev = platform_get_drvdata(pdev);
what happened to dev_get_drvdata(dev) ??
> + omap_i2c_idle(_dev);
> +
> + return 0;
> +}
> +
> +static int omap_i2c_runtime_resume(struct device *dev)
> +{
> + struct platform_device *pdev = to_platform_device(dev);
> + struct omap_i2c_dev *_dev = platform_get_drvdata(pdev);
ditto
> + omap_i2c_unidle(_dev);
> +
> + return 0;
> +}
> +
> +static struct dev_pm_ops omap_i2c_pm_ops = {
> + .runtime_suspend = omap_i2c_runtime_suspend,
> + .runtime_resume = omap_i2c_runtime_resume,
> +};
> +#define OMAP_I2C_PM_OPS (&omap_i2c_pm_ops)
> +#else
> +#define OMAP_I2C_PM_OPS NULL
> +#endif
OMAP_I2C_PM_OPS isn't used anywhere ??
--
balbi
signature.asc
Description: Digital signature
