Re: [PATCH 1/1] Clocksource: Add sched_clock to Atmel TCB clocksource

[Carsten Emde <C.Emde@osadl.org>]

Hi Thomas,

>>>>    static void __iomem *tcaddr;
>>>> +static cycle_t (*do_sched_clock_get_cycles)(struct clocksource *cs);
>>>
>>> what's that for? Indirection for indirection sake?
>> The code apparently provides two different functions to read the current clock
>> - depending on whether the clock is setup in single-channel mode or not.
>>
>> The default function is predefined in the struct clocksource clksrc:
>>    .read = tc_get_cycles
>> This structure element may later on be overwritten, if the counter is
>> configured in single-channel mode:
>>    clksrc.read = tc_get_cycles32;
>>
>> This is why my code sets do_sched_clock_get_cycles to clksrc.read before
>> sched_clock is registered to make sure the appropriate read function will be
>> used.
>    return clksrc.read(NULL);
> then?
Much better, of course, V2 will have it.

>>>> +	local_irq_save(flags);
>>> What's the purpose of this?
>> The sched_clock_register() function contains
>>    WARN_ON(!irqs_disabled());
>> which I did not want to trigger.
> That code is run in the early setup with interrupts disabled unless
> I'm missing something ...
Maybe, something else is wrong, and interrupts are enabled too early on 
this system. I added the lines around sched_clock_register(), because 
WARN_ON triggered. Will investigate further.

Thanks,
	-Carsten.