On 10/06/2015 12:14 PM, Felipe Balbi wrote: > > Hi, > > Suman Anna <s-anna@xxxxxx> writes: >>>>>> 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. > > no doubt this needs a solution, but seesm like a solution here will have > to be incremental. See new revision of my series. I'm now skipping 32k > only and keeping it enabled so drivers/clocksource/ can use it. Yeah, have noticed, and it's better for the current problem at hand than this patch. > >>>>>>> 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. > > I still need omap_hwmod_enable() to be called, that's why it's still > used. I don't think replacing it with of_find_compatible_node() will buy > us much, we will still need to check for of_device_is_available() > anyway. Sure we skip all the timer flags (alwon, dsp, pwm, secure), but > that really isn't big deal. Yeah, I would think the counter_32k and the timer code will have to be decoupled in the future anyway, so it is best to localize the change now rather than later, so that the omap2_sync32k_clocksource_init() function can be decoupled cleanly from the timr code. The check for of_device_is_available may be moot if this device is always needed. But as long as current timer functionality is unchanged, I am not too particular about this. If it is always needed, the check for of_device regards Suman > When converting gptimer to clocksource, then I'll look at what I can do > for the timer request side of things. For now, things are working, > apparently without regressions (or at least I couldn't catch any so far) > and it seems clean enough that it could possibly be queued for v4.4 > merge window. For v4.5 I'll look at this again and try to figure out how > to handle gptimer. -- 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
