Messages in this thread | | | From | Anup Patel <> | Date | Wed, 4 Oct 2023 21:38:06 +0530 | Subject | Re: [PATCH] irqchip/riscv-intc: Mark INTC nodes for secondary CPUs as initialized. |
| |
On Wed, Oct 4, 2023 at 9:02 PM Marc Zyngier <maz@kernel.org> wrote: > > On Wed, 04 Oct 2023 15:59:33 +0100, > Anup Patel <apatel@ventanamicro.com> wrote: > > > > On Wed, Oct 4, 2023 at 3:48 PM Marc Zyngier <maz@kernel.org> wrote: > > > > > > On Tue, 26 Sep 2023 11:36:31 +0100, > > > Anup Patel <apatel@ventanamicro.com> wrote: > > > > > > > > On Tue, Sep 26, 2023 at 3:59 PM Dmitry Dunaev <dunaev@tecon.ru> 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@tecon.ru> > > > > > --- > > > > > 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. > > I hope you're not suggesting that only the DTs that are present in the > kernel tree are valid. Because as far as I'm concern, the DTs in the > kernel tree are only some *examples*, and not a reference.
I am only saying why this issue was not observed before.
> > I fully expect the vast majority of DTs to live *outside* of the > kernel tree, provided by the firmware, and never upstreamed. Would you > expect every PC vendor to upstream their ACPI tables?
I agree. We can't expect all vendors to submit DT to 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. > > Is this configuration legal or not as per the DT binding? I don't see > anything that suggests it isn't legal, and having per-CPU interrupts > isn't exactly a new thing.
This is a perfect legal configuration in the RISC-V world. We have similar DT binding for AIA drivers as well.
> > > 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. > > You don't, but Dimitry does. Who wins?
I am totally fine taking PATCH3 of the Linux AIA v10 series as a fix PATCH for 6.6-rcX. The PATCH3 is pretty self contained and does not depend on any other PATCH of Linux AIA v10 series.
Do you want me to re-send it as an individual PATCH ?
Regards, Anup
> > M. > > -- > Without deviation from the norm, progress is not possible.
| |