Hi Marc, On Tuesday, 15 August 2017 09:44:24 PDT Marc Zyngier wrote: > On 15/08/17 17:23, Paul Burton wrote: > > Hi Marc, > > > > On Tuesday, 15 August 2017 04:34:46 PDT Marc Zyngier wrote: > >> The MIPS GIC driver only targets a single CPU at a time, even if > >> the notional affinity is wider. Let's inform the core code > >> about this. > >> > >> Signed-off-by: Marc Zyngier <[email protected]> > >> --- > >> > >> drivers/irqchip/Kconfig | 1 + > >> drivers/irqchip/irq-mips-gic.c | 2 ++ > >> 2 files changed, 3 insertions(+) > >> > >> diff --git a/drivers/irqchip/Kconfig b/drivers/irqchip/Kconfig > >> index 39bfa5b25b54..bca9a88012f0 100644 > >> --- a/drivers/irqchip/Kconfig > >> +++ b/drivers/irqchip/Kconfig > >> @@ -141,6 +141,7 @@ config IRQ_MIPS_CPU > >> > >> select GENERIC_IRQ_IPI if SYS_SUPPORTS_MULTITHREADING > >> select IRQ_DOMAIN > >> select IRQ_DOMAIN_HIERARCHY if GENERIC_IRQ_IPI > >> > >> + select GENERIC_IRQ_EFFECTIVE_AFF_MASK > >> > >> config CLPS711X_IRQCHIP > >> > >> bool > >> > >> diff --git a/drivers/irqchip/irq-mips-gic.c > >> b/drivers/irqchip/irq-mips-gic.c index 6ab1d3afec02..e075cb25fad6 100644 > >> --- a/drivers/irqchip/irq-mips-gic.c > >> +++ b/drivers/irqchip/irq-mips-gic.c > >> @@ -463,6 +463,7 @@ static int gic_set_affinity(struct irq_data *d, const > >> struct cpumask *cpumask, set_bit(irq, > >> pcpu_masks[cpumask_first(&tmp)].pcpu_mask); > >> > >> cpumask_copy(irq_data_get_affinity_mask(d), cpumask); > >> > >> + irq_data_update_effective_affinity(d, cpumask); > > > > This doesn't seem right - it's just setting the effective affinity to the > > same as affinity, not taking into account CPU restrictions at all. I > > think this> > > should be: > > irq_data_update_effective_affinity(d, cpumask_of(cpumask_first(&tmp))); > > > > (or something cleaner but to that effect) > > Gah, you're absolutely right (/me plugs brain back in). How about something > like this: > > diff --git a/drivers/irqchip/irq-mips-gic.c b/drivers/irqchip/irq-mips-gic.c > index e075cb25fad6..6461380ff1a4 100644 > --- a/drivers/irqchip/irq-mips-gic.c > +++ b/drivers/irqchip/irq-mips-gic.c > @@ -445,25 +445,27 @@ static int gic_set_affinity(struct irq_data *d, const > struct cpumask *cpumask, unsigned int irq = GIC_HWIRQ_TO_SHARED(d->hwirq); > cpumask_t tmp = CPU_MASK_NONE; > unsigned long flags; > - int i; > + int i, cpu; > > cpumask_and(&tmp, cpumask, cpu_online_mask); > if (cpumask_empty(&tmp)) > return -EINVAL; > > + cpu = cpumask_first(&tmp); > + > /* Assumption : cpumask refers to a single CPU */ > spin_lock_irqsave(&gic_lock, flags); > > /* Re-route this IRQ */ > - gic_map_to_vpe(irq, mips_cm_vp_id(cpumask_first(&tmp))); > + gic_map_to_vpe(irq, mips_cm_vp_id(cpu)); > > /* Update the pcpu_masks */ > for (i = 0; i < min(gic_vpes, NR_CPUS); i++) > clear_bit(irq, pcpu_masks[i].pcpu_mask); > - set_bit(irq, pcpu_masks[cpumask_first(&tmp)].pcpu_mask); > + set_bit(irq, pcpu_masks[cpu].pcpu_mask); > > cpumask_copy(irq_data_get_affinity_mask(d), cpumask); > - irq_data_update_effective_affinity(d, cpumask); > + irq_data_update_effective_affinity(d, cpumask_of(cpu)); > spin_unlock_irqrestore(&gic_lock, flags); > > return IRQ_SET_MASK_OK_NOCOPY; > > Thanks, > > M.
That looks better (though patch 37 of my "irqchip: mips-gic: Cleanup &
optimisation" series would get you that cpu variable already ;) ).
Thanks,
Paul
signature.asc
Description: This is a digitally signed message part.
