Hi Felipe, >>>> >>>> On 10/05/2015 02:48 PM, Balbi, Felipe wrote: >>>>> We actually want these devices to be created because >>>>> we will be moving timers to drivers/clocksource and >>>>> this will prevent them from probing. >>>> >>>> This logic is also used to remove the secure timer from being >>>> registered to the kernel on HS devices, while allowing the timer to be >>>> available on GP devices. Your patch actually would break that >>>> functionality. I suggest that you look at the history of the code that >>> >>> heh, that's a silly way of doing so. Could just detect if we're running >>> on HS or not. >> >> This function is invoked only after detecting that we are running on a >> HS device. > > What actually disables the timer is omap_get_timer_dt() and that's > called in more than one place. Yes indeed, and this patch is changing the behavior of omap_get_timer_dt(), so you have to check all call-sites. Also, one another thing is that this trick was used for DT boots so that the timers used for clocksource and clockevent are eliminated from the omap_dm_timer_request() API. The legacy boot used a specific API to mark those as reserved so that the _request API does not give them out. Granted that it may not have been the best approach, but that needs a solution if you change the behavior of this API. > >>>> originally added this logic - this function seems to be designed to >>>> actually remove the node. The OMAP DMTimer provides an API to request >>>> timers, and I think this logic was used to eliminate giving out the >>>> timers used for clocksource and clockevent when the driver got adapted >>>> to DT. >>> >>> again not the best way of achieving what you want. In any case, other >>> than ir-rx51.c who's using that API ? Seems like we could just pass a >>> phandle to ir-rx51 and be done with it. >> >> We will gain a user from OMAP remoteproc driver as well (out of tree at >> the moment, but it does follow the DT phandle and >> omap_dm_timer_request_by_node semantics). I do not think ir-rx51.c is >> converted to DT, and also some of the API are alive just because of the >> continued OMAP3 legacy boot support. Guess, it is a just a question of >> not breaking existing API until we kill some of them. > > sure, but until remoteproc gets upstream, it's only 1 user ;-) > >>>>> Signed-off-by: Felipe Balbi <balbi@xxxxxx> >>>>> --- >>>>> >>>>> Tony, I wonder if you can get merged as a fix, or do you >>>>> prefer receiving it together with my timer series ? >>>> >>>> NAK for rc, as it breaks other stuff. >>> >>> a single stuff which is likely easy enough to fix. But fair enough >> >> Don't think this patch is fixing any critical bug either, better to club >> it with your cleanup series. > > it is kinda critical when you consider that the timer is disabled > outside of the soc type detection. This has been like this since DMTimer is converted to DT (from 3.8 kernel), and is a need for your counter32k clocksource series. The problem seems to stem from the reuse of omap_get_timer_dt for counter32k nodes. A better solution would be to remove the omap_get_timer_dt() from omap2_sync32k_clocksource_init() code and just replace it with of_find_compatible_node - you seem to be limiting the majority of that function to legacy mode in Patch 10 of your RFC series anyways. regards Suman -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
