Hi Ludovic, Thanks a lot. > -----Original Message----- > From: Ludovic Desroches [mailto:ludovic.desroches@xxxxxxxxx] > Sent: Monday, October 20, 2014 8:39 PM > To: Yang, Wenyou > Cc: wsa@xxxxxxxxxxxxx; Desroches, Ludovic; linux-i2c@xxxxxxxxxxxxxxx; linux- > kernel@xxxxxxxxxxxxxxx; Ferre, Nicolas; linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH 1/3] i2c/at91: add support for runtime PM > > Hi Wenyou, > > On Mon, Oct 20, 2014 at 11:42:12AM +0800, Wenyou Yang wrote: > > Drivers should put the device into low power states proactively > > whenever the device is not in use. Thus implement support for runtime > > PM and use the autosuspend feature to make sure that we can still > > perform well in case we see lots of i2c traffic within short period of time. > > > > Signed-off-by: Wenyou Yang <wenyou.yang@xxxxxxxxx> > > --- > > drivers/i2c/busses/i2c-at91.c | 48 ++++++++++++++++++++++++++++++++--- > ------ > > 1 file changed, 38 insertions(+), 10 deletions(-) > > > > diff --git a/drivers/i2c/busses/i2c-at91.c > > b/drivers/i2c/busses/i2c-at91.c index 917d545..03b9f48 100644 > > --- a/drivers/i2c/busses/i2c-at91.c > > +++ b/drivers/i2c/busses/i2c-at91.c > > @@ -31,10 +31,12 @@ > > #include <linux/platform_device.h> > > #include <linux/slab.h> > > #include <linux/platform_data/dma-atmel.h> > > +#include <linux/pm_runtime.h> > > > > #define DEFAULT_TWI_CLK_HZ 100000 /* max 400 Kbits/s > */ > > #define AT91_I2C_TIMEOUT msecs_to_jiffies(100) /* transfer timeout */ > > #define AT91_I2C_DMA_THRESHOLD 8 /* enable > DMA if transfer size is bigger than this threshold */ > > +#define AUTOSUSPEND_TIMEOUT 2000 > > > > /* AT91 TWI register definitions */ > > #define AT91_TWI_CR 0x0000 /* Control Register */ > > @@ -475,12 +477,16 @@ error: > > static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg > > *msg, int num) { > > struct at91_twi_dev *dev = i2c_get_adapdata(adap); > > - int ret; > > + int ret = 0; > > Not necessary. > > > unsigned int_addr_flag = 0; > > struct i2c_msg *m_start = msg; > > > > dev_dbg(&adap->dev, "at91_xfer: processing %d messages:\n", num); > > > > + ret = pm_runtime_get_sync(dev->dev); > > + if (ret < 0) > > + goto out; > > + > > /* > > * The hardware can handle at most two messages concatenated by a > > * repeated start via it's internal address feature. > > @@ -488,18 +494,21 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct > i2c_msg *msg, int num) > > if (num > 2) { > > dev_err(dev->dev, > > "cannot handle more than two concatenated messages.\n"); > > - return 0; > > + ret = 0; > > + goto out; > > } else if (num == 2) { > > int internal_address = 0; > > int i; > > > > if (msg->flags & I2C_M_RD) { > > dev_err(dev->dev, "first transfer must be write.\n"); > > - return -EINVAL; > > + ret = -EINVAL; > > + goto out; > > } > > if (msg->len > 3) { > > dev_err(dev->dev, "first message size must be <= 3.\n"); > > - return -EINVAL; > > + ret = -EINVAL; > > + goto out; > > } > > > > /* 1st msg is put into the internal address, start with 2nd */ @@ > > -523,7 +532,13 @@ static int at91_twi_xfer(struct i2c_adapter *adap, > > struct i2c_msg *msg, int num) > > > > ret = at91_do_twi_transfer(dev); > > > > - return (ret < 0) ? ret : num; > > + if (ret == 0) > > I don't figure out why you've changed this condition. Right, I will change it. > > > + ret = num; > > +out: > > + pm_runtime_mark_last_busy(dev->dev); > > + pm_runtime_put_autosuspend(dev->dev); > > + > > + return ret; > > } > > > > static u32 at91_twi_func(struct i2c_adapter *adapter) @@ -795,11 > > +810,20 @@ static int at91_twi_probe(struct platform_device *pdev) > > dev->adapter.timeout = AT91_I2C_TIMEOUT; > > dev->adapter.dev.of_node = pdev->dev.of_node; > > > > + pm_runtime_set_autosuspend_delay(dev->dev, > AUTOSUSPEND_TIMEOUT); > > + pm_runtime_use_autosuspend(dev->dev); > > + pm_runtime_set_active(dev->dev); > > + pm_runtime_enable(dev->dev); > > + > > rc = i2c_add_numbered_adapter(&dev->adapter); > > if (rc) { > > dev_err(dev->dev, "Adapter %s registration failed\n", > > dev->adapter.name); > > clk_disable_unprepare(dev->clk); > > + > > + pm_runtime_disable(dev->dev); > > + pm_runtime_set_suspended(dev->dev); > > + > > return rc; > > } > > > > @@ -814,16 +838,19 @@ static int at91_twi_remove(struct platform_device > *pdev) > > i2c_del_adapter(&dev->adapter); > > clk_disable_unprepare(dev->clk); > > > > + pm_runtime_disable(dev->dev); > > + pm_runtime_set_suspended(dev->dev); > > + > > return 0; > > } > > > > #ifdef CONFIG_PM > > - > > +#ifdef CONFIG_PM_RUNTIME > > static int at91_twi_runtime_suspend(struct device *dev) { > > struct at91_twi_dev *twi_dev = dev_get_drvdata(dev); > > > > - clk_disable(twi_dev->clk); > > + clk_disable_unprepare(twi_dev->clk); > > > > return 0; > > } > > @@ -832,12 +859,13 @@ static int at91_twi_runtime_resume(struct device > > *dev) { > > struct at91_twi_dev *twi_dev = dev_get_drvdata(dev); > > > > - return clk_enable(twi_dev->clk); > > + return clk_prepare_enable(twi_dev->clk); > > } > > +#endif > > > > static const struct dev_pm_ops at91_twi_pm = { > > - .runtime_suspend = at91_twi_runtime_suspend, > > - .runtime_resume = at91_twi_runtime_resume, > > + SET_RUNTIME_PM_OPS(at91_twi_runtime_suspend, > > + at91_twi_runtime_resume, NULL) > > }; > > > > #define at91_twi_pm_ops (&at91_twi_pm) > > -- > > 1.7.9.5 > > > > > Regards > > Ludovic Best Regards, Wenyou Yang -- 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
