[PATCH v2] i2c/designware: enable i2c controller to suspend/resume asynchronously

[PATCH v2] i2c/designware: enable i2c controller to suspend/resume asynchronously

[Fu, Zhonghui]

Now, PM core supports asynchronous suspend/resume mode for devices
during system suspend/resume, and the power state transition of one
device may be completed in separate kernel thread. PM core ensures
all power state transition timing dependency between devices. This
patch enables i2c controller to suspend/resume asynchronously. This
will take advantage of multicore and improve system suspend/resume
speed.

Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
---
Changes in v2:
- Amend commit message.

 drivers/i2c/busses/i2c-designware-platdrv.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 3dd2de3..fec0fce 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -245,6 +245,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
 
 	adap = &dev->adapter;
 	i2c_set_adapdata(adap, dev);
+	device_enable_async_suspend(&pdev->dev);
 	adap->owner = THIS_MODULE;
 	adap->class = I2C_CLASS_DEPRECATED;
 	strlcpy(adap->name, "Synopsys DesignWare I2C adapter",
-- 1.7.1

Re: [PATCH v2] i2c/designware: enable i2c controller to suspend/resume asynchronously

[Fu, Zhonghui]

Any comments are welcome.


Thanks,
Zhonghui



On 9/24/2015 3:10 PM, Fu, Zhonghui wrote:
> Now, PM core supports asynchronous suspend/resume mode for devices
> during system suspend/resume, and the power state transition of one
> device may be completed in separate kernel thread. PM core ensures
> all power state transition timing dependency between devices. This
> patch enables i2c controller to suspend/resume asynchronously. This
> will take advantage of multicore and improve system suspend/resume
> speed.
>
> Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> ---
> Changes in v2:
> - Amend commit message.
>
>  drivers/i2c/busses/i2c-designware-platdrv.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 3dd2de3..fec0fce 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -245,6 +245,7 @@ static int dw_i2c_probe(struct platform_device *pdev)
>  
>  	adap = &dev->adapter;
>  	i2c_set_adapdata(adap, dev);
> +	device_enable_async_suspend(&pdev->dev);
>  	adap->owner = THIS_MODULE;
>  	adap->class = I2C_CLASS_DEPRECATED;
>  	strlcpy(adap->name, "Synopsys DesignWare I2C adapter",
> -- 1.7.1
>

Re: [PATCH v2] i2c/designware: enable i2c controller to suspend/resume asynchronously

[Fu, Zhonghui]

On 2/1/2016 11:54 PM, Wolfram Sang wrote:
>>>> device may be completed in separate kernel thread. PM core ensures
>>>> all power state transition dependency between devices. This patch
> I'd like an Ack on that from a PM maintainer, because I think chips like
> PMICs are special and might not be covered by the generic case...
>
>> Besides that we have to be really aware about DMA power related fix
>> introduced in v4.5-rc1 in acpi_lpss.c [1]. So, I would like to see a
>> wide testing especially on Intel Baytrail / Braswell platforms before
>> enabling it.
> And this one, too.
Because of long leave, so sorry for very late reply.

I agree with you. Need more investigation and test for this patch before enabling it.


Thanks,
Zhonghui
>

Re: [PATCH v2] i2c/designware: enable i2c controller to suspend/resume asynchronously

[Wolfram Sang]

[-- Attachment #1: Type: text/plain, Size: 536 bytes --]


> > > device may be completed in separate kernel thread. PM core ensures
> > > all power state transition dependency between devices. This patch

I'd like an Ack on that from a PM maintainer, because I think chips like
PMICs are special and might not be covered by the generic case...

> Besides that we have to be really aware about DMA power related fix
> introduced in v4.5-rc1 in acpi_lpss.c [1]. So, I would like to see a
> wide testing especially on Intel Baytrail / Braswell platforms before
> enabling it.

And this one, too.


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH v2] i2c/designware: enable i2c controller to suspend/resume asynchronously

[Andy Shevchenko]

On Mon, 2016-02-01 at 17:04 +0200, Jarkko Nikula wrote:
> On 01/15/2016 08:38 AM, Fu, Zhonghui wrote:
> > Now, PM core supports asynchronous suspend/resume mode for devices
> > during system suspend/resume, and the power state transition of one
> > device may be completed in separate kernel thread. PM core ensures
> > all power state transition dependency between devices. This patch
> > enables designware i2c controllers to suspend/resume
> > asynchronously.
> > This will take advantage of multicore and improve system
> > suspend/resume
> > speed. After enabling all i2c devices, i2c adapters and i2c
> > controllers
> > on ASUS T100TA tablet, the system suspend-to-idle time is reduced
> > to
> > about 510ms from 750ms, and the system resume time is reduced to
> > about
> > 790ms from 900ms.
> > 
> > Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> > ---
> > Changes in v2:
> > - Move the device_enable_async_suspend() call into i2c_dw_proble()
> > 
> >   drivers/i2c/busses/i2c-designware-core.c |    1 +
> >   1 files changed, 1 insertions(+), 0 deletions(-)
> > 
> > diff --git a/drivers/i2c/busses/i2c-designware-core.c
> > b/drivers/i2c/busses/i2c-designware-core.c
> > index ba9732c..5d6ad27 100644
> > --- a/drivers/i2c/busses/i2c-designware-core.c
> > +++ b/drivers/i2c/busses/i2c-designware-core.c
> > @@ -861,6 +861,7 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
> > 
> >   	init_completion(&dev->cmd_complete);
> >   	mutex_init(&dev->lock);
> > +	device_enable_async_suspend(dev->dev);
> > 
> I'm aware Andy had concerns about about stability and you also
> mention 
> in another thread:
> 
> "if enable all LPSS devices suspend/resume asynchronously, the
> system 
> can't resume sometimes on ASUS T100TA(BayTrail-T SoC). But, I have 
> verified that the system can resume normally every time if enable
> only 
> i2c controller async mode and let other LPSS devices in sync mode on 
> ASUS T100TA.".
> 
> What I'm thinking would this change move things forward and help to
> find 
> out what potential underlying issues there are in LPSS code.
> 
> I'm fine with this I2C side change but can we assure we don't 
> accidentally enable the same on other LPSS devices before finding
> the 
> root cause for the crash?

Besides that we have to be really aware about DMA power related fix
introduced in v4.5-rc1 in acpi_lpss.c [1]. So, I would like to see a
wide testing especially on Intel Baytrail / Braswell platforms before
enabling it.

[1] http://comments.gmane.org/gmane.linux.acpi.devel/80263

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

Re: [PATCH v2] i2c/designware: enable i2c controller to suspend/resume asynchronously

[Jarkko Nikula]

On 01/15/2016 08:38 AM, Fu, Zhonghui wrote:
> Now, PM core supports asynchronous suspend/resume mode for devices
> during system suspend/resume, and the power state transition of one
> device may be completed in separate kernel thread. PM core ensures
> all power state transition dependency between devices. This patch
> enables designware i2c controllers to suspend/resume asynchronously.
> This will take advantage of multicore and improve system suspend/resume
> speed. After enabling all i2c devices, i2c adapters and i2c controllers
> on ASUS T100TA tablet, the system suspend-to-idle time is reduced to
> about 510ms from 750ms, and the system resume time is reduced to about
> 790ms from 900ms.
>
> Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
> ---
> Changes in v2:
> - Move the device_enable_async_suspend() call into i2c_dw_proble()
>
>   drivers/i2c/busses/i2c-designware-core.c |    1 +
>   1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
> index ba9732c..5d6ad27 100644
> --- a/drivers/i2c/busses/i2c-designware-core.c
> +++ b/drivers/i2c/busses/i2c-designware-core.c
> @@ -861,6 +861,7 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
>
>   	init_completion(&dev->cmd_complete);
>   	mutex_init(&dev->lock);
> +	device_enable_async_suspend(dev->dev);
>
I'm aware Andy had concerns about about stability and you also mention 
in another thread:

"if enable all LPSS devices suspend/resume asynchronously, the system 
can't resume sometimes on ASUS T100TA(BayTrail-T SoC). But, I have 
verified that the system can resume normally every time if enable only 
i2c controller async mode and let other LPSS devices in sync mode on 
ASUS T100TA.".

What I'm thinking would this change move things forward and help to find 
out what potential underlying issues there are in LPSS code.

I'm fine with this I2C side change but can we assure we don't 
accidentally enable the same on other LPSS devices before finding the 
root cause for the crash?

-- 
Jarkko

[PATCH v2] i2c/designware: enable i2c controller to suspend/resume asynchronously

[Fu, Zhonghui]

Now, PM core supports asynchronous suspend/resume mode for devices
during system suspend/resume, and the power state transition of one
device may be completed in separate kernel thread. PM core ensures
all power state transition dependency between devices. This patch
enables designware i2c controllers to suspend/resume asynchronously.
This will take advantage of multicore and improve system suspend/resume
speed. After enabling all i2c devices, i2c adapters and i2c controllers
on ASUS T100TA tablet, the system suspend-to-idle time is reduced to
about 510ms from 750ms, and the system resume time is reduced to about
790ms from 900ms.

Signed-off-by: Zhonghui Fu <zhonghui.fu@linux.intel.com>
---
Changes in v2:
- Move the device_enable_async_suspend() call into i2c_dw_proble()

 drivers/i2c/busses/i2c-designware-core.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/i2c/busses/i2c-designware-core.c b/drivers/i2c/busses/i2c-designware-core.c
index ba9732c..5d6ad27 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -861,6 +861,7 @@ int i2c_dw_probe(struct dw_i2c_dev *dev)
 
 	init_completion(&dev->cmd_complete);
 	mutex_init(&dev->lock);
+	device_enable_async_suspend(dev->dev);
 
 	r = i2c_dw_init(dev);
 	if (r)
-- 1.7.1