Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1736205 > unrolled thread

[PATCH 1/2] i2c: designware: switch to suspend_late/resume_early

Started byRajat Jain <rajatja@google.com>
First post2017-09-21 00:40 +0200
Last post2017-09-23 18:20 +0200
Articles 8 — 4 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 1/2] i2c: designware: switch to suspend_late/resume_early Rajat Jain <rajatja@google.com> - 2017-09-21 00:40 +0200
    Re: [PATCH 1/2] i2c: designware: switch to suspend_late/resume_early Rajat Jain <rajatxjain@gmail.com> - 2017-09-21 00:50 +0200
    Re: [PATCH 1/2] i2c: designware: switch to suspend_late/resume_early "Rafael J. Wysocki" <rafael@kernel.org> - 2017-09-21 02:30 +0200
      Re: [PATCH 1/2] i2c: designware: switch to suspend_late/resume_early Rajat Jain <rajatja@google.com> - 2017-09-21 03:20 +0200
        Re: [PATCH 1/2] i2c: designware: switch to suspend_late/resume_early "Rafael J. Wysocki" <rjw@rjwysocki.net> - 2017-09-21 16:30 +0200
          Re: [PATCH 1/2] i2c: designware: switch to suspend_late/resume_early "Rafael J. Wysocki" <rafael@kernel.org> - 2017-09-23 15:00 +0200
            Re: [PATCH 1/2] i2c: designware: switch to suspend_late/resume_early Rajat Jain <rajatxjain@gmail.com> - 2017-09-23 17:50 +0200
              Re: [PATCH 1/2] i2c: designware: switch to suspend_late/resume_early Rajat Jain <rajatxjain@gmail.com> - 2017-09-23 18:20 +0200

#1736205 — [PATCH 1/2] i2c: designware: switch to suspend_late/resume_early

FromRajat Jain <rajatja@google.com>
Date2017-09-21 00:40 +0200
Subject[PATCH 1/2] i2c: designware: switch to suspend_late/resume_early
Message-ID<urVOW-J7-21@gated-at.bofh.it>
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 <rajatja@google.com>
---
 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

[toc] | [next] | [standalone]

#1736209

FromRajat Jain <rajatxjain@gmail.com>
Date2017-09-21 00:50 +0200
Message-ID<urVYC-MK-3@gated-at.bofh.it>
In reply to#1736205
On Wed, Sep 20, 2017 at 3:31 PM, Rajat Jain <rajatja@google.com> 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 <rajatja@google.com>
> ---

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
>

[toc] | [prev] | [next] | [standalone]

#1736280

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2017-09-21 02:30 +0200
Message-ID<urXxo-1QS-13@gated-at.bofh.it>
In reply to#1736205
On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <rajatja@google.com> 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 <rajatja@google.com>
> ---
>  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

[toc] | [prev] | [next] | [standalone]

#1736312

FromRajat Jain <rajatja@google.com>
Date2017-09-21 03:20 +0200
Message-ID<urYjL-2of-5@gated-at.bofh.it>
In reply to#1736280
On Wed, Sep 20, 2017 at 5:24 PM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <rajatja@google.com> 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 <rajatja@google.com>
>> ---
>>  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

[toc] | [prev] | [next] | [standalone]

#1736709

From"Rafael J. Wysocki" <rjw@rjwysocki.net>
Date2017-09-21 16:30 +0200
Message-ID<usaEi-297-15@gated-at.bofh.it>
In reply to#1736312
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 <rafael@kernel.org> wrote:
> > On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <rajatja@google.com> 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 <rajatja@google.com>
> >> ---
> >>  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

[toc] | [prev] | [next] | [standalone]

#1737997

From"Rafael J. Wysocki" <rafael@kernel.org>
Date2017-09-23 15:00 +0200
Message-ID<usSch-3ca-1@gated-at.bofh.it>
In reply to#1736709
On Thu, Sep 21, 2017 at 4:11 PM, Rafael J. Wysocki <rjw@rjwysocki.net> 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 <rafael@kernel.org> wrote:
>> > On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <rajatja@google.com> 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 <rajatja@google.com>
>> >> ---
>> >>  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

[toc] | [prev] | [next] | [standalone]

#1738043

FromRajat Jain <rajatxjain@gmail.com>
Date2017-09-23 17:50 +0200
Message-ID<usUQN-4T0-3@gated-at.bofh.it>
In reply to#1737997
On Sat, Sep 23, 2017 at 5:55 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
> On Thu, Sep 21, 2017 at 4:11 PM, Rafael J. Wysocki <rjw@rjwysocki.net> 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 <rafael@kernel.org> wrote:
>>> > On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <rajatja@google.com> 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 <rajatja@google.com>
>>> >> ---
>>> >>  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 <rajatja@google.com>

>
> Thanks,
> Rafael

[toc] | [prev] | [next] | [standalone]

#1738045

FromRajat Jain <rajatxjain@gmail.com>
Date2017-09-23 18:20 +0200
Message-ID<usVjQ-5i1-27@gated-at.bofh.it>
In reply to#1738043
On Sat, Sep 23, 2017 at 8:49 AM, Rajat Jain <rajatxjain@gmail.com> wrote:
> On Sat, Sep 23, 2017 at 5:55 AM, Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Thu, Sep 21, 2017 at 4:11 PM, Rafael J. Wysocki <rjw@rjwysocki.net> 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 <rafael@kernel.org> wrote:
>>>> > On Thu, Sep 21, 2017 at 12:31 AM, Rajat Jain <rajatja@google.com> 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 <rajatja@google.com>
>>>> >> ---
>>>> >>  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 <rajatja@google.com>
>
>>
>> Thanks,
>> Rafael

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web