Re: [RFC PATCH V2] acpi/irq: Add stacked IRQ domain support to PCI interrupt link

["Rafael J. Wysocki" <rafael@kernel.org>]

On Wed, Nov 18, 2020 at 2:46 PM Ard Biesheuvel <ardb@kernel.org> wrote:
>
> On Tue, 17 Nov 2020 at 19:57, Bjorn Helgaas <helgaas@kernel.org> wrote:
> >
> > Nit: please don't just make up random styles for the subject.  Run
> > "git log --oneline" on the file and/or the directory and try to follow
> > the existing convention.  Using random styles adds noise to the
> > system.
> >
> > On Tue, Nov 17, 2020 at 09:42:14PM +0800, Chen Baozi wrote:
> > > Some PCIe designs require software to do extra acknowledgements for
> > > legacy INTx interrupts. If the driver is written only for device tree,
> > > things are simple. In that case, a new driver can be written under
> > > driver/pci/controller/ with a DT node of PCIe host written like:
> > >
> > >   pcie {
> > >     ...
> > >     interrupt-map = <0 0 0  1  &pcie_intc 0>,
> > >                     <0 0 0  2  &pcie_intc 1>,
> > >                     <0 0 0  3  &pcie_intc 2>,
> > >                     <0 0 0  4  &pcie_intc 3>;
> > >
> > >     pcie_intc: legacy-interrupt-controller {
> > >       interrupt-controller;
> > >       #interrupt-cells = <1>;
> > >       interrupt-parent = <&gic>;
> > >       interrupts = <0 226 4>;
> > >     };
> > >   };
> > >
> > > Similar designs can be found on Aardvark, MediaTek Gen2 and Socionext
> > > UniPhier PCIe controller at the moment. Essentially, those designs are
> > > supported by inserting an extra interrupt controller between PCIe host
> > > and GIC and parse the topology in a DT-based PCI controller driver.
> >
> > If I understand correctly, we previously ignored the Resource Source
> > field of an Extended Interrupt Descriptor in the _PRS method of
> > PNP0C0F PCI Interrupt Link devices, and this patch adds support for
> > it.
> >
> > If that's true, this has nothing to do with DT, other than DT being
> > another way to describe the same topology, and the above details
> > really aren't relevant to this patch.
> >
> > > As we turn to ACPI, All the PCIe hosts are described the same ID of
> > > "PNP0A03" and share driver/acpi/pci_root.c. It comes to be a problem
> > > to make this kind of PCI INTx work under ACPI.
> >
> > s/All the PCIe/all the PCIe/
> >
> > But this paragraph should probably just go away in favor of something
> > about implementing Resource Source support.
> >
> > > Therefore, we introduce an stacked IRQ domain support to PCI interrupt
> > > link for ACPI. With this support, we can populate the ResourceSource
> > > to refer to a device object that describes an interrupt controller.
> > > That would allow us to refer to a dedicated driver which implements
> > > the logic needed to manage the interrupt state. With this patch,
> > > those PCI interrupt links can be supported by describing the INTx
> > > in ACPI table as the following example:
> >
> > "Stacked IRQ domain" sounds like a detail of how you're implementing
> > support for the Resource Source field for PCI Interrupt Links.
> >
> > I don't know what the dedicated driver refers to.  This *should* be
> > all generic code the follows the ACPI spec (which is pretty sketchy in
> > this area).  But I assume that there's no special driver needed for
> > devices like \SB.IXIU, and the logic associated with the interrupt
> > controller is in the AML associated with IXIU.  It would probably be
> > useful to mention the relevant methods in the IXIU methods in the
> > example below.
> >
>
> As I understand it, the intent is to provide a driver for \SB.IXIU
> that acknowledges the legacy INTx interrupts in a SoC specific way,
> and I don't see how AML could be involved here.
>
> That also explains why the routines are exported to modules - the IXIU
> driver could be modularized.

OK, but every new symbol export requires an in-the-tree user or the
patch is basically not applicable.