Re: [PATCH V11 00/10] irqchip: Add LoongArch-related irqchip drivers

[Huacai Chen]

Hi, Marc,

On Thu, Jun 9, 2022 at 8:01 PM Marc Zyngier <maz@xxxxxxxxxx> wrote:
>
> + Jianmin Lv
>
> On Fri, 20 May 2022 16:04:28 +0100,
> Jiaxun Yang <jiaxun.yang@xxxxxxxxxxx> wrote:
> >
> >
> >
> > 在 2022/5/5 16:58, Marc Zyngier 写道:
> > >> LoongArch use ACPI, but ACPI tables cannot describe the hierarchy of
> > >> irqchips, so we initilize the irqchip subsystem in this way (from arch
> > >> code):
> > >>
> > >>    cpu_domain = loongarch_cpu_irq_init();
> > >>    liointc_domain = liointc_acpi_init(cpu_domain, acpi_liointc);
> > >>    eiointc_domain = eiointc_acpi_init(cpu_domain, acpi_eiointc);
> > >>    pch_pic_domain = pch_pic_acpi_init(eiointc_domain, acpi_pchpic);
> > >>    pch_msi_domain = pch_msi_acpi_init(eiointc_domain, acpi_pchmsi);
> > > I said no to this in the past, and I'm going to reiterate: this is
> > > *not* acceptable. This obviously doesn't scale and isn't manageable in
> > > the long run. Hardcoding the topology and the probing order in the
> > > kernel code has repeatedly proved to be a disaster, and yet you refuse
> > > to take any input from past experience. This is pretty worrying.
> > Just my two cents here.
> >
> > Those drivers have such a topology just because this was my design to
> > handle irqchip differences between RS780E and LS7A for MIPS-era Loongson.
> >
> > TBH, for LoongArch-era Loongson, they should be handled by the same driver,
> > cuz the topology behind them just looks like GIC PPI SPI and MSI for
> > Arm GIC.
> >
> > PCH PIC and eiointc in combination relays interrupts from
> > peripherals just like SPI.  liointc is doing the PPI job. They are
> > not separated modules in hardware, they are interlocked.
>
> That was my impression too, but I keep getting pushback on that. I
> wouldn't mind leaving the existing drivers for MIPS only and get new
> ones for Loongson if that made things clearer.
>
> > The system should be treated as a whole, pretty much like how we see
> > Arm's GIC. The topology will last forever for every ACPI enabled
> > LoongArch PC.
> >
> > I see no reason they should be described separately. Adding complicities to
> > ACPI bindings brings no benefit. Changing ACPI binding which is already in
> > final draft stage can only leave us with chaos.
>
> OK. So how do we move forward? You seem to have a good grasp on how
> this should be structured, so can you work with Jianmin Lv to make
> some progress on this?
Thank you for helping us to move this forward, and I have some
considerations below:
1, Jianmin Lv and I have both made some effort on the irqchip driver
design, but Jianmin more or less lacks some community experience, so
he has made some mistakes as most new-comers. I think he will improve
in the future.
2, And then, maybe we can temporarily ignore those new-comers'
mistakes and focus on the key problem. I think the key problem is that
our irqchip topology is fixed in hardware (not re-organizable, which
can nearly be treated as a whole, as Jiaxun described before), and the
ACPI spec is frozen in late 2021.
3, To solve the key problem, Jianmin and I propose two different
designs. My solution is in V11 and previous versions: initialize the
root irqchip in the arch code, and return the root domain, then the
downstream irqchip takes the root domain as its parent to initialize
itself, and so on. While Jianmin's design is in RFC versions: the arch
code only calls irqchip_init() which causes it to initialize the root
irqchip, and the root irqchip driver explicitly calls its downstream
irqchips' initialization. I think this is the main difference between
them.
4, There is coupling between arch and driver in my solution, and there
is coupling between one driver and some other drivers in Jianmin's
solution. Both of the two solutions are not perfect (or we can even
say they are ugly). Which one do you think is a little better?


Huacai

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