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

Nishanth Menon
2010-01-18T13:47:23Z

Alexander Shishkin said the following on 01/18/2010 04:46 AM:
> On Wed, Jan 13, 2010 at 06:58:28 -0600, Nishanth Menon wrote:
>> Alexander Shishkin said the following on 01/13/2010 05:36 AM:
>>> On Tue, Jan 12, 2010 at 04:08:23 -0600, Nishanth Menon wrote:
>>>> 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
>>>> counter: having a disabled defconfig allows relevant folks to
>>>> enable on a need basis
>>>> counter to counter: what do you do when a user reports
>>>> an issue in a release and you'd want to debug it with
>>>> ETM on his platform other than doing a rebuild?
>>> Well, my intention is to have it enabled for most of the cases only having
>>> it disabled for testing purposes.
>> with a sysfs you can go either way, with proper #ifdeferry, you can
>> get the best of all worlds I guess.. I know in one of the products,
>> a similar patch was not taken in due to introduction of additional
>> scratchpad space and latencies - so there are folks who would like
>> this and those who would like to see this not present in the binary
>> they flash to thier device.
>
> What would you suggest for a place in sysfs for such a file? I'm thinking
> /sys/power.
I would have imagined this is a perfect candidate for debugfs?
Regards,
Nishanth Menon