On 21/06/2019 12:34, Claudiu.Beznea@xxxxxxxxxxxxx wrote: > Hi Daniel, > > On 20.06.2019 11:53, Daniel Lezcano wrote: >> Hi Claudiu, >> >> sorry for the late reply. > > No problem, I understand. > >> >> >> On 13/06/2019 16:12, Claudiu.Beznea@xxxxxxxxxxxxx wrote: >>> Hi Daniel, >>> >>> On 31.05.2019 13:41, Daniel Lezcano wrote: >>>> >>>> Hi Claudiu, >>>> >>>> >>>> On 30/05/2019 09:46, Claudiu.Beznea@xxxxxxxxxxxxx wrote: >>>>> Hi Daniel, >>>>> >>>>> Taking into account the discussion on this tread and the fact that we have >>>>> no answer from Rob on this topic (I'm talking about [1]), what do you think >>>>> it would be best for this driver to be accepted the soonest? Would it be OK >>>>> for you to mimic the approach done by: >>>>> >>>>> drivers/clocksource/timer-integrator-ap.c >>>>> >>>>> with the following bindings in DT: >>>>> >>>>> aliases { >>>>> arm,timer-primary = &timer2; >>>>> arm,timer-secondary = &timer1; >>>>> }; >>>>> >>>>> also in PIT64B driver? >>>>> >>>>> Or do you think re-spinning the Alexandre's patches at [2] (which seems to >>>>> me like the generic way to do it) would be better? >>>> >>>> This hardware / OS connection problem is getting really annoying for >>>> everyone and this pattern is repeating itself since several years. It is >>>> time we fix it properly. >>>> >>>> The first solution looks hackish from my POV. The second approach looks >>>> nicer and generic as you say. So I would vote for [2] >>>> flagging approach proposed by Mark [3]. >>> >>> With this flagging approach this would mean a kind unification of >>> clocksource and clockevent functionalities under a single one, right? So >>> that the driver would register to the above layers only one device w/ 2 >>> functionalities (clocksource and clockevent)? Please correct me if I'm >>> wrong? If so, from my point of view this would require major re-working of >>> clocksource and clockevent subsystems. Correctly if I wrongly understood, >>> please. >> >> Well, actually I was not expecting to change all the framework but just >> pass a flag to the probe function telling if the node is for a >> clocksource, a clockevent or both. >> > > Giving so, whit these proposals I'm thinking at having something like this, > using Alexandre's new macros from [2] and passing a bitmask to timer's > probing functions (in the above example adapted only for pit64b driver > introduced in this thread): Yes basically that is what I had in mind. Thanks for taking care of that. However after seeing the code I realize the impact is larger than expected as all the TIMER_OF_DECLARE will be impacted. AFAICT, this driver can be converted to the timer-of API. So I think it makes sense to keep this contained in the API. In the function timer_of_init(), we can add the parsing of the node and set the timer-of flags with: #define TIMER_OF_IS_CLOCKSOURCE 0x8 #define TIMER_OF_IS_CLOCKEVENT 0x10 In addition the API: int timer_of_is_clocksource(struct timer_of *to) { return (to & TIMER_OF_IS_CLOCKSOURCE); } int timer_of_is_clockevents(struct timer_of *to) { return (to & TIMER_OF_IS_CLOCKEVENT); } -- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog