* [GIT PULL] i2c: tegra: Changes for v5.8-rc1
@ 2020-05-15 14:39 Thierry Reding
2020-05-16 7:45 ` Dmitry Osipenko
2020-05-20 13:40 ` Wolfram Sang
0 siblings, 2 replies; 7+ messages in thread
From: Thierry Reding @ 2020-05-15 14:39 UTC (permalink / raw)
To: Wolfram Sang; +Cc: linux-i2c, linux-tegra
Hi,
The following changes since commit 0e698dfa282211e414076f9dc7e83c1c288314fd:
Linux 5.7-rc4 (2020-05-03 14:56:04 -0700)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git tags/for-5.8-i2c
for you to fetch changes up to c73178b93754edd8449dccd3faf05baafd4d3f0e:
i2c: tegra: Add support for the VI I2C on Tegra210 (2020-05-12 22:47:52 +0200)
Thanks,
Thierry
----------------------------------------------------------------
i2c: tegra: Changes for v5.8-rc1
This includes a few improvements to make the Tegra I2C controller behave
properly on suspend/resume, does a bit of cleanup and adds support for
the VI-variant of the I2C controller that is used primarily for video
capture purposes.
----------------------------------------------------------------
Dmitry Osipenko (2):
i2c: tegra: Better handle case where CPU0 is busy for a long time
i2c: tegra: Synchronize DMA before termination
Thierry Reding (5):
Revert "i2c: tegra: Fix suspending in active runtime PM state"
i2c: tegra: Restore pinmux on system resume
i2c: tegra: Keep IRQs enabled during suspend/resume
i2c: tegra: Use FIELD_PREP/FIELD_GET macros
i2c: tegra: Add support for the VI I2C on Tegra210
drivers/i2c/busses/i2c-tegra.c | 248 +++++++++++++++++++++++++++++------------
1 file changed, 179 insertions(+), 69 deletions(-)
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [GIT PULL] i2c: tegra: Changes for v5.8-rc1 2020-05-15 14:39 [GIT PULL] i2c: tegra: Changes for v5.8-rc1 Thierry Reding @ 2020-05-16 7:45 ` Dmitry Osipenko 2020-05-19 16:08 ` Thierry Reding 2020-05-20 13:40 ` Wolfram Sang 1 sibling, 1 reply; 7+ messages in thread From: Dmitry Osipenko @ 2020-05-16 7:45 UTC (permalink / raw) To: Thierry Reding, Wolfram Sang; +Cc: linux-i2c, linux-tegra 15.05.2020 17:39, Thierry Reding пишет: > Hi, > > The following changes since commit 0e698dfa282211e414076f9dc7e83c1c288314fd: > > Linux 5.7-rc4 (2020-05-03 14:56:04 -0700) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git tags/for-5.8-i2c > > for you to fetch changes up to c73178b93754edd8449dccd3faf05baafd4d3f0e: > > i2c: tegra: Add support for the VI I2C on Tegra210 (2020-05-12 22:47:52 +0200) > > Thanks, > Thierry > > ---------------------------------------------------------------- > i2c: tegra: Changes for v5.8-rc1 > > This includes a few improvements to make the Tegra I2C controller behave > properly on suspend/resume, does a bit of cleanup and adds support for > the VI-variant of the I2C controller that is used primarily for video > capture purposes. > > ---------------------------------------------------------------- > Dmitry Osipenko (2): > i2c: tegra: Better handle case where CPU0 is busy for a long time > i2c: tegra: Synchronize DMA before termination > > Thierry Reding (5): > Revert "i2c: tegra: Fix suspending in active runtime PM state" > i2c: tegra: Restore pinmux on system resume In general this series is good to me, although I have some concerns about this patch. Could you please answer the review comments? > i2c: tegra: Keep IRQs enabled during suspend/resume > i2c: tegra: Use FIELD_PREP/FIELD_GET macros > i2c: tegra: Add support for the VI I2C on Tegra210 > > drivers/i2c/busses/i2c-tegra.c | 248 +++++++++++++++++++++++++++++------------ > 1 file changed, 179 insertions(+), 69 deletions(-) > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] i2c: tegra: Changes for v5.8-rc1 2020-05-16 7:45 ` Dmitry Osipenko @ 2020-05-19 16:08 ` Thierry Reding 2020-05-20 2:19 ` Dmitry Osipenko 0 siblings, 1 reply; 7+ messages in thread From: Thierry Reding @ 2020-05-19 16:08 UTC (permalink / raw) To: Dmitry Osipenko; +Cc: Wolfram Sang, linux-i2c, linux-tegra [-- Attachment #1: Type: text/plain, Size: 1632 bytes --] On Sat, May 16, 2020 at 10:45:32AM +0300, Dmitry Osipenko wrote: > 15.05.2020 17:39, Thierry Reding пишет: > > Hi, > > > > The following changes since commit 0e698dfa282211e414076f9dc7e83c1c288314fd: > > > > Linux 5.7-rc4 (2020-05-03 14:56:04 -0700) > > > > are available in the Git repository at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git tags/for-5.8-i2c > > > > for you to fetch changes up to c73178b93754edd8449dccd3faf05baafd4d3f0e: > > > > i2c: tegra: Add support for the VI I2C on Tegra210 (2020-05-12 22:47:52 +0200) > > > > Thanks, > > Thierry > > > > ---------------------------------------------------------------- > > i2c: tegra: Changes for v5.8-rc1 > > > > This includes a few improvements to make the Tegra I2C controller behave > > properly on suspend/resume, does a bit of cleanup and adds support for > > the VI-variant of the I2C controller that is used primarily for video > > capture purposes. > > > > ---------------------------------------------------------------- > > Dmitry Osipenko (2): > > i2c: tegra: Better handle case where CPU0 is busy for a long time > > i2c: tegra: Synchronize DMA before termination > > > > Thierry Reding (5): > > Revert "i2c: tegra: Fix suspending in active runtime PM state" > > > i2c: tegra: Restore pinmux on system resume > > In general this series is good to me, although I have some concerns > about this patch. Could you please answer the review comments? Sorry, those had been burried under too much email. I've answered your questions now. Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] i2c: tegra: Changes for v5.8-rc1 2020-05-19 16:08 ` Thierry Reding @ 2020-05-20 2:19 ` Dmitry Osipenko 2020-05-20 10:51 ` Thierry Reding 0 siblings, 1 reply; 7+ messages in thread From: Dmitry Osipenko @ 2020-05-20 2:19 UTC (permalink / raw) To: Thierry Reding; +Cc: Wolfram Sang, linux-i2c, linux-tegra 19.05.2020 19:08, Thierry Reding пишет: > On Sat, May 16, 2020 at 10:45:32AM +0300, Dmitry Osipenko wrote: >> 15.05.2020 17:39, Thierry Reding пишет: >>> Hi, >>> >>> The following changes since commit 0e698dfa282211e414076f9dc7e83c1c288314fd: >>> >>> Linux 5.7-rc4 (2020-05-03 14:56:04 -0700) >>> >>> are available in the Git repository at: >>> >>> git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git tags/for-5.8-i2c >>> >>> for you to fetch changes up to c73178b93754edd8449dccd3faf05baafd4d3f0e: >>> >>> i2c: tegra: Add support for the VI I2C on Tegra210 (2020-05-12 22:47:52 +0200) >>> >>> Thanks, >>> Thierry >>> >>> ---------------------------------------------------------------- >>> i2c: tegra: Changes for v5.8-rc1 >>> >>> This includes a few improvements to make the Tegra I2C controller behave >>> properly on suspend/resume, does a bit of cleanup and adds support for >>> the VI-variant of the I2C controller that is used primarily for video >>> capture purposes. >>> >>> ---------------------------------------------------------------- >>> Dmitry Osipenko (2): >>> i2c: tegra: Better handle case where CPU0 is busy for a long time >>> i2c: tegra: Synchronize DMA before termination >>> >>> Thierry Reding (5): >>> Revert "i2c: tegra: Fix suspending in active runtime PM state" >> >>> i2c: tegra: Restore pinmux on system resume >> >> In general this series is good to me, although I have some concerns >> about this patch. Could you please answer the review comments? > > Sorry, those had been burried under too much email. I've answered your > questions now. Hello Thierry, Thank you for the answers, I'd also want to see the answer to the question about how RPM works, i.e. why I2C is RPM-resumed during system's suspend in some cases and not the others. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] i2c: tegra: Changes for v5.8-rc1 2020-05-20 2:19 ` Dmitry Osipenko @ 2020-05-20 10:51 ` Thierry Reding 2020-05-21 16:02 ` Dmitry Osipenko 0 siblings, 1 reply; 7+ messages in thread From: Thierry Reding @ 2020-05-20 10:51 UTC (permalink / raw) To: Dmitry Osipenko; +Cc: Wolfram Sang, linux-i2c, linux-tegra [-- Attachment #1: Type: text/plain, Size: 3512 bytes --] On Wed, May 20, 2020 at 05:19:27AM +0300, Dmitry Osipenko wrote: > 19.05.2020 19:08, Thierry Reding пишет: > > On Sat, May 16, 2020 at 10:45:32AM +0300, Dmitry Osipenko wrote: > >> 15.05.2020 17:39, Thierry Reding пишет: > >>> Hi, > >>> > >>> The following changes since commit 0e698dfa282211e414076f9dc7e83c1c288314fd: > >>> > >>> Linux 5.7-rc4 (2020-05-03 14:56:04 -0700) > >>> > >>> are available in the Git repository at: > >>> > >>> git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git tags/for-5.8-i2c > >>> > >>> for you to fetch changes up to c73178b93754edd8449dccd3faf05baafd4d3f0e: > >>> > >>> i2c: tegra: Add support for the VI I2C on Tegra210 (2020-05-12 22:47:52 +0200) > >>> > >>> Thanks, > >>> Thierry > >>> > >>> ---------------------------------------------------------------- > >>> i2c: tegra: Changes for v5.8-rc1 > >>> > >>> This includes a few improvements to make the Tegra I2C controller behave > >>> properly on suspend/resume, does a bit of cleanup and adds support for > >>> the VI-variant of the I2C controller that is used primarily for video > >>> capture purposes. > >>> > >>> ---------------------------------------------------------------- > >>> Dmitry Osipenko (2): > >>> i2c: tegra: Better handle case where CPU0 is busy for a long time > >>> i2c: tegra: Synchronize DMA before termination > >>> > >>> Thierry Reding (5): > >>> Revert "i2c: tegra: Fix suspending in active runtime PM state" > >> > >>> i2c: tegra: Restore pinmux on system resume > >> > >> In general this series is good to me, although I have some concerns > >> about this patch. Could you please answer the review comments? > > > > Sorry, those had been burried under too much email. I've answered your > > questions now. > > Hello Thierry, > > Thank you for the answers, I'd also want to see the answer to the > question about how RPM works, i.e. why I2C is RPM-resumed during > system's suspend in some cases and not the others. I don't think I've seen a question regarding that, so let me answer here: the way I understand how this works is that for each device the PM core will grab a runtime PM reference during the suspend/resume transition, which is done as a way of preventing any of the resumed devices from unexpectedly going to runtime suspend. This code is in drivers/base/power/main.c, device_prepare(). Note that this will only increment the runtime PM usage counter (and therefore block the device from entering runtime suspend) but it won't resume the device if it is already suspended. So far any devices that are already runtime suspended, there isn't really anything we have to do during suspend. For those who are not yet suspended, we need to force them into suspend so that their clocks are disabled and the pinmux state set properly (to "idle"). On resume we first need to runtime resume the device undconditionally so that context can be restored. Runtime resume will take care of enabling the clock and setting the pinmux state to "default". Then we restore the context and if the device had been runtime suspended, we manually call the runtime suspend callback again so that the state matches what the PM core thinks it is. However, if the device had not been runtime suspended during system suspend, then we must not manually put it back into suspend because it would no longer match the PM core's state. Does that answer your question? Thierry [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] i2c: tegra: Changes for v5.8-rc1 2020-05-20 10:51 ` Thierry Reding @ 2020-05-21 16:02 ` Dmitry Osipenko 0 siblings, 0 replies; 7+ messages in thread From: Dmitry Osipenko @ 2020-05-21 16:02 UTC (permalink / raw) To: Thierry Reding; +Cc: Wolfram Sang, linux-i2c, linux-tegra 20.05.2020 13:51, Thierry Reding пишет: > On Wed, May 20, 2020 at 05:19:27AM +0300, Dmitry Osipenko wrote: >> 19.05.2020 19:08, Thierry Reding пишет: >>> On Sat, May 16, 2020 at 10:45:32AM +0300, Dmitry Osipenko wrote: >>>> 15.05.2020 17:39, Thierry Reding пишет: >>>>> Hi, >>>>> >>>>> The following changes since commit 0e698dfa282211e414076f9dc7e83c1c288314fd: >>>>> >>>>> Linux 5.7-rc4 (2020-05-03 14:56:04 -0700) >>>>> >>>>> are available in the Git repository at: >>>>> >>>>> git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git tags/for-5.8-i2c >>>>> >>>>> for you to fetch changes up to c73178b93754edd8449dccd3faf05baafd4d3f0e: >>>>> >>>>> i2c: tegra: Add support for the VI I2C on Tegra210 (2020-05-12 22:47:52 +0200) >>>>> >>>>> Thanks, >>>>> Thierry >>>>> >>>>> ---------------------------------------------------------------- >>>>> i2c: tegra: Changes for v5.8-rc1 >>>>> >>>>> This includes a few improvements to make the Tegra I2C controller behave >>>>> properly on suspend/resume, does a bit of cleanup and adds support for >>>>> the VI-variant of the I2C controller that is used primarily for video >>>>> capture purposes. >>>>> >>>>> ---------------------------------------------------------------- >>>>> Dmitry Osipenko (2): >>>>> i2c: tegra: Better handle case where CPU0 is busy for a long time >>>>> i2c: tegra: Synchronize DMA before termination >>>>> >>>>> Thierry Reding (5): >>>>> Revert "i2c: tegra: Fix suspending in active runtime PM state" >>>> >>>>> i2c: tegra: Restore pinmux on system resume >>>> >>>> In general this series is good to me, although I have some concerns >>>> about this patch. Could you please answer the review comments? >>> >>> Sorry, those had been burried under too much email. I've answered your >>> questions now. >> >> Hello Thierry, >> >> Thank you for the answers, I'd also want to see the answer to the >> question about how RPM works, i.e. why I2C is RPM-resumed during >> system's suspend in some cases and not the others. > > I don't think I've seen a question regarding that, so let me answer > here: the way I understand how this works is that for each device the PM > core will grab a runtime PM reference during the suspend/resume > transition, which is done as a way of preventing any of the resumed > devices from unexpectedly going to runtime suspend. > > This code is in drivers/base/power/main.c, device_prepare(). Note that > this will only increment the runtime PM usage counter (and therefore > block the device from entering runtime suspend) but it won't resume the > device if it is already suspended. Does it mean that device won't be RPM-suspended if RPM-resume is invoked during of system's suspend? I tried to dig through the code, commit messages, kernel docs, etc and came to a conclusion that it's hacks made on top of other hacks in order to accommodate needs of the PCI subsystem. This behaviour isn't documented, which makes RPM a bit awkward to use. > So far any devices that are already runtime suspended, there isn't > really anything we have to do during suspend. For those who are not yet > suspended, we need to force them into suspend so that their clocks are > disabled and the pinmux state set properly (to "idle"). > > On resume we first need to runtime resume the device undconditionally so > that context can be restored. Runtime resume will take care of enabling > the clock and setting the pinmux state to "default". Then we restore the > context and if the device had been runtime suspended, we manually call > the runtime suspend callback again so that the state matches what the PM > core thinks it is. However, if the device had not been runtime suspended > during system suspend, then we must not manually put it back into > suspend because it would no longer match the PM core's state. > > Does that answer your question? Yes, this partially answers my question, thank you. Still I don't have a full picture. I also don't like the fact that RPM behavior during of system's suspend isn't symmetrical to system's resume, yet not sure if it is really intended. I.e. that RPM allows to resume during of late suspend, but not on early resume. Anyways, I haven't noticed any problems using this series, so should be good. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [GIT PULL] i2c: tegra: Changes for v5.8-rc1 2020-05-15 14:39 [GIT PULL] i2c: tegra: Changes for v5.8-rc1 Thierry Reding 2020-05-16 7:45 ` Dmitry Osipenko @ 2020-05-20 13:40 ` Wolfram Sang 1 sibling, 0 replies; 7+ messages in thread From: Wolfram Sang @ 2020-05-20 13:40 UTC (permalink / raw) To: Thierry Reding; +Cc: linux-i2c, linux-tegra [-- Attachment #1: Type: text/plain, Size: 783 bytes --] On Fri, May 15, 2020 at 04:39:24PM +0200, Thierry Reding wrote: > Hi, > > The following changes since commit 0e698dfa282211e414076f9dc7e83c1c288314fd: > > Linux 5.7-rc4 (2020-05-03 14:56:04 -0700) > > are available in the Git repository at: > > git://git.kernel.org/pub/scm/linux/kernel/git/tegra/linux.git tags/for-5.8-i2c > > for you to fetch changes up to c73178b93754edd8449dccd3faf05baafd4d3f0e: > > i2c: tegra: Add support for the VI I2C on Tegra210 (2020-05-12 22:47:52 +0200) > > Thanks, > Thierry > Thanks! I pulled it into my for-next branch to get some more testing of all this. I can merge updated branches if that should be needed, of course. I see there are still some on-going questions running but no real show stopper AFAICS. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 833 bytes --] ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-05-21 16:03 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-05-15 14:39 [GIT PULL] i2c: tegra: Changes for v5.8-rc1 Thierry Reding 2020-05-16 7:45 ` Dmitry Osipenko 2020-05-19 16:08 ` Thierry Reding 2020-05-20 2:19 ` Dmitry Osipenko 2020-05-20 10:51 ` Thierry Reding 2020-05-21 16:02 ` Dmitry Osipenko 2020-05-20 13:40 ` Wolfram Sang
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).