Re: [PATCH] MIPS: add proper set_mode() to cevt-r4k

[John Crispin <john@phrozen.org>]

On 31/07/13 21:26, Florian Fainelli wrote:
> Le mercredi 31 juillet 2013 12:22:15 David Daney a écrit :
>> On 07/29/2013 04:14 AM, Florian Fainelli wrote:
>>> 2013/7/29 John Crispin<john@phrozen.org>:
>> [...]
>>
>>>>> It looks to me like you are moving the irq setup later just to ensure
>>>>> that your ralink clockevent device has been registered before and has
>>>>> set cp0_timer_irq_installed when the set_mode() r4k clockevent device
>>>>> runs, such that it won't register the same IRQ that your platforms
>>>>> uses. If that it the case, cannot you just ensure that you run your
>>>>> cevt device registration before mips_clockevent_init() is called?
>>>>
>>>> i dont like relying on the order in which the modules get loaded.
>>>
>>> plat_time_init() runs before mips_clockevent_init() and the ordering
>>> is explicit, would not that work for what you are trying to do?
>>>
>>>> the actual problem is not the irq sharing but that the cevt-r4k registers
>>>> the irq when the cevt is registered and not when it is activated. i
>>>> believe
>>>> that the patch fixes this problem
>>>
>>> Your patch certainly does what you say it does, but that is kind of an
>>> abuse of the set_mode() callback.
>>
>> I might as add my $0.02...
>>
>> There are many other clockevent drivers that do this type of thing
>> aren't there?  The clockevent framework uses this to
>> install/remove/switch drivers, so why should cevt-r4k not be made to
>> work like this?
>
> Whatever works for you. I still would like to understand why plat_time_init()
> is not suitable for John's specific use case.

Hi Florian,

the reason is that fixing it in plat_time_init() works around the real 
problem. the double request of the irq is a symptom of the actual 
problem, which is, that the cevt-r4k sets up the timer during init and 
not during setup. additionally, plat_time_init is used to probe the cevt 
drivers from OF already. currently the mips code just assumes that on a 
r4k we always have and want to run the cevt-r4k. this assumption is 
wrong and can quickly be fixed by making the cevt-r4k use the correct api.

also fixing it this way allows the user to control the clocksource and 
change it at runtime via sysfs, a feature als not working currently on 
r4k as the cevt driver did not implement the set_mode() handler 
correctly. to be quite honest, i cannot think of a single way in which 
this can be fixed cleanly in the ralink plat_time_init() without using 
some weird heuristic. also if i fix this inside ralink plat_time_init() 
it is fixed only on ralink SoC and not on any other platform.

	John