From: Kevin Hilman <khilman@linaro.org>
To: Doug Anderson <dianders@chromium.org>
Cc: Wolfram Sang <wsa@the-dreams.de>,
Kukjin Kim <kgene.kim@samsung.com>,
Tomasz Figa <tomasz.figa@gmail.com>,
Javier Martinez Canillas <javier.martinez@collabora.co.uk>,
naveen krishna <ch.naveen@samsung.com>,
Jingoo Han <jg1.han@samsung.com>, Jean Delvare <jdelvare@suse.de>,
Simon Glass <sjg@google.com>,
Paul Gortmaker <paul.gortmaker@windriver.com>,
Masanari Iida <standby24x7@gmail.com>,
Sachin Kamat <sachin.kamat@linaro.org>,
"linux-i2c@vger.kernel.org" <linux-i2c@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] i2c: exynos5: Properly use the "noirq" variants of suspend/resume
Date: Mon, 23 Jun 2014 16:31:21 -0700 [thread overview]
Message-ID: <7hwqc7nqye.fsf@paris.lan> (raw)
In-Reply-To: <CAD=FV=VBcWC2Z2WCmsiZUPCrakqrdd9Cd6QL9TLYS4c23+sMkA@mail.gmail.com> (Doug Anderson's message of "Mon, 23 Jun 2014 15:42:01 -0700")
Doug Anderson <dianders@chromium.org> writes:
> Kevin,
>
> On Mon, Jun 23, 2014 at 3:23 PM, Kevin Hilman <khilman@linaro.org> wrote:
>>> So I guess in this case the truly correct way to handle it is:
>>>
>>> 1. i2c controller should have Runtime PM even though (as per the code
>>> now) there's nothing you can do to it to save power under normal
>>> circumstances. So the runtime "suspend" code would be a no-op.
>>>
>>> 2. When the i2c controller is told to runtime resume, it should
>>> double-check if a full SoC poweroff has happened since the last time
>>> it checked. In this case it should reinit its hardware.
>>>
>>> 3. If the i2c controller gets a full "resume" callback then it should
>>> also reinit the hardware just so it's not sitting in a half-configured
>>> state until the first peripheral uses it.
>>>
>>> If later someone finds a way to power gate the i2c controller when no
>>> active transfers are going (and we actually save non-trivial power
>>> doing this) then we've got a nice place to put that code.
>>>
>>> NOTE: Unless we can actually save power by power gating the i2c
>>> peripheral when there are no active transfers, we would also just have
>>> the i2c_xfer() init the hardware if needed. Maybe that's kinda gross,
>>> though.
>>
>> Yes, this is how we manage the i2c controller on OMAP.
>>
>> Essentially, between every xfer, the hw is disabled and can potentially
>> lose context, so eveery xfer requires a hw init. We use the runtime PM
>> "autosuspend" feature so that it stays alive for X milliseconds so
>> bursty i2c xfers are not punished.
>>
>> Have a look at drivers/i2c/busses/i2c-omap.c.
>>
>> You'll notice there are not callbacks for system suspend/resume, it's
>> only doing runtime PM.
>
> OK, cool! That might be a bit too aggressive of a change for what I
> can take on right now. I've filed http://crbug.com/388007 to see if
> Samsung can take a look at this.
Sure. While I think moving to runtime PM is the right thing to do, that
alone shouldn't block this patch.
Kevin
WARNING: multiple messages have this Message-ID (diff)
From: khilman@linaro.org (Kevin Hilman)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] i2c: exynos5: Properly use the "noirq" variants of suspend/resume
Date: Mon, 23 Jun 2014 16:31:21 -0700 [thread overview]
Message-ID: <7hwqc7nqye.fsf@paris.lan> (raw)
In-Reply-To: <CAD=FV=VBcWC2Z2WCmsiZUPCrakqrdd9Cd6QL9TLYS4c23+sMkA@mail.gmail.com> (Doug Anderson's message of "Mon, 23 Jun 2014 15:42:01 -0700")
Doug Anderson <dianders@chromium.org> writes:
> Kevin,
>
> On Mon, Jun 23, 2014 at 3:23 PM, Kevin Hilman <khilman@linaro.org> wrote:
>>> So I guess in this case the truly correct way to handle it is:
>>>
>>> 1. i2c controller should have Runtime PM even though (as per the code
>>> now) there's nothing you can do to it to save power under normal
>>> circumstances. So the runtime "suspend" code would be a no-op.
>>>
>>> 2. When the i2c controller is told to runtime resume, it should
>>> double-check if a full SoC poweroff has happened since the last time
>>> it checked. In this case it should reinit its hardware.
>>>
>>> 3. If the i2c controller gets a full "resume" callback then it should
>>> also reinit the hardware just so it's not sitting in a half-configured
>>> state until the first peripheral uses it.
>>>
>>> If later someone finds a way to power gate the i2c controller when no
>>> active transfers are going (and we actually save non-trivial power
>>> doing this) then we've got a nice place to put that code.
>>>
>>> NOTE: Unless we can actually save power by power gating the i2c
>>> peripheral when there are no active transfers, we would also just have
>>> the i2c_xfer() init the hardware if needed. Maybe that's kinda gross,
>>> though.
>>
>> Yes, this is how we manage the i2c controller on OMAP.
>>
>> Essentially, between every xfer, the hw is disabled and can potentially
>> lose context, so eveery xfer requires a hw init. We use the runtime PM
>> "autosuspend" feature so that it stays alive for X milliseconds so
>> bursty i2c xfers are not punished.
>>
>> Have a look at drivers/i2c/busses/i2c-omap.c.
>>
>> You'll notice there are not callbacks for system suspend/resume, it's
>> only doing runtime PM.
>
> OK, cool! That might be a bit too aggressive of a change for what I
> can take on right now. I've filed http://crbug.com/388007 to see if
> Samsung can take a look at this.
Sure. While I think moving to runtime PM is the right thing to do, that
alone shouldn't block this patch.
Kevin
WARNING: multiple messages have this Message-ID (diff)
From: Kevin Hilman <khilman@linaro.org>
To: Doug Anderson <dianders@chromium.org>
Cc: Wolfram Sang <wsa@the-dreams.de>,
Kukjin Kim <kgene.kim@samsung.com>,
Tomasz Figa <tomasz.figa@gmail.com>,
Javier Martinez Canillas <javier.martinez@collabora.co.uk>,
naveen krishna <ch.naveen@samsung.com>,
Jingoo Han <jg1.han@samsung.com>, Jean Delvare <jdelvare@suse.de>,
Simon Glass <sjg@google.com>,
Paul Gortmaker <paul.gortmaker@windriver.com>,
Masanari Iida <standby24x7@gmail.com>,
Sachin Kamat <sachin.kamat@linaro.org>,
"linux-i2c\@vger.kernel.org" <linux-i2c@vger.kernel.org>,
"linux-arm-kernel\@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>,
linux-samsung-soc <linux-samsung-soc@vger.kernel.org>,
"linux-kernel\@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH] i2c: exynos5: Properly use the "noirq" variants of suspend/resume
Date: Mon, 23 Jun 2014 16:31:21 -0700 [thread overview]
Message-ID: <7hwqc7nqye.fsf@paris.lan> (raw)
In-Reply-To: <CAD=FV=VBcWC2Z2WCmsiZUPCrakqrdd9Cd6QL9TLYS4c23+sMkA@mail.gmail.com> (Doug Anderson's message of "Mon, 23 Jun 2014 15:42:01 -0700")
Doug Anderson <dianders@chromium.org> writes:
> Kevin,
>
> On Mon, Jun 23, 2014 at 3:23 PM, Kevin Hilman <khilman@linaro.org> wrote:
>>> So I guess in this case the truly correct way to handle it is:
>>>
>>> 1. i2c controller should have Runtime PM even though (as per the code
>>> now) there's nothing you can do to it to save power under normal
>>> circumstances. So the runtime "suspend" code would be a no-op.
>>>
>>> 2. When the i2c controller is told to runtime resume, it should
>>> double-check if a full SoC poweroff has happened since the last time
>>> it checked. In this case it should reinit its hardware.
>>>
>>> 3. If the i2c controller gets a full "resume" callback then it should
>>> also reinit the hardware just so it's not sitting in a half-configured
>>> state until the first peripheral uses it.
>>>
>>> If later someone finds a way to power gate the i2c controller when no
>>> active transfers are going (and we actually save non-trivial power
>>> doing this) then we've got a nice place to put that code.
>>>
>>> NOTE: Unless we can actually save power by power gating the i2c
>>> peripheral when there are no active transfers, we would also just have
>>> the i2c_xfer() init the hardware if needed. Maybe that's kinda gross,
>>> though.
>>
>> Yes, this is how we manage the i2c controller on OMAP.
>>
>> Essentially, between every xfer, the hw is disabled and can potentially
>> lose context, so eveery xfer requires a hw init. We use the runtime PM
>> "autosuspend" feature so that it stays alive for X milliseconds so
>> bursty i2c xfers are not punished.
>>
>> Have a look at drivers/i2c/busses/i2c-omap.c.
>>
>> You'll notice there are not callbacks for system suspend/resume, it's
>> only doing runtime PM.
>
> OK, cool! That might be a bit too aggressive of a change for what I
> can take on right now. I've filed http://crbug.com/388007 to see if
> Samsung can take a look at this.
Sure. While I think moving to runtime PM is the right thing to do, that
alone shouldn't block this patch.
Kevin
next prev parent reply other threads:[~2014-06-23 23:31 UTC|newest]
Thread overview: 48+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-06-19 5:21 [PATCH] i2c: exynos5: Properly use the "noirq" variants of suspend/resume Doug Anderson
2014-06-19 5:21 ` Doug Anderson
2014-06-19 5:21 ` Doug Anderson
2014-06-19 18:43 ` Kevin Hilman
2014-06-19 18:43 ` Kevin Hilman
2014-06-19 22:43 ` Doug Anderson
2014-06-19 22:43 ` Doug Anderson
2014-06-20 21:48 ` Kevin Hilman
2014-06-20 21:48 ` Kevin Hilman
2014-06-20 21:48 ` Kevin Hilman
[not found] ` <7hwqcbs166.fsf-4poPxKt068f/PtFMR13I2A@public.gmane.org>
2014-06-20 22:05 ` Doug Anderson
2014-06-20 22:05 ` Doug Anderson
2014-06-20 22:05 ` Doug Anderson
2014-06-20 23:13 ` Kevin Hilman
2014-06-20 23:13 ` Kevin Hilman
2014-06-20 23:13 ` Kevin Hilman
2014-06-20 23:53 ` Doug Anderson
2014-06-20 23:53 ` Doug Anderson
[not found] ` <CAD=FV=VmPMf6tCNmKJ1o5J7PyAXoU8fb6+s+Fkv18FH2WaQp7A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-06-20 23:59 ` Tomasz Figa
2014-06-20 23:59 ` Tomasz Figa
2014-06-20 23:59 ` Tomasz Figa
2014-06-23 22:01 ` Doug Anderson
2014-06-23 22:01 ` Doug Anderson
2014-06-23 22:19 ` Kevin Hilman
2014-06-23 22:19 ` Kevin Hilman
2014-06-23 22:19 ` Kevin Hilman
2014-06-23 22:24 ` Tomasz Figa
2014-06-23 22:24 ` Tomasz Figa
2014-06-23 22:27 ` Doug Anderson
2014-06-23 22:27 ` Doug Anderson
[not found] ` <CAD=FV=U0P2kyfnxdU2E1Gm8TdD_cFd6brmtQ7-gpajNJuKmWSA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-06-23 22:31 ` Tomasz Figa
2014-06-23 22:31 ` Tomasz Figa
2014-06-23 22:31 ` Tomasz Figa
[not found] ` <53A8AAC9.8030407-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2014-06-23 22:46 ` Doug Anderson
2014-06-23 22:46 ` Doug Anderson
2014-06-23 22:46 ` Doug Anderson
2014-06-23 23:35 ` Kevin Hilman
2014-06-23 23:35 ` Kevin Hilman
2014-06-23 23:35 ` Kevin Hilman
2014-06-23 22:23 ` Kevin Hilman
2014-06-23 22:23 ` Kevin Hilman
2014-06-23 22:23 ` Kevin Hilman
[not found] ` <7h38evp8ng.fsf-4poPxKt068f/PtFMR13I2A@public.gmane.org>
2014-06-23 22:42 ` Doug Anderson
2014-06-23 22:42 ` Doug Anderson
2014-06-23 22:42 ` Doug Anderson
2014-06-23 23:31 ` Kevin Hilman [this message]
2014-06-23 23:31 ` Kevin Hilman
2014-06-23 23:31 ` Kevin Hilman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7hwqc7nqye.fsf@paris.lan \
--to=khilman@linaro.org \
--cc=ch.naveen@samsung.com \
--cc=dianders@chromium.org \
--cc=javier.martinez@collabora.co.uk \
--cc=jdelvare@suse.de \
--cc=jg1.han@samsung.com \
--cc=kgene.kim@samsung.com \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-i2c@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-samsung-soc@vger.kernel.org \
--cc=paul.gortmaker@windriver.com \
--cc=sachin.kamat@linaro.org \
--cc=sjg@google.com \
--cc=standby24x7@gmail.com \
--cc=tomasz.figa@gmail.com \
--cc=wsa@the-dreams.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.