| Message ID | 20181130080207.20505-4-anup@brainfault.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | IRQ affinity support in PLIC driver | expand |
> -static inline void plic_toggle(struct plic_handler *handler, > - int hwirq, int enable) > +static void plic_toggle(struct plic_handler *handler, int hwirq, int enable) > { > u32 __iomem *reg = handler->enable_base + (hwirq / 32) * sizeof(u32); > u32 hwirq_mask = 1 << (hwirq % 32); > @@ -92,27 +91,27 @@ static inline void plic_toggle(struct plic_handler *handler, > raw_spin_unlock(&handler->enable_lock); > } > > -static inline void plic_irq_toggle(struct irq_data *d, int enable) > +static void plic_irq_toggle(const struct cpumask *mask, int hwirq, int enable) It also removes inline statements which seems rather unrelated to the patch description. Also the actual addintion of the single cpumask argument is simple enough that it should probably go into the patch that makes use of it.
On Mon, Dec 17, 2018 at 11:57 PM Christoph Hellwig <hch@infradead.org> wrote: > > > -static inline void plic_toggle(struct plic_handler *handler, > > - int hwirq, int enable) > > +static void plic_toggle(struct plic_handler *handler, int hwirq, int enable) > > { > > u32 __iomem *reg = handler->enable_base + (hwirq / 32) * sizeof(u32); > > u32 hwirq_mask = 1 << (hwirq % 32); > > @@ -92,27 +91,27 @@ static inline void plic_toggle(struct plic_handler *handler, > > raw_spin_unlock(&handler->enable_lock); > > } > > > > -static inline void plic_irq_toggle(struct irq_data *d, int enable) > > +static void plic_irq_toggle(const struct cpumask *mask, int hwirq, int enable) > > It also removes inline statements which seems rather unrelated to > the patch description. Actually these functions should not be inline because plic_toggle() uses raw_spin_lock() and plic_irq_toggle() uses for-loop. > > Also the actual addintion of the single cpumask argument is simple > enough that it should probably go into the patch that makes use of it. OK, I will have separate patch for removing "inline" and move addition of cpumask argument to last patch. Regards, Anup
On Tue, Dec 18, 2018 at 02:20:10PM +0530, Anup Patel wrote: > Actually these functions should not be inline because plic_toggle() uses > raw_spin_lock() and plic_irq_toggle() uses for-loop. So? It still inlines the all of two instances into each caller for slightly different but related work. Not sure it is 100% worth it, but probably more than the one to move the calculations to init time..
On Wed, Dec 19, 2018 at 9:58 PM Christoph Hellwig <hch@infradead.org> wrote: > > On Tue, Dec 18, 2018 at 02:20:10PM +0530, Anup Patel wrote: > > Actually these functions should not be inline because plic_toggle() uses > > raw_spin_lock() and plic_irq_toggle() uses for-loop. > > So? It still inlines the all of two instances into each caller > for slightly different but related work. Not sure it is 100% worth > it, but probably more than the one to move the calculations to init > time.. Not just at init time but these functions will also be used when irq_affinity is changed by IRQ balancer at runtime. Regards, Anup
diff --git a/drivers/irqchip/irq-sifive-plic.c b/drivers/irqchip/irq-sifive-plic.c index 48bee877e0f1..d4433399eb89 100644 --- a/drivers/irqchip/irq-sifive-plic.c +++ b/drivers/irqchip/irq-sifive-plic.c @@ -78,8 +78,7 @@ struct plic_hw { static struct plic_hw plic; -static inline void plic_toggle(struct plic_handler *handler, - int hwirq, int enable) +static void plic_toggle(struct plic_handler *handler, int hwirq, int enable) { u32 __iomem *reg = handler->enable_base + (hwirq / 32) * sizeof(u32); u32 hwirq_mask = 1 << (hwirq % 32); @@ -92,27 +91,27 @@ static inline void plic_toggle(struct plic_handler *handler, raw_spin_unlock(&handler->enable_lock); } -static inline void plic_irq_toggle(struct irq_data *d, int enable) +static void plic_irq_toggle(const struct cpumask *mask, int hwirq, int enable) { int cpu; - writel(enable, plic.regs + PRIORITY_BASE + d->hwirq * PRIORITY_PER_ID); - for_each_cpu(cpu, irq_data_get_affinity_mask(d)) { + writel(enable, plic.regs + PRIORITY_BASE + hwirq * PRIORITY_PER_ID); + for_each_cpu(cpu, mask) { struct plic_handler *handler = per_cpu_ptr(&plic_handlers, cpu); if (handler->present) - plic_toggle(handler, d->hwirq, enable); + plic_toggle(handler, hwirq, enable); } } static void plic_irq_enable(struct irq_data *d) { - plic_irq_toggle(d, 1); + plic_irq_toggle(irq_data_get_affinity_mask(d), d->hwirq, 1); } static void plic_irq_disable(struct irq_data *d) { - plic_irq_toggle(d, 0); + plic_irq_toggle(irq_data_get_affinity_mask(d), d->hwirq, 0); } static struct irq_chip plic_chip = {
We make plic_irq_toggle() more generic so that we can enable/disable hwirq for given cpumask. This generic plic_irq_toggle() will be eventually used to implement set_affinity for PLIC driver. Signed-off-by: Anup Patel <anup@brainfault.org> --- drivers/irqchip/irq-sifive-plic.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-)