Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752221AbaJTNOR (ORCPT ); Mon, 20 Oct 2014 09:14:17 -0400 Received: from eusmtp01.atmel.com ([212.144.249.243]:8627 "EHLO eusmtp01.atmel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751721AbaJTNOO (ORCPT ); Mon, 20 Oct 2014 09:14:14 -0400 Date: Mon, 20 Oct 2014 15:14:12 +0200 From: Ludovic Desroches To: Wenyou Yang , , , , , CC: Subject: Re: [PATCH 1/3] i2c/at91: add support for runtime PM Message-ID: <20141020131412.GF7443@ldesroches-Latitude-E6320> Mail-Followup-To: Wenyou Yang , wsa@the-dreams.de, linux-i2c@vger.kernel.org, linux-kernel@vger.kernel.org, nicolas.ferre@atmel.com, linux-arm-kernel@lists.infradead.org, khilman@kernel.org References: <1413776535-10123-1-git-send-email-wenyou.yang@atmel.com> <1413776535-10123-2-git-send-email-wenyou.yang@atmel.com> <20141020123914.GC7443@ldesroches-Latitude-E6320> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline In-Reply-To: <20141020123914.GC7443@ldesroches-Latitude-E6320> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > > --- > > 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 > > #include > > #include > > +#include > > > > #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-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/