Re: [PATCH 0/2] omap_wdt: fix interface clock handling

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Kalle,

On 3/9/2011 10:02 AM, kalle.jokiniemi@xxxxxxxxx wrote:

[mailto:paul@xxxxxxxxx] Sent: 9. maaliskuuta 2011 1:11 To:
Jokiniemi Kalle (Nokia-MS/Tampere) Cc: Kevin Hilman;
linux-omap@xxxxxxxxxxxxxxx; tony@xxxxxxxxxxx; b- cousson@xxxxxx;
Koskinen Ilkka (Nokia-MS/Tampere); Charulatha Varadarajan;
wim@xxxxxxxxx Subject: Re: [PATCH 0/2] omap_wdt: fix interface
clock handling

Hi

On Tue, 8 Mar 2011, Paul Walmsley wrote:

On Tue, 8 Mar 2011, Kevin Hilman wrote:

Kalle Jokiniemi<kalle.jokiniemi@xxxxxxxxx>  writes:

The runtime PM does not offer enough granularity to control
individual clocks separately as needed. Current pm
implementation of omap_wdt blocks the CORE power domain from
idling in OMAP SoC, as it keeps the interface clock of the
watchdog always on. This causes severe power leakage in
devices where the omap watchdog is in active use.

The iclk remains active because it's in hardware-controlled
auto-idle mode.

... and we think that the interface clock never enters auto-idle
because the WDT doesn't SIdleAck to the PRCM until the special
sequence has been written to the WDT registers to disable it.  So
even if its *CLKEN bits are set to zero, the WDT prevents the
system from going
idle.

Actually, thinking about this further, I think this statement must
be wrong. The WDT may actually SIdleAck even when it's enabled.
Otherwise, how could disabling the clocks save much power?  The WDT
shouldn't need its interface clock to be enabled in order to keep
its timer ticking or to reset the system.

Yes, the interface clock is not needed for the WDTIMER to work, and
disabling the interface clock allows the sleeping to happen. This is
what I observed. To my understanding, the pm_runtime calls
automatically disable also the functional clock. I know the WDTIMER
won't stop by just disabling the fclk bit, but it does not seem
correct operation to assume this. This is why I reverted the
runtime_pm patches. Can we somehow control only the iclk via
runtime-pm?


So this first patch might not even be necessary.

Kalle, if both patches work, perhaps you might try dropping this
one and see if the problem is still there?

Ok, I tried them both: first both individually, and then combined. No
luck, CORE power domain still remains ON with all combinations.

Looking at the registers dump, the clock bits are disabled as one
would expect, however this does not seem to help in getting CORE to
sleep. Actually, with the working clk_enable/disable approach, both
fclk and iclk clocks are shown as enabled, though I guess this is due
to the clk_disable(iclk) only allowing HW power domain state
transition to disable the interface clock (not to disable it
immediately). Maybe the transition goes somehow sour with r-pm, as
both clocks seem disabled, while the wdt is internally still ticking
(due to the special disable sequence not done, and it should not be
done, as we need the watchdog).

This gets us back to square one, how to use the runtime-pm to only
disable the iclk (or more precisely allow the device to sleep, when
wdt is used)? Is it out of the question to use direct clock control
via clock framework? It "just works" :)

In theory, you should not have to disable the interface clock at all.
Assuming that the smartidle is working properly, the iclk should be gated during clock domain transition automagically. The only thing that can prevent that is potentially a wrong setting in the clockactivity bits? It should be 0x2, meaning fclk is active but iclk can be gated.

Regards,
Benoit
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Arm (vger)]     [ARM Kernel]     [ARM MSM]     [Linux Tegra]     [Linux WPAN Networking]     [Linux Wireless Networking]     [Maemo Users]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux