Messages in this thread |  | | From | Huacai Chen <> | Date | Sun, 17 Jul 2022 22:08:14 +0800 | Subject | Re: [PATCH V15 00/15] irqchip: Add LoongArch-related irqchip drivers |
| |
Hi, Marc, Jianmin,
On Sun, Jul 17, 2022 at 7:29 PM Jianmin Lv <lvjianmin@loongson.cn> wrote: > > > > On 2022/7/17 下午6:02, Marc Zyngier wrote: > > On Sun, 17 Jul 2022 02:06:12 +0100, > > Jianmin Lv <lvjianmin@loongson.cn> wrote: > >> > >> > >> > >> On 2022/7/17 上午2:39, Marc Zyngier wrote: > >>> On Fri, 15 Jul 2022 08:05:36 +0100, > >>> Jianmin Lv <lvjianmin@loongson.cn> wrote: > >>>> > >>>> LoongArch is a new RISC ISA, which is a bit like MIPS or RISC-V. > >>>> LoongArch includes a reduced 32-bit version (LA32R), a standard 32-bit > >>>> version (LA32S) and a 64-bit version (LA64). LoongArch use ACPI as its > >>>> boot protocol LoongArch-specific interrupt controllers (similar to APIC) > >>>> are already added in the ACPI Specification 6.5(which may be published in > >>>> early June this year and the board is reviewing the draft). > >>>> > >>>> Currently, LoongArch based processors (e.g. Loongson-3A5000) can only > >>>> work together with LS7A chipsets. The irq chips in LoongArch computers > >>>> include CPUINTC (CPU Core Interrupt Controller), LIOINTC (Legacy I/O > >>>> Interrupt Controller), EIOINTC (Extended I/O Interrupt Controller), > >>>> HTVECINTC (Hyper-Transport Vector Interrupt Controller), PCH-PIC (Main > >>>> Interrupt Controller in LS7A chipset), PCH-LPC (LPC Interrupt Controller > >>>> in LS7A chipset) and PCH-MSI (MSI Interrupt Controller). > >>> > >>> [...] > >>> > >>> Compiling this series for loongarch with loongson3_defconfig on top of > >>> 5.19-rc3 results in the following: > >>> > >>> loongarch64-linux-ld: drivers/irqchip/irq-loongson-eiointc.o: in function `.L60': > >>> irq-loongson-eiointc.c:(.init.text+0x4c): undefined reference to `pch_msi_acpi_init' > >>> loongarch64-linux-ld: drivers/irqchip/irq-loongson-htvec.o: in function `pch_msi_parse_madt': > >>> irq-loongson-htvec.c:(.init.text+0x14): undefined reference to `pch_msi_acpi_init' > >>> make: *** [Makefile:1164: vmlinux] Error 1 > >>> > >>> I *really* would have expected this series to be in a better shape > >>> after over 15 rounds, but it looks like I'm expecting too much. I > >>> haven't investigated the breakage, but this should (at the very least) > >>> pass the defconfig test and optional drivers not being selected. > >>> > >>> The corresponding MIPS configuration seems to build fine. > >>> > >>> M. > >>> > >> > >> Hi, Marc > >> > >> Sorry for that first, pch_msi_acpi_init is defined in pch_msi driver > >> which is compiled depend on CONFIG_PCI, and I test the patches each > >> time with PCI patches and other(or else, kernel can not be boot), so > >> I'm ok for testing the patches. The PCI patches has been accepted by > >> PCI maintainers and will be merged in this merge window. > > > > But each series *must* at the very least compile in isolation. > > > >> > >> I don't know how to deal with this situation. Should I add *#ifdef > >> CONFIG_PCI* at position of calling pch_msi_acpi_init or some other > >> way? > > > > You could try something like this, which results in a kernel that > > fully links with defconfig and no additional patch: > > > > diff --git a/arch/loongarch/include/asm/irq.h b/arch/loongarch/include/asm/irq.h > > index ca468564fc85..4479d95867ec 100644 > > --- a/arch/loongarch/include/asm/irq.h > > +++ b/arch/loongarch/include/asm/irq.h > > @@ -99,8 +99,17 @@ int htvec_acpi_init(struct irq_domain *parent, > > struct acpi_madt_ht_pic *acpi_htvec); > > int pch_lpc_acpi_init(struct irq_domain *parent, > > struct acpi_madt_lpc_pic *acpi_pchlpc); > > +#if IS_ENABLED(CONFIG_LOONGSON_PCH_MSI) > > int pch_msi_acpi_init(struct irq_domain *parent, > > - struct acpi_madt_msi_pic *acpi_pchmsi); > > + struct acpi_madt_msi_pic *acpi_pchmsi); > > +#else > > +static inline int pch_msi_acpi_init(struct irq_domain *parent, > > + struct acpi_madt_msi_pic *acpi_pchmsi) > > +{ > > + return 0; > > + > > +} > > +#endif > > int pch_pic_acpi_init(struct irq_domain *parent, > > struct acpi_madt_bio_pic *acpi_pchpic); > > int find_pch_pic(u32 gsi); > > > > Ok, thanks, I'll add this into that patch. > > > > But the other issue is that you seem to call this function from two > > different locations. This cannot be right, as there should be only one > > probe order, and not multiple. > > > > As we described two IRQ models(Legacy and Extended) in this cover > letter, the parent domain of MSI domain can be htvec domain(Legacy) or > eiointc domain(Extended). In MADT, only one APIC(HTPIC for htvec or > EIOPIC for eiointc) is allowed to pass into kernel, and then in the > irqchip driver, only one kind APIC of them can be parsed from MADT, so > we have to support two probe order for them.
I have an idea but I don't know whether it is acceptable: Marc gives an Acked-by for the whole series, then this irqchip series goes through the loongarch tree together with the PCI patches, then we don't need other hacks except the ACPI definitions.
Huacai > > > M. > > > >
|  |