| Message ID | 1347447496-16793-1-git-send-email-shubhrajyoti@ti.com |
|---|---|
| State | New |
| Headers | show |
On Wed, Sep 12, 2012 at 04:27:54PM +0530, Shubhrajyoti D wrote: > Changes since v1: > - removed tabification on patch 6/17 > - removed dev_err() which was introduced on patch 09/17 > Changes since v2: > - do not set full fifo depth in the RDR interrupt. > - some changelog updates. > - rebase to the Wolfram's tree. > Changes since v3: > - Remove a redundant read of status register > - Read the dev->buf_len variable instead of the register > as the information of the remaining bytes is there. > Changes since v4: > - Ack the arbitration lost. > - Rebase to the i2c-embedded/for-next branch. > Changes since v5: > - Rebase to latest mainline > - Added some more cleanup patches so as have a consolidated series. > Changes since v6: > - Fix comments on setting the pdev to NULL. > - Trivial changelog update > Changes since v7: > - Remove a patch as the code is getting changed anyways > - Changelogs update. > > Previous discussions can be found here > http://www.spinics.net/lists/linux-i2c/msg09764.html > > This is the cleanup only series. > > Tested on omap4sdp and 3430sdp. > > The following changes since commit 55d512e245bc7699a8800e23df1a24195dd08217: > > Linux 3.6-rc5 (2012-09-08 16:43:45 -0700) > > are available in the git repository at: > git://gitorious.org/linus-tree/linus-tree.git for_3.7/i2c/big_cleanups Pushed to -next. Thanks to all involved!
On Wednesday 12 September 2012 06:46 PM, Wolfram Sang wrote: >> Tested on omap4sdp and 3430sdp. >> > >> > The following changes since commit 55d512e245bc7699a8800e23df1a24195dd08217: >> > >> > Linux 3.6-rc5 (2012-09-08 16:43:45 -0700) >> > >> > are available in the git repository at: >> > git://gitorious.org/linus-tree/linus-tree.git for_3.7/i2c/big_cleanups > Pushed to -next. Thanks to all involved! Thanks.
Shubhrajyoti D <shubhrajyoti@ti.com> writes: [...] > > This is the cleanup only series. > > Tested on omap4sdp and 3430sdp. It would be extremely helpful if you would describe how this was tested. And for me, it would be a source of extreme joy if you described any PM testing. I gave this some additional PM testing on 3430/n900, 3530/Overo, 3730/OveroSTORM, 3730/Beagle-xM and 4430/Panda. I tested v3.6-rc5 which passed all PM tests and then added this series (by merging the i2c-embedded/i2c-next branch.) PM tests then fail. At least on 3530/Overo and 3730/Overo, CORE no longer hits retenion (or off) during idle. The easy way to notice this is to see that even when no i2c transactions are happening, the runtime PM status for omap_i2c.1 is remains 'active': # cat omap_i2c.*/power/runtime_status active suspended Of course that means that clocks are never gated during idle, and CORE will never hit retention. I noticed it because my PM tests detected that the CORE powerdomain was not transitioning to retention (or off) during idle. To reproduce, simply enable UART timeouts so CORE will hit retention: echo 3000 > /sys/devices/platform/omap_uart.0/power/autosuspend_delay_ms echo 3000 > /sys/devices/platform/omap_uart.1/power/autosuspend_delay_ms echo 3000 > /sys/devices/platform/omap_uart.2/power/autosuspend_delay_ms echo 3000 > /sys/devices/platform/omap_uart.3/power/autosuspend_delay_ms and check the core_pwrm state counters: cat /debug/pm_debug/count wait > 3 seconds for the UART autosuspend timers to kick in. At this point, CORE should be transitioning to retenion in idle (determined by noticing that the RET counter for core_pwrdm is increasing.) With $SUBJECT series applied, you'll notice that CORE is not transitioning. Kevin > The following changes since commit 55d512e245bc7699a8800e23df1a24195dd08217: > > Linux 3.6-rc5 (2012-09-08 16:43:45 -0700) > > are available in the git repository at: > git://gitorious.org/linus-tree/linus-tree.git for_3.7/i2c/big_cleanups > > > Felipe Balbi (22): > i2c: omap: switch to devm_* API > i2c: omap: simplify num_bytes handling > i2c: omap: decrease indentation level on data handling > i2c: omap: add blank lines > i2c: omap: simplify omap_i2c_ack_stat() > i2c: omap: split out [XR]DR and [XR]RDY > i2c: omap: improve i462 errata handling > i2c: omap: re-factor receive/transmit data loop > i2c: omap: switch over to do {} while loop > i2c: omap: ack IRQ in parts > i2c: omap: switch to platform_get_irq() > i2c: omap: bus: add a receiver flag > i2c: omap: simplify errata check > i2c: omap: always return IRQ_HANDLED > i2c: omap: simplify IRQ exit path > i2c: omap: resize fifos before each message > i2c: omap: get rid of the "complete" label > i2c: omap: always return IRQ_HANDLED > i2c: omap: switch to threaded IRQ support > i2c: omap: remove unnecessary pm_runtime_suspended check > i2c: omap: switch over to autosuspend API > i2c: omap: sanitize exit path > > Shubhrajyoti D (1): > i2c: omap: remove redundant status read > > drivers/i2c/busses/i2c-omap.c | 442 +++++++++++++++++++++++++---------------- > 1 files changed, 271 insertions(+), 171 deletions(-)
Wolfram Sang <w.sang@pengutronix.de> writes: > On Wed, Sep 12, 2012 at 04:27:54PM +0530, Shubhrajyoti D wrote: [...] >> This is the cleanup only series. >> >> Tested on omap4sdp and 3430sdp. >> >> The following changes since commit 55d512e245bc7699a8800e23df1a24195dd08217: >> >> Linux 3.6-rc5 (2012-09-08 16:43:45 -0700) >> >> are available in the git repository at: >> git://gitorious.org/linus-tree/linus-tree.git for_3.7/i2c/big_cleanups > > Pushed to -next. Thanks to all involved! Sorry to be late to the party (again), but still catching up after some time off. Unfortunately, this series causes PM regressions on several OMAP platforms. I hope we can hold off on this until those issues are addressed. We *really* need to have some more thorough testing (especially PM) before merging large series like this. Kevin
Shubhrajyoti D <shubhrajyoti@ti.com> writes: > From: Felipe Balbi <balbi@ti.com> > > this helps us reduce unnecessary pm transitions > in case we have another i2c message starting soon. > > Signed-off-by: Felipe Balbi <balbi@ti.com> > Signed-off-by: Shubhrajyoti D <shubhrajyoti@ti.com> I tracked the PM regression down to this patch. > --- > drivers/i2c/busses/i2c-omap.c | 12 ++++++++++-- > 1 files changed, 10 insertions(+), 2 deletions(-) > > diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c > index 6d38a57..122f517 100644 > --- a/drivers/i2c/busses/i2c-omap.c > +++ b/drivers/i2c/busses/i2c-omap.c > @@ -55,6 +55,9 @@ > /* timeout waiting for the controller to respond */ > #define OMAP_I2C_TIMEOUT (msecs_to_jiffies(1000)) > > +/* timeout for pm runtime autosuspend */ > +#define OMAP_I2C_PM_TIMEOUT 1000 /* ms */ > + > /* For OMAP3 I2C_IV has changed to I2C_WE (wakeup enable) */ > enum { > OMAP_I2C_REV_REG = 0, > @@ -645,7 +648,8 @@ omap_i2c_xfer(struct i2c_adapter *adap, struct i2c_msg msgs[], int num) > > omap_i2c_wait_for_bb(dev); > out: > - pm_runtime_put(dev->dev); > + pm_runtime_mark_last_busy(dev->dev); > + pm_runtime_put_autosuspend(dev->dev); Reverting this change allows CORE to hit retention again. I didn't debug this any further, so I'm not sure exactly why the async suspend works but not the autosuspend. Kevin
Kevin Hilman <khilman@deeprootsystems.com> writes: > Wolfram Sang <w.sang@pengutronix.de> writes: > >> On Wed, Sep 12, 2012 at 04:27:54PM +0530, Shubhrajyoti D wrote: > > [...] > >>> This is the cleanup only series. >>> >>> Tested on omap4sdp and 3430sdp. >>> >>> The following changes since commit 55d512e245bc7699a8800e23df1a24195dd08217: >>> >>> Linux 3.6-rc5 (2012-09-08 16:43:45 -0700) >>> >>> are available in the git repository at: >>> git://gitorious.org/linus-tree/linus-tree.git for_3.7/i2c/big_cleanups >> >> Pushed to -next. Thanks to all involved! > > Sorry to be late to the party (again), but still catching up after some > time off. > > Unfortunately, this series causes PM regressions on several OMAP > platforms. I hope we can hold off on this until those issues are > addressed. I tracked the regression down to [PATCHv8 21/22] (see reply there.) Since this series is already merged, I suggest that the problem patch be reverted, at least for v3.7 and until the problem is better understood and tested. With that patch reverted, all my PM tests are passing. Feel free to add: Tested-by: Kevin Hilman <khilman@ti.com> Kevin
On Thursday 13 September 2012 03:57 AM, Kevin Hilman wrote: > Shubhrajyoti D <shubhrajyoti@ti.com> writes: > > [...] > >> This is the cleanup only series. >> >> Tested on omap4sdp and 3430sdp. > It would be extremely helpful if you would describe how this was tested. > And for me, it would be a source of extreme joy if you described any PM > testing. > > I gave this some additional PM testing on 3430/n900, 3530/Overo, > 3730/OveroSTORM, 3730/Beagle-xM and 4430/Panda. > > I tested v3.6-rc5 which passed all PM tests and then added this series > (by merging the i2c-embedded/i2c-next branch.) PM tests then fail. > > At least on 3530/Overo and 3730/Overo, CORE no longer hits retenion (or > off) during idle. > > The easy way to notice this is to see that even when no i2c transactions > are happening, the runtime PM status for omap_i2c.1 is remains 'active': > > # cat omap_i2c.*/power/runtime_status > active > suspended > > Of course that means that clocks are never gated during idle, and CORE > will never hit retention. > > I noticed it because my PM tests detected that the CORE powerdomain was > not transitioning to retention (or off) during idle. > > To reproduce, simply enable UART timeouts so CORE will hit retention: > > echo 3000 > /sys/devices/platform/omap_uart.0/power/autosuspend_delay_ms > echo 3000 > /sys/devices/platform/omap_uart.1/power/autosuspend_delay_ms > echo 3000 > /sys/devices/platform/omap_uart.2/power/autosuspend_delay_ms > echo 3000 > /sys/devices/platform/omap_uart.3/power/autosuspend_delay_ms > > and check the core_pwrm state counters: > > cat /debug/pm_debug/count > > wait > 3 seconds for the UART autosuspend timers to kick in. At this > point, CORE should be transitioning to retenion in idle (determined by > noticing that the RET counter for core_pwrdm is increasing.) > > With $SUBJECT series applied, you'll notice that CORE is not > transitioning. However I do not see the issue,let me know if I missed something see below. On my 3430sdp /sys/bus/platform/devices # cat omap_i2c.*/power/runtime_status suspended suspended suspended /sys/bus/platform/devices # /sys/bus/platform/devices # cat omap_i2c.*/power/autosuspend_delay_ms 1000 1000 1000 /sys/bus/platform/devices # echo 3000 > /sys/devices/platform/omap_uart.0/power/ autosuspend_delay_ms /sys/bus/platform/devices # echo 3000 > /sys/devices/platform/omap_uart.1/power/ autosuspend_delay_ms /sys/bus/platform/devices # echo 3000 > /sys/devices/platform/omap_uart.2/power/ autosuspend_delay_ms /sys/bus/platform/devices # /sys/bus/platform/devices # cat /debug/pm_debug/count usbhost_pwrdm (ON),OFF:0,RET:1373,INA:0,ON:1374,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 sgx_pwrdm (OFF),OFF:1,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 core_pwrdm (ON),OFF:0,RET:1,INA:0,ON:2,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0 per_pwrdm (ON),OFF:0,RET:79,INA:0,ON:80,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 dss_pwrdm (ON),OFF:0,RET:1373,INA:0,ON:1374,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 cam_pwrdm (RET),OFF:0,RET:1,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 neon_pwrdm (ON),OFF:0,RET:1373,INA:0,ON:1374,RET-LOGIC-OFF:0 mpu_pwrdm (ON),OFF:0,RET:1373,INA:0,ON:1374,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 iva2_pwrdm (RET),OFF:0,RET:1,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0,RET-MEMBANK3-OFF:0,RE0 usbhost_clkdm->usbhost_pwrdm (1) sgx_clkdm->sgx_pwrdm (0) per_clkdm->per_pwrdm (13) cam_clkdm->cam_pwrdm (0) dss_clkdm->dss_pwrdm (1) d2d_clkdm->core_pwrdm (0) iva2_clkdm->iva2_pwrdm (0) mpu_clkdm->mpu_pwrdm (0) core_l4_clkdm->core_pwrdm (23) core_l3_clkdm->core_pwrdm (4) neon_clkdm->neon_pwrdm (0) /sys/bus/platform/devices # /sys/bus/platform/devices # /sys/bus/platform/devices # sleep 5 /sys/bus/platform/devices # cat /debug/pm_debug/count usbhost_pwrdm (ON),OFF:0,RET:1512,INA:0,ON:1513,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 sgx_pwrdm (OFF),OFF:1,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 core_pwrdm (ON),OFF:0,RET:12,INA:0,ON:13,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0 per_pwrdm (ON),OFF:0,RET:218,INA:0,ON:219,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 dss_pwrdm (ON),OFF:0,RET:1512,INA:0,ON:1513,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 cam_pwrdm (RET),OFF:0,RET:1,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 neon_pwrdm (ON),OFF:0,RET:1512,INA:0,ON:1513,RET-LOGIC-OFF:0 mpu_pwrdm (ON),OFF:0,RET:1512,INA:0,ON:1513,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 iva2_pwrdm (RET),OFF:0,RET:1,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0,RET-MEMBANK3-OFF:0,RE0 usbhost_clkdm->usbhost_pwrdm (1) sgx_clkdm->sgx_pwrdm (0) per_clkdm->per_pwrdm (13) cam_clkdm->cam_pwrdm (0) dss_clkdm->dss_pwrdm (1) d2d_clkdm->core_pwrdm (0) iva2_clkdm->iva2_pwrdm (0) mpu_clkdm->mpu_pwrdm (0) core_l4_clkdm->core_pwrdm (23) core_l3_clkdm->core_pwrdm (4) neon_clkdm->neon_pwrdm (0) /sys/bus/platform/devices # cat omap_i2c.*/power/runtime_status suspended suspended suspended /sys/bus/platform/devices # > > Kevin > >> The following changes since commit 55d512e245bc7699a8800e23df1a24195dd08217: >> >> Linux 3.6-rc5 (2012-09-08 16:43:45 -0700) >> >> are available in the git repository at: >> git://gitorious.org/linus-tree/linus-tree.git for_3.7/i2c/big_cleanups >> >> >> Felipe Balbi (22): >> i2c: omap: switch to devm_* API >> i2c: omap: simplify num_bytes handling >> i2c: omap: decrease indentation level on data handling >> i2c: omap: add blank lines >> i2c: omap: simplify omap_i2c_ack_stat() >> i2c: omap: split out [XR]DR and [XR]RDY >> i2c: omap: improve i462 errata handling >> i2c: omap: re-factor receive/transmit data loop >> i2c: omap: switch over to do {} while loop >> i2c: omap: ack IRQ in parts >> i2c: omap: switch to platform_get_irq() >> i2c: omap: bus: add a receiver flag >> i2c: omap: simplify errata check >> i2c: omap: always return IRQ_HANDLED >> i2c: omap: simplify IRQ exit path >> i2c: omap: resize fifos before each message >> i2c: omap: get rid of the "complete" label >> i2c: omap: always return IRQ_HANDLED >> i2c: omap: switch to threaded IRQ support >> i2c: omap: remove unnecessary pm_runtime_suspended check >> i2c: omap: switch over to autosuspend API >> i2c: omap: sanitize exit path >> >> Shubhrajyoti D (1): >> i2c: omap: remove redundant status read >> >> drivers/i2c/busses/i2c-omap.c | 442 +++++++++++++++++++++++++---------------- >> 1 files changed, 271 insertions(+), 171 deletions(-)
Hi, On Thu, Sep 13, 2012 at 11:34:48AM +0530, Shubhrajyoti wrote: > On Thursday 13 September 2012 03:57 AM, Kevin Hilman wrote: > > Shubhrajyoti D <shubhrajyoti@ti.com> writes: > > > > [...] > > > >> This is the cleanup only series. > >> > >> Tested on omap4sdp and 3430sdp. > > It would be extremely helpful if you would describe how this was tested. > > And for me, it would be a source of extreme joy if you described any PM > > testing. > > > > I gave this some additional PM testing on 3430/n900, 3530/Overo, > > 3730/OveroSTORM, 3730/Beagle-xM and 4430/Panda. > > > > I tested v3.6-rc5 which passed all PM tests and then added this series > > (by merging the i2c-embedded/i2c-next branch.) PM tests then fail. > > > > At least on 3530/Overo and 3730/Overo, CORE no longer hits retenion (or > > off) during idle. > > > > The easy way to notice this is to see that even when no i2c transactions > > are happening, the runtime PM status for omap_i2c.1 is remains 'active': > > > > # cat omap_i2c.*/power/runtime_status > > active > > suspended > > > > Of course that means that clocks are never gated during idle, and CORE > > will never hit retention. > > > > I noticed it because my PM tests detected that the CORE powerdomain was > > not transitioning to retention (or off) during idle. > > > > To reproduce, simply enable UART timeouts so CORE will hit retention: > > > > echo 3000 > /sys/devices/platform/omap_uart.0/power/autosuspend_delay_ms > > echo 3000 > /sys/devices/platform/omap_uart.1/power/autosuspend_delay_ms > > echo 3000 > /sys/devices/platform/omap_uart.2/power/autosuspend_delay_ms > > echo 3000 > /sys/devices/platform/omap_uart.3/power/autosuspend_delay_ms > > > > and check the core_pwrm state counters: > > > > cat /debug/pm_debug/count > > > > wait > 3 seconds for the UART autosuspend timers to kick in. At this > > point, CORE should be transitioning to retenion in idle (determined by > > noticing that the RET counter for core_pwrdm is increasing.) > > > > With $SUBJECT series applied, you'll notice that CORE is not > > transitioning. > However I do not see the issue,let me know if I missed something see below. > > On my 3430sdp > > /sys/bus/platform/devices # cat omap_i2c.*/power/runtime_status > suspended > suspended > suspended > /sys/bus/platform/devices # > /sys/bus/platform/devices # cat omap_i2c.*/power/autosuspend_delay_ms > 1000 > 1000 > 1000 I just revived my very, very old 3503 overo and while i2c isn't active, I don't see transitions on core power domain: # mount -t proc none /proc # mount -t sysfs none /sys # mount -t debugfs none /debug # echo 3000 > /sys/devices/platform/omap_uart.0/power/autosuspend_delay_ms # echo 3000 > /sys/devices/platform/omap_uart.1/power/autosuspend_delay_ms # echo 3000 > /sys/devices/platform/omap_uart.2/power/autosuspend_delay_ms # cat /sys/devices/platform/omap_i2c.*/power/runtime_status suspended suspended # grep ^core_pwrdm /debug/pm_debug/count && sleep 5 && grep ^core_pwrdm /debug/pm_debug/count core_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0 core_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0 Weird, but I don't think i2c is to blame here (unless I'm missing something). I can see that after i2c transfers, my suspend function is called and right after that i2c is suspended: # cat /sys/class/regulator/regulator.*/status && sleep 1 && cat /sys/devices/platform/omap_i2c.*/power/runtime_status [ 411.072998] omap_i2c omap_i2c.1: omap_i2c_runtime_resume normal normal normal [ 412.075347] omap_i2c omap_i2c.1: omap_i2c_runtime_suspend suspended suspended What am I missing to get any pm transitions to happen with my overo ? # cat /debug/pm_debug/count && sleep 5 && cat /debug/pm_debug/count usbhost_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 sgx_pwrdm (OFF),OFF:1,RET:0,INA:0,ON:0,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 core_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0 per_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 dss_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 cam_pwrdm (RET),OFF:0,RET:1,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 neon_pwrdm (ON),OFF:0,RET:2379,INA:0,ON:2380,RET-LOGIC-OFF:0 mpu_pwrdm (ON),OFF:0,RET:2377,INA:0,ON:2378,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 iva2_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0,RET-MEMBANK3-OFF:0,RET-MEMBANK4-OFF:0 usbhost_clkdm->usbhost_pwrdm (1) sgx_clkdm->sgx_pwrdm (0) per_clkdm->per_pwrdm (19) cam_clkdm->cam_pwrdm (0) dss_clkdm->dss_pwrdm (1) d2d_clkdm->core_pwrdm (0) iva2_clkdm->iva2_pwrdm (0) mpu_clkdm->mpu_pwrdm (0) core_l4_clkdm->core_pwrdm (21) core_l3_clkdm->core_pwrdm (4) neon_clkdm->neon_pwrdm (0) usbhost_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 sgx_pwrdm (OFF),OFF:1,RET:0,INA:0,ON:0,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 core_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0 per_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 dss_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 cam_pwrdm (RET),OFF:0,RET:1,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 neon_pwrdm (ON),OFF:0,RET:2442,INA:0,ON:2443,RET-LOGIC-OFF:0 mpu_pwrdm (ON),OFF:0,RET:2440,INA:0,ON:2441,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0 iva2_pwrdm (ON),OFF:0,RET:0,INA:0,ON:1,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0,RET-MEMBANK3-OFF:0,RET-MEMBANK4-OFF:0 usbhost_clkdm->usbhost_pwrdm (1) sgx_clkdm->sgx_pwrdm (0) per_clkdm->per_pwrdm (19) cam_clkdm->cam_pwrdm (0) dss_clkdm->dss_pwrdm (1) d2d_clkdm->core_pwrdm (0) iva2_clkdm->iva2_pwrdm (0) mpu_clkdm->mpu_pwrdm (0) core_l4_clkdm->core_pwrdm (21) core_l3_clkdm->core_pwrdm (4) neon_clkdm->neon_pwrdm (0)
On Thursday 13 September 2012 11:34 AM, Shubhrajyoti wrote: > > At least on 3530/Overo and 3730/Overo, CORE no longer hits retenion (or > > off) during idle. I dont have an Overo, anyways will see if I can get one However on beagleXm even off count increases / # grep "^core_pwrdm" /debug/pm_debug/count core_pwrdm (ON),OFF:48,RET:856,INA:0,ON:905,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0 / # cat /sys/devices/platform/omap_i2c.*/power/runtime_status suspended suspended / # / # / # cat /sys/devices/platform/omap_i2c.*/power/autosuspend_delay_ms 1000 1000 / # / # / # grep "^core_pwrdm" /debug/pm_debug/count core_pwrdm (ON),OFF:218,RET:856,INA:0,ON:1075,RET-LOGIC-OFF:0,RET-MEMBANK1-OFF:0,RET-MEMBANK2-OFF:0 / #
Hi Kevin, > Since this series is already merged, I suggest that the problem patch be > reverted, at least for v3.7 and until the problem is better understood > and tested. I thought I'll give you guys some more days to fix the problem before reverting. > > With that patch reverted, all my PM tests are passing. Feel free to > add: > > Tested-by: Kevin Hilman <khilman@ti.com> Thanks. That indeed increases confidence in this series. I won't add the tags to the tree, though, since that would mean rebasing. All the best, Wolfram
Felipe Balbi <balbi@ti.com> writes: [...] > I just ran same tests on pandaboard and i2c is suspended actually, > though no power domain transitions to RET. Do we not have retention > while idle on pandaboard ? Not for CORE. Only CPUx and MPU domains will be transitioning on OMAP4, and then, only with CPUidle enabled. Until we have CORE retention support in mainline (we're almost there) OMAP4 is not a useful test platform for device PM. Kevin
Kevin Hilman <khilman@deeprootsystems.com> writes: > Kevin Hilman <khilman@deeprootsystems.com> writes: > [...] >> Sorry to be late to the party (again), but still catching up after some >> time off. >> >> Unfortunately, this series causes PM regressions on several OMAP >> platforms. I hope we can hold off on this until those issues are >> addressed. > > I tracked the regression down to [PATCHv8 21/22] (see reply there.) > > Since this series is already merged, I suggest that the problem patch be > reverted, at least for v3.7 and until the problem is better understood > and tested. > > With that patch reverted, all my PM tests are passing. Feel free to > add: OK, the i2c series is off the hook. Felipe and I spent a little time tracking this down. Felipe suggested that there might be a driver with periodic i2c activity keeping I2C awake, and thus preventing CORE retention. He was right. On the boards where I'm seeing the failure, the RTC is firing every second, and since the RTC is on the I2C connected PMIC, the PMIC IRQ reads and the RTC reads are causing lots of I2C activity every second. With the new autosuspend feature, that is enough to keep the I2C active continually and prevent CORE retention. So, all that to say, from my PoV, this series can go in as is. The PM problem is caused by the RTC driver someplace. Thanks Felipe, Kevin
Hi, On Thu, Sep 13, 2012 at 11:04:42AM -0700, Kevin Hilman wrote: > Kevin Hilman <khilman@deeprootsystems.com> writes: > > > Kevin Hilman <khilman@deeprootsystems.com> writes: > > > [...] > > >> Sorry to be late to the party (again), but still catching up after some > >> time off. > >> > >> Unfortunately, this series causes PM regressions on several OMAP > >> platforms. I hope we can hold off on this until those issues are > >> addressed. > > > > I tracked the regression down to [PATCHv8 21/22] (see reply there.) > > > > Since this series is already merged, I suggest that the problem patch be > > reverted, at least for v3.7 and until the problem is better understood > > and tested. > > > > With that patch reverted, all my PM tests are passing. Feel free to > > add: > > OK, the i2c series is off the hook. > > Felipe and I spent a little time tracking this down. Felipe suggested > that there might be a driver with periodic i2c activity keeping I2C > awake, and thus preventing CORE retention. He was right. FYI, the original idea came from Shubhro. We agreed that would be the only way i2c would be prevented from idling.
Felipe Balbi <balbi@ti.com> writes: > Hi, > > On Thu, Sep 13, 2012 at 11:04:42AM -0700, Kevin Hilman wrote: >> Kevin Hilman <khilman@deeprootsystems.com> writes: >> >> > Kevin Hilman <khilman@deeprootsystems.com> writes: >> > >> [...] >> >> >> Sorry to be late to the party (again), but still catching up after some >> >> time off. >> >> >> >> Unfortunately, this series causes PM regressions on several OMAP >> >> platforms. I hope we can hold off on this until those issues are >> >> addressed. >> > >> > I tracked the regression down to [PATCHv8 21/22] (see reply there.) >> > >> > Since this series is already merged, I suggest that the problem patch be >> > reverted, at least for v3.7 and until the problem is better understood >> > and tested. >> > >> > With that patch reverted, all my PM tests are passing. Feel free to >> > add: >> >> OK, the i2c series is off the hook. >> >> Felipe and I spent a little time tracking this down. Felipe suggested >> that there might be a driver with periodic i2c activity keeping I2C >> awake, and thus preventing CORE retention. He was right. > > FYI, the original idea came from Shubhro. We agreed that would be the > only way i2c would be prevented from idling. Great, thanks Shubhro! Also, FYI, I just submitted a patch to the TWL RTC driver which was the source of all the I2C activity since it's on the I2C-connected PMIC. Thanks for the help and suggestions, Kevin [1] https://groups.google.com/forum/#!topic/rtc-linux/sFbYmAzCRLQ
On Friday 14 September 2012 02:58 AM, Kevin Hilman wrote: > Felipe Balbi <balbi@ti.com> writes: > >> Hi, >> >> On Thu, Sep 13, 2012 at 11:04:42AM -0700, Kevin Hilman wrote: >>> Kevin Hilman <khilman@deeprootsystems.com> writes: >>> >>>> Kevin Hilman <khilman@deeprootsystems.com> writes: >>>> >>> [...] >>> >>>>> Sorry to be late to the party (again), but still catching up after some >>>>> time off. >>>>> >>>>> Unfortunately, this series causes PM regressions on several OMAP >>>>> platforms. I hope we can hold off on this until those issues are >>>>> addressed. >>>> I tracked the regression down to [PATCHv8 21/22] (see reply there.) >>>> >>>> Since this series is already merged, I suggest that the problem patch be >>>> reverted, at least for v3.7 and until the problem is better understood >>>> and tested. >>>> >>>> With that patch reverted, all my PM tests are passing. Feel free to >>>> add: >>> OK, the i2c series is off the hook. >>> >>> Felipe and I spent a little time tracking this down. Felipe suggested >>> that there might be a driver with periodic i2c activity keeping I2C >>> awake, and thus preventing CORE retention. He was right. >> FYI, the original idea came from Shubhro. We agreed that would be the >> only way i2c would be prevented from idling. > Great, thanks Shubhro! > > Also, FYI, I just submitted a patch to the TWL RTC driver which was the > source of all the I2C activity since it's on the I2C-connected PMIC. > > Thanks for the help and suggestions, > > Kevin > > [1] https://groups.google.com/forum/#!topic/rtc-linux/sFbYmAzCRLQ Thanks for the testing and the patch.
Changes since v1: - removed tabification on patch 6/17 - removed dev_err() which was introduced on patch 09/17 Changes since v2: - do not set full fifo depth in the RDR interrupt. - some changelog updates. - rebase to the Wolfram's tree. Changes since v3: - Remove a redundant read of status register - Read the dev->buf_len variable instead of the register as the information of the remaining bytes is there. Changes since v4: - Ack the arbitration lost. - Rebase to the i2c-embedded/for-next branch. Changes since v5: - Rebase to latest mainline - Added some more cleanup patches so as have a consolidated series. Changes since v6: - Fix comments on setting the pdev to NULL. - Trivial changelog update Changes since v7: - Remove a patch as the code is getting changed anyways - Changelogs update. Previous discussions can be found here http://www.spinics.net/lists/linux-i2c/msg09764.html This is the cleanup only series. Tested on omap4sdp and 3430sdp. The following changes since commit 55d512e245bc7699a8800e23df1a24195dd08217: Linux 3.6-rc5 (2012-09-08 16:43:45 -0700) are available in the git repository at: git://gitorious.org/linus-tree/linus-tree.git for_3.7/i2c/big_cleanups Felipe Balbi (22): i2c: omap: switch to devm_* API i2c: omap: simplify num_bytes handling i2c: omap: decrease indentation level on data handling i2c: omap: add blank lines i2c: omap: simplify omap_i2c_ack_stat() i2c: omap: split out [XR]DR and [XR]RDY i2c: omap: improve i462 errata handling i2c: omap: re-factor receive/transmit data loop i2c: omap: switch over to do {} while loop i2c: omap: ack IRQ in parts i2c: omap: switch to platform_get_irq() i2c: omap: bus: add a receiver flag i2c: omap: simplify errata check i2c: omap: always return IRQ_HANDLED i2c: omap: simplify IRQ exit path i2c: omap: resize fifos before each message i2c: omap: get rid of the "complete" label i2c: omap: always return IRQ_HANDLED i2c: omap: switch to threaded IRQ support i2c: omap: remove unnecessary pm_runtime_suspended check i2c: omap: switch over to autosuspend API i2c: omap: sanitize exit path Shubhrajyoti D (1): i2c: omap: remove redundant status read drivers/i2c/busses/i2c-omap.c | 442 +++++++++++++++++++++++++---------------- 1 files changed, 271 insertions(+), 171 deletions(-)