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

[Nishanth Menon <nm@ti.com>]

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 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,
Nishanth Menon