Ref: https://lkml.org/lkml/2017/9/19/649
The bus controllers should suspend the bus operations only after
all of the devices on the bus have suspended their device
completely. Since the i2c_client drivers could be talking to
their devices in their suspend_late() calls, lets ensure that the
bus is alive by that time. Thus moving the controller suspend logic to
suspend_late().
Signed-off-by: Rajat Jain <[email protected]>
---
drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
index 0e65b97842b4..66dd7f844c40 100644
--- a/drivers/i2c/busses/i2c-designware-platdrv.c
+++ b/drivers/i2c/busses/i2c-designware-platdrv.c
@@ -468,7 +468,7 @@ static int dw_i2c_plat_suspend(struct device *dev)
static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
.prepare = dw_i2c_plat_prepare,
.complete = dw_i2c_plat_complete,
- SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
+ SET_LATE_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
SET_RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend,
dw_i2c_plat_resume,
NULL)
--
2.14.1.821.g8fa685d3b7-goog
Ref: https://lkml.org/lkml/2017/9/19/649
The intel-lpss hosts the designware i2c controller device, which
needs to be up and running until all its i2c child devices have
suspended (child devices need to be accessible over i2c until
suspend_late() has been called for all those devices).
So lets delay the resetting of the controller until suspend_late().
I thought of renaming function to say *_late / *_early but that
might cause confusion since the same function is also used for runtime
suspend/resume.
Fixes: 0b471aaa0e1a ("Put I2C and SPI controllers into reset state on suspend")
Signed-off-by: Rajat Jain <[email protected]>
---
drivers/mfd/intel-lpss.h | 7 +------
1 file changed, 1 insertion(+), 6 deletions(-)
diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
index 694116630ffa..865bbeaaf00c 100644
--- a/drivers/mfd/intel-lpss.h
+++ b/drivers/mfd/intel-lpss.h
@@ -38,12 +38,7 @@ int intel_lpss_resume(struct device *dev);
#ifdef CONFIG_PM_SLEEP
#define INTEL_LPSS_SLEEP_PM_OPS \
.prepare = intel_lpss_prepare, \
- .suspend = intel_lpss_suspend, \
- .resume = intel_lpss_resume, \
- .freeze = intel_lpss_suspend, \
- .thaw = intel_lpss_resume, \
- .poweroff = intel_lpss_suspend, \
- .restore = intel_lpss_resume,
+ SET_LATE_SYSTEM_SLEEP_PM_OPS(intel_lpss_suspend, intel_lpss_resume)
#else
#define INTEL_LPSS_SLEEP_PM_OPS
#endif
--
2.14.1.821.g8fa685d3b7-goog
On Wed, Sep 20, 2017 at 3:31 PM, Rajat Jain <[email protected]> wrote:
> Ref: https://lkml.org/lkml/2017/9/19/649
>
> The bus controllers should suspend the bus operations only after
> all of the devices on the bus have suspended their device
> completely. Since the i2c_client drivers could be talking to
> their devices in their suspend_late() calls, lets ensure that the
> bus is alive by that time. Thus moving the controller suspend logic to
> suspend_late().
>
> Signed-off-by: Rajat Jain <[email protected]>
> ---
Forgot to mention, this was needed because we were seeing problems
when the ACPI routines (called at resume_early() time) for the i2c
client device were trying to access one of the I2C devices, and were
failing because the bus was not ready.
Thanks,
Rajat
> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 0e65b97842b4..66dd7f844c40 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -468,7 +468,7 @@ static int dw_i2c_plat_suspend(struct device *dev)
> static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
> .prepare = dw_i2c_plat_prepare,
> .complete = dw_i2c_plat_complete,
> - SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
> + SET_LATE_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
> SET_RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend,
> dw_i2c_plat_resume,
> NULL)
> --
> 2.14.1.821.g8fa685d3b7-goog
>
On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <[email protected]> wrote:
> Ref: https://lkml.org/lkml/2017/9/19/649
>
> The bus controllers should suspend the bus operations only after
> all of the devices on the bus have suspended their device
> completely. Since the i2c_client drivers could be talking to
> their devices in their suspend_late() calls, lets ensure that the
> bus is alive by that time. Thus moving the controller suspend logic to
> suspend_late().
>
> Signed-off-by: Rajat Jain <[email protected]>
> ---
> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> index 0e65b97842b4..66dd7f844c40 100644
> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> @@ -468,7 +468,7 @@ static int dw_i2c_plat_suspend(struct device *dev)
> static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
> .prepare = dw_i2c_plat_prepare,
> .complete = dw_i2c_plat_complete,
> - SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
> + SET_LATE_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
> SET_RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend,
> dw_i2c_plat_resume,
> NULL)
No, you can't just do that.
I sent patches to do it properly before my trip to LA last week, it
shouldn't be overly difficult to find them in the mailing list
archives. I can look them up tomorrow if need be.
Thanks,
Rafael
On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <[email protected]> wrote:
> Ref: https://lkml.org/lkml/2017/9/19/649
>
> The intel-lpss hosts the designware i2c controller device, which
> needs to be up and running until all its i2c child devices have
> suspended (child devices need to be accessible over i2c until
> suspend_late() has been called for all those devices).
>
> So lets delay the resetting of the controller until suspend_late().
> I thought of renaming function to say *_late / *_early but that
> might cause confusion since the same function is also used for runtime
> suspend/resume.
>
> Fixes: 0b471aaa0e1a ("Put I2C and SPI controllers into reset state on suspend")
> Signed-off-by: Rajat Jain <[email protected]>
> ---
> drivers/mfd/intel-lpss.h | 7 +------
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
> index 694116630ffa..865bbeaaf00c 100644
> --- a/drivers/mfd/intel-lpss.h
> +++ b/drivers/mfd/intel-lpss.h
> @@ -38,12 +38,7 @@ int intel_lpss_resume(struct device *dev);
> #ifdef CONFIG_PM_SLEEP
> #define INTEL_LPSS_SLEEP_PM_OPS \
> .prepare = intel_lpss_prepare, \
> - .suspend = intel_lpss_suspend, \
> - .resume = intel_lpss_resume, \
> - .freeze = intel_lpss_suspend, \
> - .thaw = intel_lpss_resume, \
> - .poweroff = intel_lpss_suspend, \
> - .restore = intel_lpss_resume,
> + SET_LATE_SYSTEM_SLEEP_PM_OPS(intel_lpss_suspend, intel_lpss_resume)
> #else
> #define INTEL_LPSS_SLEEP_PM_OPS
> #endif
So I sent this exact patch several days ago:
https://patchwork.kernel.org/patch/9939809/
Thanks,
Rafael
On Wed, Sep 20, 2017 at 5:24 PM, Rafael J. Wysocki <[email protected]> wrote:
> On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <[email protected]> wrote:
>> Ref: https://lkml.org/lkml/2017/9/19/649
>>
>> The bus controllers should suspend the bus operations only after
>> all of the devices on the bus have suspended their device
>> completely. Since the i2c_client drivers could be talking to
>> their devices in their suspend_late() calls, lets ensure that the
>> bus is alive by that time. Thus moving the controller suspend logic to
>> suspend_late().
>>
>> Signed-off-by: Rajat Jain <[email protected]>
>> ---
>> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
>> index 0e65b97842b4..66dd7f844c40 100644
>> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
>> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
>> @@ -468,7 +468,7 @@ static int dw_i2c_plat_suspend(struct device *dev)
>> static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
>> .prepare = dw_i2c_plat_prepare,
>> .complete = dw_i2c_plat_complete,
>> - SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
>> + SET_LATE_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
>> SET_RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend,
>> dw_i2c_plat_resume,
>> NULL)
>
> No, you can't just do that.
>
> I sent patches to do it properly before my trip to LA last week, it
> shouldn't be overly difficult to find them in the mailing list
> archives. I can look them up tomorrow if need be.
Thanks, I am guessing you mean this?
https://patchwork.kernel.org/patch/9939807/
>
> Thanks,
> Rafael
On Wed, Sep 20, 2017 at 5:27 PM, Rafael J. Wysocki <[email protected]> wrote:
> On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <[email protected]> wrote:
>> Ref: https://lkml.org/lkml/2017/9/19/649
>>
>> The intel-lpss hosts the designware i2c controller device, which
>> needs to be up and running until all its i2c child devices have
>> suspended (child devices need to be accessible over i2c until
>> suspend_late() has been called for all those devices).
>>
>> So lets delay the resetting of the controller until suspend_late().
>> I thought of renaming function to say *_late / *_early but that
>> might cause confusion since the same function is also used for runtime
>> suspend/resume.
>>
>> Fixes: 0b471aaa0e1a ("Put I2C and SPI controllers into reset state on suspend")
>> Signed-off-by: Rajat Jain <[email protected]>
>> ---
>> drivers/mfd/intel-lpss.h | 7 +------
>> 1 file changed, 1 insertion(+), 6 deletions(-)
>>
>> diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
>> index 694116630ffa..865bbeaaf00c 100644
>> --- a/drivers/mfd/intel-lpss.h
>> +++ b/drivers/mfd/intel-lpss.h
>> @@ -38,12 +38,7 @@ int intel_lpss_resume(struct device *dev);
>> #ifdef CONFIG_PM_SLEEP
>> #define INTEL_LPSS_SLEEP_PM_OPS \
>> .prepare = intel_lpss_prepare, \
>> - .suspend = intel_lpss_suspend, \
>> - .resume = intel_lpss_resume, \
>> - .freeze = intel_lpss_suspend, \
>> - .thaw = intel_lpss_resume, \
>> - .poweroff = intel_lpss_suspend, \
>> - .restore = intel_lpss_resume,
>> + SET_LATE_SYSTEM_SLEEP_PM_OPS(intel_lpss_suspend, intel_lpss_resume)
>> #else
>> #define INTEL_LPSS_SLEEP_PM_OPS
>> #endif
>
> So I sent this exact patch several days ago:
> https://patchwork.kernel.org/patch/9939809/
Sorry, I missed it. Please consider my patch withdrawn / self-nack'ed
in favor of Rafael's patch.
>
> Thanks,
> Rafael
On Thu, 21 Sep 2017, Rafael J. Wysocki wrote:
> On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <[email protected]> wrote:
> > Ref: https://lkml.org/lkml/2017/9/19/649
> >
> > The intel-lpss hosts the designware i2c controller device, which
> > needs to be up and running until all its i2c child devices have
> > suspended (child devices need to be accessible over i2c until
> > suspend_late() has been called for all those devices).
> >
> > So lets delay the resetting of the controller until suspend_late().
> > I thought of renaming function to say *_late / *_early but that
> > might cause confusion since the same function is also used for runtime
> > suspend/resume.
> >
> > Fixes: 0b471aaa0e1a ("Put I2C and SPI controllers into reset state on suspend")
> > Signed-off-by: Rajat Jain <[email protected]>
> > ---
> > drivers/mfd/intel-lpss.h | 7 +------
> > 1 file changed, 1 insertion(+), 6 deletions(-)
> >
> > diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
> > index 694116630ffa..865bbeaaf00c 100644
> > --- a/drivers/mfd/intel-lpss.h
> > +++ b/drivers/mfd/intel-lpss.h
> > @@ -38,12 +38,7 @@ int intel_lpss_resume(struct device *dev);
> > #ifdef CONFIG_PM_SLEEP
> > #define INTEL_LPSS_SLEEP_PM_OPS \
> > .prepare = intel_lpss_prepare, \
> > - .suspend = intel_lpss_suspend, \
> > - .resume = intel_lpss_resume, \
> > - .freeze = intel_lpss_suspend, \
> > - .thaw = intel_lpss_resume, \
> > - .poweroff = intel_lpss_suspend, \
> > - .restore = intel_lpss_resume,
> > + SET_LATE_SYSTEM_SLEEP_PM_OPS(intel_lpss_suspend, intel_lpss_resume)
> > #else
> > #define INTEL_LPSS_SLEEP_PM_OPS
> > #endif
>
> So I sent this exact patch several days ago:
> https://patchwork.kernel.org/patch/9939809/
You did? Any reason you didn't send it to me?
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Thursday, September 21, 2017 10:41:33 AM CEST Lee Jones wrote:
> On Thu, 21 Sep 2017, Rafael J. Wysocki wrote:
>
> > On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <[email protected]> wrote:
> > > Ref: https://lkml.org/lkml/2017/9/19/649
> > >
> > > The intel-lpss hosts the designware i2c controller device, which
> > > needs to be up and running until all its i2c child devices have
> > > suspended (child devices need to be accessible over i2c until
> > > suspend_late() has been called for all those devices).
> > >
> > > So lets delay the resetting of the controller until suspend_late().
> > > I thought of renaming function to say *_late / *_early but that
> > > might cause confusion since the same function is also used for runtime
> > > suspend/resume.
> > >
> > > Fixes: 0b471aaa0e1a ("Put I2C and SPI controllers into reset state on suspend")
> > > Signed-off-by: Rajat Jain <[email protected]>
> > > ---
> > > drivers/mfd/intel-lpss.h | 7 +------
> > > 1 file changed, 1 insertion(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
> > > index 694116630ffa..865bbeaaf00c 100644
> > > --- a/drivers/mfd/intel-lpss.h
> > > +++ b/drivers/mfd/intel-lpss.h
> > > @@ -38,12 +38,7 @@ int intel_lpss_resume(struct device *dev);
> > > #ifdef CONFIG_PM_SLEEP
> > > #define INTEL_LPSS_SLEEP_PM_OPS \
> > > .prepare = intel_lpss_prepare, \
> > > - .suspend = intel_lpss_suspend, \
> > > - .resume = intel_lpss_resume, \
> > > - .freeze = intel_lpss_suspend, \
> > > - .thaw = intel_lpss_resume, \
> > > - .poweroff = intel_lpss_suspend, \
> > > - .restore = intel_lpss_resume,
> > > + SET_LATE_SYSTEM_SLEEP_PM_OPS(intel_lpss_suspend, intel_lpss_resume)
> > > #else
> > > #define INTEL_LPSS_SLEEP_PM_OPS
> > > #endif
> >
> > So I sent this exact patch several days ago:
> > https://patchwork.kernel.org/patch/9939809/
>
> You did?
Yes, I did.
> Any reason you didn't send it to me?
Well, Lee Jones <[email protected]> is very much in the CC list of it.
Was I expected to use a different e-mail address for that?
Thanks,
Rafael
On Thursday, September 21, 2017 3:13:56 AM CEST Rajat Jain wrote:
> On Wed, Sep 20, 2017 at 5:24 PM, Rafael J. Wysocki <[email protected]> wrote:
> > On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <[email protected]> wrote:
> >> Ref: https://lkml.org/lkml/2017/9/19/649
> >>
> >> The bus controllers should suspend the bus operations only after
> >> all of the devices on the bus have suspended their device
> >> completely. Since the i2c_client drivers could be talking to
> >> their devices in their suspend_late() calls, lets ensure that the
> >> bus is alive by that time. Thus moving the controller suspend logic to
> >> suspend_late().
> >>
> >> Signed-off-by: Rajat Jain <[email protected]>
> >> ---
> >> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
> >> index 0e65b97842b4..66dd7f844c40 100644
> >> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
> >> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
> >> @@ -468,7 +468,7 @@ static int dw_i2c_plat_suspend(struct device *dev)
> >> static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
> >> .prepare = dw_i2c_plat_prepare,
> >> .complete = dw_i2c_plat_complete,
> >> - SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
> >> + SET_LATE_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
> >> SET_RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend,
> >> dw_i2c_plat_resume,
> >> NULL)
> >
> > No, you can't just do that.
> >
> > I sent patches to do it properly before my trip to LA last week, it
> > shouldn't be overly difficult to find them in the mailing list
> > archives. I can look them up tomorrow if need be.
>
> Thanks, I am guessing you mean this?
>
> https://patchwork.kernel.org/patch/9939807/
Yes, that's what I mean.
Thanks,
Rafael
On Thu, 21 Sep 2017, Rafael J. Wysocki wrote:
> On Thursday, September 21, 2017 10:41:33 AM CEST Lee Jones wrote:
> > On Thu, 21 Sep 2017, Rafael J. Wysocki wrote:
> >
> > > On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <[email protected]> wrote:
> > > > Ref: https://lkml.org/lkml/2017/9/19/649
> > > >
> > > > The intel-lpss hosts the designware i2c controller device, which
> > > > needs to be up and running until all its i2c child devices have
> > > > suspended (child devices need to be accessible over i2c until
> > > > suspend_late() has been called for all those devices).
> > > >
> > > > So lets delay the resetting of the controller until suspend_late().
> > > > I thought of renaming function to say *_late / *_early but that
> > > > might cause confusion since the same function is also used for runtime
> > > > suspend/resume.
> > > >
> > > > Fixes: 0b471aaa0e1a ("Put I2C and SPI controllers into reset state on suspend")
> > > > Signed-off-by: Rajat Jain <[email protected]>
> > > > ---
> > > > drivers/mfd/intel-lpss.h | 7 +------
> > > > 1 file changed, 1 insertion(+), 6 deletions(-)
> > > >
> > > > diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
> > > > index 694116630ffa..865bbeaaf00c 100644
> > > > --- a/drivers/mfd/intel-lpss.h
> > > > +++ b/drivers/mfd/intel-lpss.h
> > > > @@ -38,12 +38,7 @@ int intel_lpss_resume(struct device *dev);
> > > > #ifdef CONFIG_PM_SLEEP
> > > > #define INTEL_LPSS_SLEEP_PM_OPS \
> > > > .prepare = intel_lpss_prepare, \
> > > > - .suspend = intel_lpss_suspend, \
> > > > - .resume = intel_lpss_resume, \
> > > > - .freeze = intel_lpss_suspend, \
> > > > - .thaw = intel_lpss_resume, \
> > > > - .poweroff = intel_lpss_suspend, \
> > > > - .restore = intel_lpss_resume,
> > > > + SET_LATE_SYSTEM_SLEEP_PM_OPS(intel_lpss_suspend, intel_lpss_resume)
> > > > #else
> > > > #define INTEL_LPSS_SLEEP_PM_OPS
> > > > #endif
> > >
> > > So I sent this exact patch several days ago:
> > > https://patchwork.kernel.org/patch/9939809/
> >
> > You did?
>
> Yes, I did.
>
> > Any reason you didn't send it to me?
>
> Well, Lee Jones <[email protected]> is very much in the CC list of it.
>
> Was I expected to use a different e-mail address for that?
I don't monitor that one. Better to use the one in MAINTAINERS.
--
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
On Thu, Sep 21, 2017 at 5:17 PM, Lee Jones <[email protected]> wrote:
> On Thu, 21 Sep 2017, Rafael J. Wysocki wrote:
>
>> On Thursday, September 21, 2017 10:41:33 AM CEST Lee Jones wrote:
>> > On Thu, 21 Sep 2017, Rafael J. Wysocki wrote:
>> >
>> > > On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <[email protected]> wrote:
>> > > > Ref: https://lkml.org/lkml/2017/9/19/649
>> > > >
>> > > > The intel-lpss hosts the designware i2c controller device, which
>> > > > needs to be up and running until all its i2c child devices have
>> > > > suspended (child devices need to be accessible over i2c until
>> > > > suspend_late() has been called for all those devices).
>> > > >
>> > > > So lets delay the resetting of the controller until suspend_late().
>> > > > I thought of renaming function to say *_late / *_early but that
>> > > > might cause confusion since the same function is also used for runtime
>> > > > suspend/resume.
>> > > >
>> > > > Fixes: 0b471aaa0e1a ("Put I2C and SPI controllers into reset state on suspend")
>> > > > Signed-off-by: Rajat Jain <[email protected]>
>> > > > ---
>> > > > drivers/mfd/intel-lpss.h | 7 +------
>> > > > 1 file changed, 1 insertion(+), 6 deletions(-)
>> > > >
>> > > > diff --git a/drivers/mfd/intel-lpss.h b/drivers/mfd/intel-lpss.h
>> > > > index 694116630ffa..865bbeaaf00c 100644
>> > > > --- a/drivers/mfd/intel-lpss.h
>> > > > +++ b/drivers/mfd/intel-lpss.h
>> > > > @@ -38,12 +38,7 @@ int intel_lpss_resume(struct device *dev);
>> > > > #ifdef CONFIG_PM_SLEEP
>> > > > #define INTEL_LPSS_SLEEP_PM_OPS \
>> > > > .prepare = intel_lpss_prepare, \
>> > > > - .suspend = intel_lpss_suspend, \
>> > > > - .resume = intel_lpss_resume, \
>> > > > - .freeze = intel_lpss_suspend, \
>> > > > - .thaw = intel_lpss_resume, \
>> > > > - .poweroff = intel_lpss_suspend, \
>> > > > - .restore = intel_lpss_resume,
>> > > > + SET_LATE_SYSTEM_SLEEP_PM_OPS(intel_lpss_suspend, intel_lpss_resume)
>> > > > #else
>> > > > #define INTEL_LPSS_SLEEP_PM_OPS
>> > > > #endif
>> > >
>> > > So I sent this exact patch several days ago:
>> > > https://patchwork.kernel.org/patch/9939809/
>> >
>> > You did?
>>
>> Yes, I did.
>>
>> > Any reason you didn't send it to me?
>>
>> Well, Lee Jones <[email protected]> is very much in the CC list of it.
>>
>> Was I expected to use a different e-mail address for that?
>
> I don't monitor that one. Better to use the one in MAINTAINERS.
OK, I will in the future.
On Thu, Sep 21, 2017 at 4:11 PM, Rafael J. Wysocki <[email protected]> wrote:
> On Thursday, September 21, 2017 3:13:56 AM CEST Rajat Jain wrote:
>> On Wed, Sep 20, 2017 at 5:24 PM, Rafael J. Wysocki <[email protected]> wrote:
>> > On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <[email protected]> wrote:
>> >> Ref: https://lkml.org/lkml/2017/9/19/649
>> >>
>> >> The bus controllers should suspend the bus operations only after
>> >> all of the devices on the bus have suspended their device
>> >> completely. Since the i2c_client drivers could be talking to
>> >> their devices in their suspend_late() calls, lets ensure that the
>> >> bus is alive by that time. Thus moving the controller suspend logic to
>> >> suspend_late().
>> >>
>> >> Signed-off-by: Rajat Jain <[email protected]>
>> >> ---
>> >> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>> >>
>> >> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
>> >> index 0e65b97842b4..66dd7f844c40 100644
>> >> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
>> >> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
>> >> @@ -468,7 +468,7 @@ static int dw_i2c_plat_suspend(struct device *dev)
>> >> static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
>> >> .prepare = dw_i2c_plat_prepare,
>> >> .complete = dw_i2c_plat_complete,
>> >> - SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
>> >> + SET_LATE_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
>> >> SET_RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend,
>> >> dw_i2c_plat_resume,
>> >> NULL)
>> >
>> > No, you can't just do that.
>> >
>> > I sent patches to do it properly before my trip to LA last week, it
>> > shouldn't be overly difficult to find them in the mailing list
>> > archives. I can look them up tomorrow if need be.
>>
>> Thanks, I am guessing you mean this?
>>
>> https://patchwork.kernel.org/patch/9939807/
>
> Yes, that's what I mean.
BTW, does this patchset work for you?
Thanks,
Rafael
On Sat, Sep 23, 2017 at 5:55 AM, Rafael J. Wysocki <[email protected]> wrote:
> On Thu, Sep 21, 2017 at 4:11 PM, Rafael J. Wysocki <[email protected]> wrote:
>> On Thursday, September 21, 2017 3:13:56 AM CEST Rajat Jain wrote:
>>> On Wed, Sep 20, 2017 at 5:24 PM, Rafael J. Wysocki <[email protected]> wrote:
>>> > On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <[email protected]> wrote:
>>> >> Ref: https://lkml.org/lkml/2017/9/19/649
>>> >>
>>> >> The bus controllers should suspend the bus operations only after
>>> >> all of the devices on the bus have suspended their device
>>> >> completely. Since the i2c_client drivers could be talking to
>>> >> their devices in their suspend_late() calls, lets ensure that the
>>> >> bus is alive by that time. Thus moving the controller suspend logic to
>>> >> suspend_late().
>>> >>
>>> >> Signed-off-by: Rajat Jain <[email protected]>
>>> >> ---
>>> >> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
>>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>>> >>
>>> >> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
>>> >> index 0e65b97842b4..66dd7f844c40 100644
>>> >> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
>>> >> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
>>> >> @@ -468,7 +468,7 @@ static int dw_i2c_plat_suspend(struct device *dev)
>>> >> static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
>>> >> .prepare = dw_i2c_plat_prepare,
>>> >> .complete = dw_i2c_plat_complete,
>>> >> - SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
>>> >> + SET_LATE_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
>>> >> SET_RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend,
>>> >> dw_i2c_plat_resume,
>>> >> NULL)
>>> >
>>> > No, you can't just do that.
>>> >
>>> > I sent patches to do it properly before my trip to LA last week, it
>>> > shouldn't be overly difficult to find them in the mailing list
>>> > archives. I can look them up tomorrow if need be.
>>>
>>> Thanks, I am guessing you mean this?
>>>
>>> https://patchwork.kernel.org/patch/9939807/
>>
>> Yes, that's what I mean.
>
> BTW, does this patchset work for you?
Yes, I don't see the issue I was seeing earlier with your patch.
Please feel free to add
Tested-by: Rajat Jain <[email protected]>
>
> Thanks,
> Rafael
On Sat, Sep 23, 2017 at 8:49 AM, Rajat Jain <[email protected]> wrote:
> On Sat, Sep 23, 2017 at 5:55 AM, Rafael J. Wysocki <[email protected]> wrote:
>> On Thu, Sep 21, 2017 at 4:11 PM, Rafael J. Wysocki <[email protected]> wrote:
>>> On Thursday, September 21, 2017 3:13:56 AM CEST Rajat Jain wrote:
>>>> On Wed, Sep 20, 2017 at 5:24 PM, Rafael J. Wysocki <[email protected]> wrote:
>>>> > On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <[email protected]> wrote:
>>>> >> Ref: https://lkml.org/lkml/2017/9/19/649
>>>> >>
>>>> >> The bus controllers should suspend the bus operations only after
>>>> >> all of the devices on the bus have suspended their device
>>>> >> completely. Since the i2c_client drivers could be talking to
>>>> >> their devices in their suspend_late() calls, lets ensure that the
>>>> >> bus is alive by that time. Thus moving the controller suspend logic to
>>>> >> suspend_late().
>>>> >>
>>>> >> Signed-off-by: Rajat Jain <[email protected]>
>>>> >> ---
>>>> >> drivers/i2c/busses/i2c-designware-platdrv.c | 2 +-
>>>> >> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>> >>
>>>> >> diff --git a/drivers/i2c/busses/i2c-designware-platdrv.c b/drivers/i2c/busses/i2c-designware-platdrv.c
>>>> >> index 0e65b97842b4..66dd7f844c40 100644
>>>> >> --- a/drivers/i2c/busses/i2c-designware-platdrv.c
>>>> >> +++ b/drivers/i2c/busses/i2c-designware-platdrv.c
>>>> >> @@ -468,7 +468,7 @@ static int dw_i2c_plat_suspend(struct device *dev)
>>>> >> static const struct dev_pm_ops dw_i2c_dev_pm_ops = {
>>>> >> .prepare = dw_i2c_plat_prepare,
>>>> >> .complete = dw_i2c_plat_complete,
>>>> >> - SET_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
>>>> >> + SET_LATE_SYSTEM_SLEEP_PM_OPS(dw_i2c_plat_suspend, dw_i2c_plat_resume)
>>>> >> SET_RUNTIME_PM_OPS(dw_i2c_plat_runtime_suspend,
>>>> >> dw_i2c_plat_resume,
>>>> >> NULL)
>>>> >
>>>> > No, you can't just do that.
>>>> >
>>>> > I sent patches to do it properly before my trip to LA last week, it
>>>> > shouldn't be overly difficult to find them in the mailing list
>>>> > archives. I can look them up tomorrow if need be.
>>>>
>>>> Thanks, I am guessing you mean this?
>>>>
>>>> https://patchwork.kernel.org/patch/9939807/
>>>
>>> Yes, that's what I mean.
>>
>> BTW, does this patchset work for you?
>
> Yes, I don't see the issue I was seeing earlier with your patch.
> Please feel free to adds,
I think I made my sentence ambiguous. Let me rephrase: Yes, with your
patch, I don't see the issue I was seeing earlier (without your patch)
>
> Tested-by: Rajat Jain <[email protected]>
>
>>
>> Thanks,
>> Rafael