Re: [PATCH v2 6/7] irqchip/gic-v5: Add ACPI ITS probing

Lorenzo Pieralisi
2026-01-08T16:13:22Z

On Mon, Jan 05, 2026 at 01:55:45PM +0000, Jonathan Cameron wrote:
> On Thu, 18 Dec 2025 11:14:32 +0100
> Lorenzo Pieralisi <lpieralisi@kernel.org> wrote:
>
> > On ACPI ARM64 systems the GICv5 ITS configuration and translate frames
> > are described in the MADT table.
> >
> > Refactor the current GICv5 ITS driver code to share common functions
> > between ACPI and OF and implement ACPI probing in the GICv5 ITS driver.
> >
> > Add iort_msi_xlate() to map a device ID and retrieve an MSI controller
> > fwnode node for ACPI systems and update pci_msi_map_rid_ctlr_node() to
> > use it in its ACPI code path.
> >
> > Add the required functions to IORT code for deviceID retrieval and IRQ
> > domain registration and look-up so that the GICv5 ITS driver in an
> > ACPI based system can be successfully probed.
> >
> > Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Hanjun Guo <guohanjun@huawei.com>
> > Cc: Sudeep Holla <sudeep.holla@arm.com>
> > Cc: Marc Zyngier <maz@kernel.org>
> Hi Lorenzo,
>
> Diff was in a rather unfriendly mood on this one and smashing the xlate
> on top of the wrong function (wrt to what was being replaced).
>
> Ah well. Only one minor comment inline. Not really my area of expertise
> beyond wanting this to move forwards quickly but none the less,
>
> Reviewed-by: Jonathan Cameron <jonathan.cameron@huawei.com>
>
> > ---
> > drivers/acpi/arm64/iort.c | 95 +++++++++++++++++-----
> > drivers/irqchip/irq-gic-its-msi-parent.c | 39 ++++-----
> > drivers/irqchip/irq-gic-v5-irs.c | 7 +-
> > drivers/irqchip/irq-gic-v5-its.c | 132 ++++++++++++++++++++++++++++++-
> > drivers/pci/msi/irqdomain.c | 2 +
> > include/linux/acpi_iort.h | 10 ++-
> > include/linux/irqchip/arm-gic-v5.h | 1 +
> > 7 files changed, 241 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> > index 65f0f56ad753..17dbe66da804 100644
> > --- a/drivers/acpi/arm64/iort.c
> > +++ b/drivers/acpi/arm64/iort.c
> > @@ -595,45 +595,45 @@ u32 iort_msi_map_id(struct device *dev, u32 input_id)
> > }
> >
> > +/**
> > + * iort_pmsi_get_msi_info() - Get the device id and translate frame PA for a device
> > + * @dev: The device for which the mapping is to be done.
> > + * @dev_id: The device ID found.
> > + * @pa: optional pointer to store translate frame address.
> > + *
> > + * Returns: 0 for successful devid and pa retrieval, -ENODEV on error
> > + */
> > +int iort_pmsi_get_msi_info(struct device *dev, u32 *dev_id, phys_addr_t *pa)
> > +{
> > + struct acpi_iort_node *node, *parent = NULL;
> > + struct acpi_iort_its_group *its;
> > + int i, index;
> > +
> > + node = iort_find_dev_node(dev);
> > + if (!node)
> > + return -ENODEV;
> > +
> > + index = iort_get_id_mapping_index(node);
> > + /* if there is a valid index, go get the dev_id directly */
> > + if (index >= 0) {
> > + parent = iort_node_get_id(node, dev_id, index);
> > + } else {
> > + for (i = 0; i < node->mapping_count; i++) {
> > + parent = iort_node_map_platform_id(node, dev_id,
> > + IORT_MSI_TYPE, i);
> > + if (parent)
> > + break;
> > + }
> > + }
> > +
> Another borderline comment on what I think is a small readabilty
> improvement.
>
> I'd handle the only error case that would otherwise use the ternary below
> here
> if (!parent)
> return -ENODEV;
>
> if (pa) {
> ...
> }
> return 0;

Updated.

Thanks,
Lorenzo

> > + if (parent && pa) {
> > + int ret;
> > +
> > + its = (struct acpi_iort_its_group *)node->node_data;
> > + ret = iort_find_its_base(its->identifiers[0], pa);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return parent ? 0 : -ENODEV;
> > +}
>
>