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

[Jonathan Cameron <jonathan.cameron@huawei.com>]

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;

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