drivers/irqchip/irq-mips-gic.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
Devices of the Realtek MIPS Otto platform use the official rtl-otto-timer
as clock event generator and cpu clocksource. It is registered for cpu
startup via cpuhp_setup_state() and forces the affinity of the clockevent
interrupts to the appropriate cpu via irq_force_affinity().
On the "smaller" devices with a vendor specific interrupt controller
(supported by irq-realtek-rtl) the registration works fine. The "larger"
RTL931x series is based on a MIPS interAptiv dual core with a MIPS GIC
controller. Interrupt routing setup is cancelled because gic_set_affinity()
does not accept the current (not yet online) cpu as a target.
Relax the checks by evaluating the force parameter that is provided for
exactly this purpose. With this patch affinity can be set as follows:
- force = false: works like before
- force = true: allow to set affinity to the current not yet online cpu
Signed-off-by: Markus Stockhausen <markus.stockhausen@gmx.de>
Signed-off-by: Sebastian Gottschall <s.gottschall@dd-wrt.com>
---
drivers/irqchip/irq-mips-gic.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c
index bca8053864b2..0a88f785e114 100644
--- a/drivers/irqchip/irq-mips-gic.c
+++ b/drivers/irqchip/irq-mips-gic.c
@@ -378,9 +378,18 @@ static int gic_set_affinity(struct irq_data *d, const struct cpumask *cpumask,
* the first online CPU in the mask.
*/
cpu = cpumask_first_and(cpumask, cpu_online_mask);
- if (cpu >= NR_CPUS)
+
+ if ((cpu >= NR_CPUS) && !force)
+ /* In normal mode allow only online CPUs. */
return -EINVAL;
+ if (cpu >= NR_CPUS) {
+ /* In force mode allow current not yet online CPU for hotplug handlers. */
+ cpu = cpumask_first(cpumask);
+ if (cpu != get_cpu())
+ return -EINVAL;
+ }
+
old_cpu = cpumask_first(irq_data_get_effective_affinity_mask(d));
old_cl = cpu_cluster(&cpu_data[old_cpu]);
cl = cpu_cluster(&cpu_data[cpu]);
--
2.47.0
On Fri, May 23 2025 at 11:15, Markus Stockhausen wrote:
> +
> + if ((cpu >= NR_CPUS) && !force)
> + /* In normal mode allow only online CPUs. */
> return -EINVAL;
>
> + if (cpu >= NR_CPUS) {
> + /* In force mode allow current not yet online CPU for hotplug handlers. */
> + cpu = cpumask_first(cpumask);
> + if (cpu != get_cpu())
> + return -EINVAL;
> + }
This logic really makes my brain hurt. Why not doing the obvious:
if (cpu >= NR_CPUS) {
/* Sensible comment */
if (!force)
return -EINVAL;
...
}
Hmm?
Thanks
tglx
> Von: Thomas Gleixner <tglx@linutronix.de>
>
> On Fri, May 23 2025 at 11:15, Markus Stockhausen wrote:
> > +
> > + if ((cpu >= NR_CPUS) && !force)
> > + /* In normal mode allow only online CPUs. */
> > return -EINVAL;
> >
> > + if (cpu >= NR_CPUS) {
> > + /* In force mode allow current not yet online CPU for
hotplug handlers. */
> > + cpu = cpumask_first(cpumask);
> > + if (cpu != get_cpu())
> > + return -EINVAL;
> > + }
>
> This logic really makes my brain hurt. Why not doing the obvious:
>
> if (cpu >= NR_CPUS) {
> /* Sensible comment */
> if (!force)
> return -EINVAL;
> ...
> }
Then what about an even more relaxed and cleaner version like in other
drivers?
If (force)
cpu = cpumask_first(cpumask);
else
cpu = cpumask_first_and(cpumask, cpu_online_mask);
Markus
On Sun, May 25 2025 at 11:43, markus stockhausen wrote:
>> Von: Thomas Gleixner <tglx@linutronix.de>
>>
>> On Fri, May 23 2025 at 11:15, Markus Stockhausen wrote:
>> > +
>> > + if ((cpu >= NR_CPUS) && !force)
>> > + /* In normal mode allow only online CPUs. */
>> > return -EINVAL;
>> >
>> > + if (cpu >= NR_CPUS) {
>> > + /* In force mode allow current not yet online CPU for
> hotplug handlers. */
>> > + cpu = cpumask_first(cpumask);
>> > + if (cpu != get_cpu())
>> > + return -EINVAL;
>> > + }
>>
>> This logic really makes my brain hurt. Why not doing the obvious:
>>
>> if (cpu >= NR_CPUS) {
>> /* Sensible comment */
>> if (!force)
>> return -EINVAL;
>> ...
>> }
>
> Then what about an even more relaxed and cleaner version like in other
> drivers?
>
> If (force)
> cpu = cpumask_first(cpumask);
> else
> cpu = cpumask_first_and(cpumask, cpu_online_mask);
Fine with me.
© 2016 - 2025 Red Hat, Inc.