Hi Tony,
Few replies inline below. Adding Suman as well who is planning to use dmtimer
directly for his work with DSP.
On 11/22/2013 09:33 AM, Tony Lindgren wrote:
> Hi,
>
> * Joel Fernandes <joelf@xxxxxx> [131121 18:00]:
>> Multiplatform support has made arch/arm/plat-omap/include/plat/ inaccessible to
>> drivers outside the plat-omap directory [1]. Due to this the following drivers
>> are disabled with !CONFIG_ARCH_MULTIPLATFORM:
>> CONFIG_IR_RX51 (drivers/media/rc/ir-rx51.c)
>> CONFIG_TIDSPBRIDGE (drivers/staging/tidspbridge/core/dsp-clock.c)
>>
>> We move the portion of the dmtimer "API" that should be accessible to the
>> drivers, into include/linux/omap-timer.h
>
> As we chatted earlier, we don't have to expose all these hardware specific
> functions and use existing Linux generic frameworks instead.
I thought over this, and strongly feel that in this case moving to a generic
framework is not the right approach. I mentioned reasons in last post, but
summarizing all of them below:
1. It is not immediately intuitive or obvious unlike GPIO that a timer can be an
irqchip.
2. There are only 1-2 users of the dmtimer directly so moving to linux generic
framework is bit overkill I feel. Further there is already a framework-
clockevents/hrtimer which everyone should be using. For the 1-2 direct dmtimer
users, they should just use the dmtimer public API.
More reasons below...
>
> We can implement an irqchip and a clocksource in the dmtimer code for the
> client drivers to use, and after that we only have a couple of dmtimer
> specific functions left to export.
>
> I'm thinkging some thing like this for the public API:
>
> omap_dm_timer_request request_irq
> omap_dm_timer_request_specific request_irq
> omap_dm_timer_get_irq request_irq
> omap_dm_timer_set_source clk_set_rate
For clk_set_rate, how would one directly access the timer node if we've hidden
it behind an irq chip abstraction?
per your suggestion, one would have something like:
dsp {
interrupt-parent = <&timer1>;
}
so how do you clk_set_rate rate something like this given the dsp node?
If the suggestion is to get the timer1 node from the interrupt-parent property,
if I may say- that's a bit ugly because now you're breaking the irq chip
abstraction just to access the timer node..
> omap_dm_timer_stop disable_irq
> omap_dm_timer_start enable_irq
> omap_dm_timer_disable disable_irq
> omap_dm_timer_free free_irq
> omap_dm_timer_set_int_enable enable_irq
>
> After that, what's left to export are some functions to configure
> the hardware timer:
>
> omap_dm_timer_write_counter
> omap_dm_timer_set_match
> omap_dm_timer_read_counter
> omap_dm_timer_read_status
> omap_dm_timer_write_status
Same comment for clk_set_rate applies here, we've to access interrupt-parent
property to get timer node and call dmtimer API which breaks the abstraction.
Usually I've seen frameworks come up because there is code duplication etc. In
this case there is no duplication as such, and the number of users of the API is
also few. What is the problem with exposing a small set of API to timer users in
drivers/ specially when they are prefixed by a unique name (omap_dm_timer)?
thanks,
-Joel
--
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
