Re: [PATCH] irqchip/riscv-intc: Mark INTC nodes for secondary CPUs as initialized.

[Anup Patel]

On Wed, Oct 4, 2023 at 3:48 PM Marc Zyngier <maz@xxxxxxxxxx> wrote:
>
> On Tue, 26 Sep 2023 11:36:31 +0100,
> Anup Patel <apatel@xxxxxxxxxxxxxxxx> wrote:
> >
> > On Tue, Sep 26, 2023 at 3:59 PM Dmitry Dunaev <dunaev@xxxxxxxx> wrote:
> > >
> > > The current Linux driver irq-riscv-intc initialize IRQ domain only once,
> > > when init function called on primary hart. In other cases no IRQ domain is
> > > created and no operation on interrupt-controller node is performed.
> > > This is cause of that no common Linux driver can use per-cpu interrupts
> > > mapped to several CPUs because fwnode of secondary cores INTC is not
> > > marked as initialized. This device is always will be marked as deferred.
> > > For example the system with devicetree
> > >
> > >     cpu0: cpu@0 {
> > >         cpu0_intc: interrupt-controller {
> > >             interrupt-controller;
> > >             compatible = riscv,cpu-intc;
> > >         };
> > >     };
> > >
> > >     cpu1: cpu@1 {
> > >         cpu1_intc: interrupt-controller {
> > >             interrupt-controller;
> > >             compatible = riscv,cpu-intc;
> > >         };
> > >     };
> > >
> > >     buserr {
> > >         compatible = riscv,buserr;
> > >         interrupts-extended = <&cpu0_intc 16 &cpu1_intc 16>;
> > >     };
> > >
> > > will always report 'buserr' node as deferred without calling any
> > > bus probe function.
> > >
> > > This patch will mark all secondary nodes passed to irq-riscv-intc
> > > driver init function as initialized to be able to act as correct
> > > IRQ phandle node.
> > >
> > > Signed-off-by: Dmitry Dunaev <dunaev@xxxxxxxx>
> > > ---
> > >  drivers/irqchip/irq-riscv-intc.c | 8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/irqchip/irq-riscv-intc.c b/drivers/irqchip/irq-riscv-intc.c
> > > index 4adeee1bc391..c01a4e8d4983 100644
> > > --- a/drivers/irqchip/irq-riscv-intc.c
> > > +++ b/drivers/irqchip/irq-riscv-intc.c
> > > @@ -155,8 +155,10 @@ static int __init riscv_intc_init(struct device_node *node,
> > >          * for each INTC DT node. We only need to do INTC initialization
> > >          * for the INTC DT node belonging to boot CPU (or boot HART).
> > >          */
> > > -       if (riscv_hartid_to_cpuid(hartid) != smp_processor_id())
> > > +       if (riscv_hartid_to_cpuid(hartid) != smp_processor_id()) {
> > > +               fwnode_dev_initialized(of_node_to_fwnode(node), true);
> >
> > There is already a patch on LKML to address this.
> > https://www.spinics.net/lists/kernel/msg4929886.html
>
> If this is a fix, why is it buried in a huge series and not brought
> forward as an independent fix that needs to be picked early?

Dmitry saw this issue in a totally different context which is not
reproducible with existing DTS files in kernel sources.

This issue only manifests when some platform driver DT node
points to the per-HART INTC nodes. For example, RISC-V
irqchip device DT nodes point to per-HART INTC nodes.

Currently, all RISC-V irqchip drivers (INTC and PLIC) are probed
early (not as platform drivers) so we don't see this issue with
existing irqchip drivers.

The Linux AIA series implements RISC-V irqchip drivers (PLIC,
APLIC, and IMSIC) as regular platform drivers (like you suggested)
so this issue is easily seen with Linux AIA series hence the
corresponding fix is part of the Linux AIA series.
(https://www.spinics.net/lists/devicetree/msg638068.html)


Regards,
Anup



>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.