Re: [PATCH V5 6/9] MIPS: Loongson-3: support irq_set_affinity() in i8259 chip

[Huacai Chen]

OK, I'll update my patch and set it at runtime.

Huacai

On Thu, Jun 15, 2017 at 9:15 PM, Marc Zyngier <marc.zyngier@xxxxxxx> wrote:
> Thanks Ralf.
>
> On 15/06/17 12:41, Ralf Baechle wrote:
>> On Thu, Jun 15, 2017 at 10:31:05AM +0800, Huacai Chen wrote:
>>
>>> With this patch we can set irq affinity via procfs, so as to improve
>>> network performance.
>>>
>>> Signed-off-by: Huacai Chen <chenhc@xxxxxxxxxx>
>>> ---
>>>  arch/mips/include/asm/irq.h           |  3 ++
>>>  arch/mips/loongson64/loongson-3/irq.c | 62 +++++++++++++++++++++++++++--------
>>>  drivers/irqchip/irq-i8259.c           |  3 ++
>>>  3 files changed, 55 insertions(+), 13 deletions(-)
>>
>> You didn't cc the IRQCHIP maintainers:
>>
>> IRQCHIP DRIVERS
>> M:      Thomas Gleixner <tglx@xxxxxxxxxxxxx>
>> M:      Jason Cooper <jason@xxxxxxxxxxxxxx>
>> M:      Marc Zyngier <marc.zyngier@xxxxxxx>
>> L:      linux-kernel@xxxxxxxxxxxxxxx
>> S:      Maintained
>> T:      git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git irq/core
>> T:      git git://git.infradead.org/users/jcooper/linux.git irqchip/core
>> F:      Documentation/devicetree/bindings/interrupt-controller/
>> F:      drivers/irqchip/
>>
>>   Ralf
>>
>>
>>> diff --git a/arch/mips/include/asm/irq.h b/arch/mips/include/asm/irq.h
>>> index ddd1c91..47ff7c6 100644
>>> --- a/arch/mips/include/asm/irq.h
>>> +++ b/arch/mips/include/asm/irq.h
>>> @@ -53,6 +53,7 @@ static inline int irq_canonicalize(int irq)
>>>  #define irq_canonicalize(irq) (irq) /* Sane hardware, sane code ... */
>>>  #endif
>>>
>>> +struct irq_data;
>>>  asmlinkage void plat_irq_dispatch(void);
>>>
>>>  extern void do_IRQ(unsigned int irq);
>>> @@ -63,6 +64,8 @@ extern void spurious_interrupt(void);
>>>  extern int allocate_irqno(void);
>>>  extern void alloc_legacy_irqno(void);
>>>  extern void free_irqno(unsigned int irq);
>>> +extern int plat_set_irq_affinity(struct irq_data *d,
>>> +                             const struct cpumask *affinity, bool force);
>>>
>>>  /*
>>>   * Before R2 the timer and performance counter interrupts were both fixed to
>>> diff --git a/arch/mips/loongson64/loongson-3/irq.c b/arch/mips/loongson64/loongson-3/irq.c
>>> index 2e6e205..e8b7a47 100644
>>> --- a/arch/mips/loongson64/loongson-3/irq.c
>>> +++ b/arch/mips/loongson64/loongson-3/irq.c
>
> [...]
>
> Not going to comment on the Loongson-specific code which doesn't make
> much sense to me (the patch doesn't explain anything about what it is
> actually doing), but...
>
>>> diff --git a/drivers/irqchip/irq-i8259.c b/drivers/irqchip/irq-i8259.c
>>> index 1aec12c..95d21e3 100644
>>> --- a/drivers/irqchip/irq-i8259.c
>>> +++ b/drivers/irqchip/irq-i8259.c
>>> @@ -46,6 +46,9 @@ static struct irq_chip i8259A_chip = {
>>>      .irq_disable            = disable_8259A_irq,
>>>      .irq_unmask             = enable_8259A_irq,
>>>      .irq_mask_ack           = mask_and_ack_8259A,
>>> +#ifdef CONFIG_CPU_LOONGSON3
>>> +    .irq_set_affinity       = plat_set_irq_affinity,
>>> +#endif
>>>  };
>
> ... that's a pretty horrible way of hooking up inside a random driver.
>
> Doesn't MIPS have some form of multi-platform kernel? If you need to add
> something like this, it'd be better to set it at runtime, once you've
> made sure that you're on the relevant HW (and preferably using an accessor).
>
> Thanks,
>
>         M.
> --
> Jazz is not dead. It just smells funny...
>