Re: [PATCH v2 2/3] clocksource/drivers/timer-ti-dm: Add clocksource support

["Markus Schneider-Pargmann" <msp@baylibre.com>]

[-- Attachment #1: Type: text/plain, Size: 1841 bytes --]

Hi Daniel,

On Tue Jan 20, 2026 at 4:04 PM CET, Daniel Lezcano wrote:
> On 12/16/25 21:03, Markus Schneider-Pargmann (TI.com) wrote:
>> Add support for using the TI Dual-Mode Timer as a clocksource. The
>> driver automatically picks the first timer that is marked as always-on
>> on with the "ti,timer-alwon" property to be the clocksource.
>> 
>> The timer can then be used for CPU independent time keeping.
>> 
>> Signed-off-by: Markus Schneider-Pargmann (TI.com) <msp@baylibre.com>
>> ---
>
> [ ... ]
>
>> +static int omap_dm_timer_setup_clocksource(struct dmtimer *timer)
>> +{
>> +	struct device *dev = &timer->pdev->dev;
>> +	struct dmtimer_clocksource *clksrc;
>> +	int err;
>> +
>> +	__omap_dm_timer_init_regs(timer);
>> +
>> +	timer->reserved = 1;
>> +
>> +	clksrc = devm_kzalloc(dev, sizeof(*clksrc), GFP_KERNEL);
>> +	if (!clksrc)
>> +		return -ENOMEM;
>
> [ ... ]
>
>> +	clksrc->timer = timer;
>> +	timer->clksrc = clksrc;
>
> This is a cyclic dependency which is not desired in general.
>
> I suggest to have struct dmtimer_clocksource containing struct dmtimer timer

Thank you! Not many timers will probably have the clocksource part
setup, so I didn't like to embed struct dmtimer into
dmtimer_clocksource. I could have used different allocation routines in
probe to use either struct dmtimer_clocksource or struct dmtimer
directly depending on the use of clocksource, but I wasn't convinced by
that either.

For the code here I am only having the timer->clksrc pointer to
unregister the clocksource in cleanup. So I found

  devm_add_action_or_reset()

which can call the clocksource_unregister and eliminate the need for the
pointer. This is also already used in the timer-nxp-stm.c driver.

So if you are happy with that, I would prefer that solution.

Best
Markus

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 289 bytes --]