Re: [PATCH v2 1/4] clocksource: fttmr010: Parametrise shutdown

[Joel Stanley]

On Wed, 29 Jan 2020 at 11:53, Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> wrote:
>
> On 29/01/2020 12:41, Joel Stanley wrote:
> > On Wed, 29 Jan 2020 at 11:25, Daniel Lezcano <daniel.lezcano@xxxxxxxxxx> wrote:
> >>
> >> On 07/11/2019 10:42, Joel Stanley wrote:
> >>> In preparation for supporting the ast2600 which uses a different method
> >>> to clear bits in the control register, use a callback for performing the
> >>> shutdown sequence.
> >>>
> >>> Reviewed-by: CÃdric Le Goater <clg@xxxxxxxx>
> >>> Reviewed-by: Linus Walleij <linus.walleij@xxxxxxxxxx>
> >>> Signed-off-by: Joel Stanley <joel@xxxxxxxxx>
> >>
> >> It will be cleaner if you create a struct of_device_id array where you
> >> store the different variant data.
> >
> > I agree, and that's the path I would have taken when writing a normal
> > driver. However as the timer drivers probe with the
> > TIMER_OF_DECLARE/timer_probe infrastructure, we can't register our own
> > .data pointer (TIMER_OF_DECLARE uses .data  to store the _init
> > function).
> >
> > Unless I'm missing something?
>
> I was suggesting to add the array like:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/clocksource/timer-atmel-tcb.c#n351
>
> TIMER_OF_DECLARE is used and the probe functions relies on the match array:
>
> match = of_match_node(atmel_tcb_of_match, node->parent);

I see. So we don't use this table for probing the driver, just getting
the associated data.

I'm not convinced that's an improvement over what we have. If you have
a strong preference I can try it out.