Re: [PATCH v5 1/8] clocksource: dmtimer: Remove all the exports

[Tony Lindgren]

* Keerthy <j-keerthy@xxxxxx> [171212 08:25]:
> 
> 
> On Tuesday 12 December 2017 01:49 PM, Ladislav Michl wrote:
> > On Tue, Dec 12, 2017 at 01:38:04PM +0530, Keerthy wrote:
> >> On Tuesday 12 December 2017 01:31 PM, Ladislav Michl wrote:
> >>> On Tue, Dec 12, 2017 at 01:01:51PM +0530, Keerthy wrote:
> >>>>
> >>>>
> >>>> On Tuesday 12 December 2017 12:46 PM, Ladislav Michl wrote:
> >>>>> Keerthy,
> >>>>>
> >>>>> On Tue, Dec 12, 2017 at 11:42:10AM +0530, Keerthy wrote:
> >>>>>> Remove all the unwanted exports from the driver
> >>>>>
> >>>>> I'm adding event capture capability to the pwm-omap driver and so far used
> >>>>> v4.15-rc3 as codebase.
> >>>>>
> >>>>> Intended use is an IR receiver; for that I need to measure pulses width and
> >>>>> spaces between pulses. So DM timer was setup to generate interupt after
> >>>>> both TCAR1 and TCAR2 are filled, values are passed to IR decoder and
> >>>>> TCAR_IT_FLAG is cleared.
> >>>>>
> >>>>> Of course, this is just proof of concept and needs to be polished and
> >>>>> generalized, but to make it at least work I need functions you just
> >>>>> unexported (plus some new).
> >>>>>
> >>>>> Question is whenever we need this level of indirection (omap_dm_timer_ops)
> >>>>> or plain exports are enough.
> >>>>
> >>>> The general guidance is not to do plain exports and go via
> >>>> omap_dm_timer_ops.
> >>>
> >>> ...in contrary what other clocksource drivers are doing.

Hmm what do you mean? We don't want to export tons of custom functions from
the timers in and then be in trouble when at some point we have a Linux
generic hw timer framework. We already had to deal with these custom
exports earlier with conversion to multiarch and then again with
device tree.

For now, it's best to pass the timer information to the pwm driver in
platform data. In the long run that will be much easier to deal with than
fixing random drivers tinkering with the timer registers directly.

> >>> Now I'm assuming it is okay to extend omap_dm_timer_ops. That would mean
> >>> check for ops members to be assigned should be also extended or we should
> >>> delete it altogether and assume all members are populated?
> >>
> >> It should be fine to extend omap_dm_timer_ops. What are the ops missing
> >> for your new implementation?
> > 
> > Read capture registers, configure capture and ack interrupt. Perhaps set_pwm
> > could be extended to configure capture as well.
> > 
> > I'll update my code on top of your changes and we'll see how it would work.

Ideally the pwm driver would just do a request_irq from the dmtimer code
where dmtimer code would implement an interrupt controller. That would
be already most fo the Linux generic hardware timer framework right there :)

Regards,

Tony