Re: [PATCH V2] irqchip/loongarch-avec:Add multi-nodes topology support

[Tianyang Zhang <zhangtianyang@xxxxxxxxxxx>]

在 2024/12/31 下午3:56, Huacai Chen 写道:
> Hi, Tianyang,
>
> On Mon, Dec 30, 2024 at 10:15 AM Tianyang Zhang
> <zhangtianyang@xxxxxxxxxxx> wrote:
> > avecintc_init() enables the Advanced Interrupt Controller (AVEC) of
> > the boot CPU node, but nothing enables the AVEC on secondary nodes.
> >
> > Move the enablement to the CPU hotplug callback so that secondary
> > nodes get the AVEC enabled too. In theory enabling it once per node
> > would be sufficient, but redundant enabling does no hard, so keep the
> > code simple and do it unconditionally.
> >
> > Signed-off-by: Tianyang Zhang <zhangtianyang@xxxxxxxxxxx>
> > ---
> >   drivers/irqchip/irq-loongarch-avec.c | 17 +++++++++++++----
> >   1 file changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-loongarch-avec.c b/drivers/irqchip/irq-loongarch-avec.c
> > index 0f6e465dd309..988d3e02ac54 100644
> > --- a/drivers/irqchip/irq-loongarch-avec.c
> > +++ b/drivers/irqchip/irq-loongarch-avec.c
> > @@ -56,6 +56,15 @@ struct avecintc_data {
> >          unsigned int            moving;
> >   };
> >
> > +static inline void avecintc_enable(void)
> > +{
> > +       u64 value;
> > +
> > +       value = iocsr_read64(LOONGARCH_IOCSR_MISC_FUNC);
> > +       value |= IOCSR_MISC_FUNC_AVEC_EN;
> > +       iocsr_write64(value, LOONGARCH_IOCSR_MISC_FUNC);
> > +}
> > +
> >   static inline void avecintc_ack_irq(struct irq_data *d)
> >   {
> >   }
> > @@ -127,6 +136,8 @@ static int avecintc_cpu_online(unsigned int cpu)
> >
> >          guard(raw_spinlock)(&loongarch_avec.lock);
> >
> > +       avecintc_enable();
> > +
> >          irq_matrix_online(loongarch_avec.vector_matrix);
> >
> >          pending_list_init(cpu);
> > @@ -339,7 +350,6 @@ static int __init irq_matrix_init(void)
> >   static int __init avecintc_init(struct irq_domain *parent)
> >   {
> >          int ret, parent_irq;
> > -       unsigned long value;
> >
> >          raw_spin_lock_init(&loongarch_avec.lock);
> >
> > @@ -378,14 +388,13 @@ static int __init avecintc_init(struct irq_domain *parent)
> >                                    "irqchip/loongarch/avecintc:starting",
> >                                    avecintc_cpu_online, avecintc_cpu_offline);
> >   #endif
> > -       value = iocsr_read64(LOONGARCH_IOCSR_MISC_FUNC);
> > -       value |= IOCSR_MISC_FUNC_AVEC_EN;
> > -       iocsr_write64(value, LOONGARCH_IOCSR_MISC_FUNC);
> > +       avecintc_enable();
> >
> >          return ret;
> >
> >   out_remove_domain:
> >          irq_domain_remove(loongarch_avec.domain);
> > +       loongarch_avec.domain = NULL;
> This is unnecessary, since no one checks it, and there is no memory
> leak even if without it.
>
> Huacai
Ok. I got it, thanks
> >   out_free_handle:
> >          irq_domain_free_fwnode(loongarch_avec.fwnode);
> >   out:
> > --
> > 2.20.1