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

[Lorenzo Pieralisi <lpieralisi@kernel.org>]

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;
> > +}
> 
>