Re: [PATCHv8 00/23]I2C big cleanup
On Friday 14 September 2012 02:58 AM, Kevin Hilman wrote: > Felipe Balbi writes: > >> Hi, >> >> On Thu, Sep 13, 2012 at 11:04:42AM -0700, Kevin Hilman wrote: >>> Kevin Hilman writes: >>> Kevin Hilman 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. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv8 00/23]I2C big cleanup
Felipe Balbi writes: > Hi, > > On Thu, Sep 13, 2012 at 11:04:42AM -0700, Kevin Hilman wrote: >> Kevin Hilman writes: >> >> > Kevin Hilman 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv8 00/23]I2C big cleanup
Hi, On Thu, Sep 13, 2012 at 11:04:42AM -0700, Kevin Hilman wrote: > Kevin Hilman writes: > > > Kevin Hilman 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. -- balbi signature.asc Description: Digital signature
Re: [PATCHv8 00/23]I2C big cleanup
Kevin Hilman writes: > Kevin Hilman 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv8 00/23]I2C big cleanup
Felipe Balbi 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv8 00/23]I2C big cleanup
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 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 -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
Re: [PATCHv8 00/23]I2C big cleanup
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 / # -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv8 00/23]I2C big cleanup
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 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
Re: [PATCHv8 00/23]I2C big cleanup
On Thursday 13 September 2012 03:57 AM, Kevin Hilman wrote: > Shubhrajyoti D 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 55d512e245bc7699a8800e23df1a2
Re: [PATCHv8 00/23]I2C big cleanup
On Wed, Sep 12, 2012 at 03:27:22PM -0700, Kevin Hilman wrote: > Shubhrajyoti D 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.) 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 ? See below for a few results... First I ran this script to set everything up: #!/bin/sh echo "mounting proc" mount -t proc none /proc echo "mounting sysfs" mount -t sysfs none /sys echo "mounting debugfs" mount -t debugfs none /debug echo "UART autosuspend delay" 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 echo "check omap_i2c.* status" cat /sys/devices/platform/omap_i2c.*/power/runtime_status echo "check CORE pm counters" grep "^core_pwrdm" /debug/pm_debug/count Then I triggered a few i2c transfers by reading regulator status through sysfs: # cat /sys/class/regulator/regulator.*/status [ 507.469299] omap_i2c omap_i2c.1: omap_i2c_runtime_resume normal normal off off normal normal normal normal normal off # [ 509.010711] omap_i2c omap_i2c.1: omap_i2c_runtime_suspend cat /sys/devices/platform/omap_i2c.*/power/runtime_status suspended suspended suspended suspended notice the prints I've added... I don't know what could go wrong only on Overo that i2c doesn't suspend. Can you add this debugging patch just for me to see if, at least, omap_i2c_runtime_suspend is being called ? diff --git a/drivers/i2c/busses/i2c-omap.c b/drivers/i2c/busses/i2c-omap.c index c78431a..ac61664 100644 --- a/drivers/i2c/busses/i2c-omap.c +++ b/drivers/i2c/busses/i2c-omap.c @@ -1238,6 +1238,8 @@ static int omap_i2c_runtime_suspend(struct device *dev) struct omap_i2c_dev *_dev = platform_get_drvdata(pdev); u16 iv; + dev_info(dev, "%s\n", __func__); + _dev->iestate = omap_i2c_read_reg(_dev, OMAP_I2C_IE_REG); omap_i2c_write_reg(_dev, OMAP_I2C_IE_REG, 0); @@ -1277,6 +1279,8 @@ static int omap_i2c_runtime_resume(struct device *dev) if (_dev->iestate) omap_i2c_write_reg(_dev, OMAP_I2C_IE_REG, _dev->iestate); + dev_info(dev, "%s\n", __func__); + return 0; } #endif /* CONFIG_PM_RUNTIME */ this is on top of Wolfram's for-next branch, btw. Maybe you should also add: CONFIG_I2C_DEBUG_CORE=y CONFIG_I2C_DEBUG_ALGO=y CONFIG_I2C_DEBUG_BUS=y to your .config just in case. cheers -- balbi signature.asc Description: Digital signature
Re: [PATCHv8 00/23]I2C big cleanup
Kevin Hilman writes: > Wolfram Sang 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 Kevin -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv8 00/23]I2C big cleanup
Wolfram Sang 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 -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv8 00/23]I2C big cleanup
Shubhrajyoti D 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(-)
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv8 00/23]I2C big cleanup
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. -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCHv8 00/23]I2C big cleanup
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! -- Pengutronix e.K. | Wolfram Sang| Industrial Linux Solutions | http://www.pengutronix.de/ | signature.asc Description: Digital signature
