Re: [PATCH] save and restore etm state across core OFF modes

Alexander Shishkin
2010-05-01T17:34:36Z

On Tue, Jan 12, 2010 at 04:53:51 -0600, Nishanth Menon wrote:
> Tony Lindgren had written, on 01/12/2010 04:15 PM, the following:
> >* Nishanth Menon <nm@ti.com> [100112 14:06]:
> >>Alexander Shishkin had written, on 01/12/2010 03:46 PM, the following:
> >>>On Tue, Jan 12, 2010 at 01:04:04 -0800, Tony Lindgren wrote:
> >>>>* Nishanth Menon <nm@ti.com> [100112 09:31]:
> >>>>>Alexander Shishkin had written, on 01/12/2010 11:30 AM, the following:
> >>>>>>On Tue, Jan 12, 2010 at 11:13:13 -0600, Nishanth Menon wrote:
> >>>>>>>Alexander Shishkin had written, on 01/12/2010 11:04 AM, the following:
> >>>>>>>>diff --git a/arch/arm/mach-omap2/sleep34xx.S b/arch/arm/mach-omap2/sleep34xx.S
> >>>>>>>>index 69521be..0a5ec86 100644
> >>>>>>>>--- a/arch/arm/mach-omap2/sleep34xx.S
> >>>>>>>>+++ b/arch/arm/mach-omap2/sleep34xx.S
> >>>>>[...]
> >>>>>>>> /* Store current cpsr*/
> >>>>>>>> mrs r2, cpsr
> >>>>>>>> stmia r8!, {r2}
> >>>>>>>>@@ -520,6 +616,7 @@ clean_caches:
> >>>>>>>> cmp r9, #1 /* Check whether L2 inval is required or not*/
> >>>>>>>> bne skip_l2_inval
> >>>>>>>>clean_l2:
> >>>>>>>>+#if 0
> >>>>>>>my aversion to #if 0 kicks in here :(.. do we have an alternative
> >>>>>>>like using the CONFIG_ENABLE_OFF_MODE_JTAG_ETM_DEBUG or something
> >>>>>>>else?
> >>>>>>Fair enough. I could replace it with "#if !defined(...)" as the first
> >>>>>>thing that comes to mind. This way it will only take disabling the
> >>>>>>config option to catch any possible regressions in between. Does this
> >>>>>>sound reasonable?
> >>>>>sounds ok to me.. unless folks have ideas coz of clean_l2 label..
> >>>>>more comments might be useful before a rev2 of the patch..
> >>>>The best solution would be to be able to toggle this via sysfs or
> >>>>debugfs by swapping the sram code for idle loop when JTAG support
> >>>>is needed.
> >>>Well, if you say, compile the ETM driver in, this will be needed most of
> >>>the time.
> >>>
> >>I can think of reasons for an against a sysfs entry (as part of
> >>discussion -warning lot of self contradictions below- but I think
> >>might save a bit of back and froth ;)):
> >>
> >>for sysfs entry:
> >>a) save and restore will have additional latency when you save a
> >>chunk such as EMU domain regs - this will not be needed in
> >>production phones, disabling it might pop up surprises
> >
> >There's no overhead if you're just replacing the function
> >loaded to SRAM as needed. But for sure it's a debug tool only.
>
> I should probably have been more clear ->I agree function relocation
> to SRAM is not a major factor here, my concern was the additional
> latency incurred during scratchpad save and restore logic as seen by
> the patch:
> -u32 omap3_arm_context[128];
> +u32 omap3_arm_context[256];
> the arm context has doubled albiet 128bytes only.. it still changes

I've tried to address this and other concerns expressed in this thread
and I'll post a new patchset in a few minutes.

> the latencies involved on the save and restore paths.. few
> interesting behavior seen with EHCI save and restore comes to mind
> here - but maybe irrelevant to the discussion..

Regards,
--
Alex