Re: [PATCH v10 4/5] clocksource/timer-econet-en751221: Support EN751627 without percpu IRQ

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] timer-econet-en751221: garbled log for skipped CPUs
--

commit 8c35e9425e27c43766e858db0613c5a5ed00f531
Author: Caleb James DeLisle <cjd@cjdns.fr>

clocksource/timer-econet-en751221: Support EN751627 without percpu IRQ

This commit adds support for the EN751627 CPU timer, which uses a different
interrupt number for each CPU timer instead of a single percpu interrupt.

> diff --git a/drivers/clocksource/timer-econet-en751221.c b/drivers/clocksource/timer-econet-en751221.c
> index f2c4c1ee0a56b..ebcc6fa342189 100644
> --- a/drivers/clocksource/timer-econet-en751221.c
> +++ b/drivers/clocksource/timer-econet-en751221.c
[ ... ]
> @@ -102,6 +104,25 @@ static int cevt_init_cpu(uint cpu)
>  	struct clock_event_device *cd = &per_cpu(econet_timer_pcpu, cpu);
>  	u32 reg;
>  
> +	if (!reg_ctl(cpu)) {
> +		pr_err("%s: missing address resource for CPU %d\n", cd->name,
> +		       cpu);

[Severity: Low]
Can cd->name be NULL here, resulting in a garbled "(null)" log message?

If a system has more possible CPUs than available IRQs and is not
using the is_percpu configuration, cevt_init() skips initializing the clock
event device for those extra CPUs:

    cevt_init() {
        ...
        for_each_possible_cpu(i) {
            ...
            if (!econet_timer.is_percpu && i >= econet_timer.num_irqs)
                continue;
            ...
            cd->name = np->name;
        }
    }

Although hotplug is unsupported, cpuhp_setup_state() is still called
during initialization and invokes cevt_init_cpu() for online CPUs. If
the topology happens to report more possible CPUs than available IRQs,
those CPUs will fail these early checks and pass the uninitialized cd->name
to pr_err().

> +		return -EINVAL;
> +	}
> +	if (cd->irq <= 0) {
> +		pr_err("%s: missing IRQ for CPU %d\n", cd->name, cpu);

[Severity: Low]
Similarly, could cd->name be uninitialized here for skipped CPUs?

> +		return -EINVAL;
> +	}
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015145331.453837-1-cjd@cjdns.fr?part=4