Adding Kevin in the CC list since he had some comments about the PM
runtime support for the SPI driver.
On Mon, Oct 20, 2014 at 02:39:14PM +0200, Ludovic Desroches wrote:
> 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.
>
> > + 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
--
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
