Re: [PATCH] of/irq: Add msi-parent check to of_msi_xlate()

Rob Herring
2025-09-18T13:56:09Z

On Tue, Sep 16, 2025 at 11:18:58AM +0200, Lorenzo Pieralisi wrote:
> In some legacy platforms the MSI controller for a PCI host
> bridge is identified by an msi-parent property whose phandle
> points at an MSI controller node with no #msi-cells property,
> that implicitly means #msi-cells == 0.
>
> For such platforms, mapping a device ID and retrieving the
> MSI controller node becomes simply a matter of checking
> whether in the device hierarchy there is an msi-parent property
> pointing at an MSI controller node with such characteristics.
>
> Add a helper function to of_msi_xlate() to check the msi-parent
> property in addition to msi-map and retrieve the MSI controller
> node (with a 1:1 ID deviceID-IN<->deviceID-OUT mapping) to
> provide support for deviceID mapping and MSI controller node
> retrieval for such platforms.

Your line wrapping is a bit short.

I had a look at who is parsing "msi-parent" themselves as that's
typically a recipe for doing it incorrectly ('interrupt-map' anyone).
Can we make iproc_pcie_msi_enable() use this? It's quite ugly reaching
into the GICv3 node...

And perhaps irq-gic-its-msi-parent.c could use this?

And looks like pcie-layerscape-gen4 is leaking a node reference...

>
> Signed-off-by: Lorenzo Pieralisi <lpieralisi@kernel.org>
> Cc: Sascha Bischoff <sascha.bischoff@arm.com>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Marc Zyngier <maz@kernel.org>
> ---
> drivers/of/irq.c | 38 +++++++++++++++++++++++++++++++++++---
> 1 file changed, 35 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index e7c12abd10ab..d0e2dfd0ee28 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -670,6 +670,35 @@ void __init of_irq_init(const struct of_device_id *matches)
> }
> }
>
> +static int of_check_msi_parent(struct device_node *dev_node, struct device_node **msi_node)
> +{
> + struct of_phandle_args msi_spec;
> + int ret;
> +
> + /*
> + * An msi-parent phandle with a missing or == 0 #msi-cells
> + * property identifies a 1:1 ID translation mapping.
> + *
> + * Set the msi controller node if the firmware matches this
> + * condition.
> + */
> + ret = of_parse_phandle_with_optional_args(dev_node, "msi-parent", "#msi-cells",
> + 0, &msi_spec);
> + if (!ret) {
> + if ((*msi_node && *msi_node != msi_spec.np) || msi_spec.args_count != 0)
> + ret = -EINVAL;
> +
> + if (!ret) {
> + /* Return with a node reference held */
> + *msi_node = msi_spec.np;
> + return 0;
> + }
> + of_node_put(msi_spec.np);
> + }
> +
> + return ret;
> +}
> +
> /**
> * of_msi_xlate - map a MSI ID and find relevant MSI controller node
> * @dev: device for which the mapping is to be done.
> @@ -677,7 +706,7 @@ void __init of_irq_init(const struct of_device_id *matches)
> * @id_in: Device ID.
> *
> * Walk up the device hierarchy looking for devices with a "msi-map"
> - * property. If found, apply the mapping to @id_in.
> + * or "msi-parent" property. If found, apply the mapping to @id_in.
> * If @msi_np points to a non-NULL device node pointer, only entries targeting
> * that node will be matched; if it points to a NULL value, it will receive the
> * device node of the first matching target phandle, with a reference held.
> @@ -691,12 +720,15 @@ u32 of_msi_xlate(struct device *dev, struct device_node **msi_np, u32 id_in)
>
> /*
> * Walk up the device parent links looking for one with a
> - * "msi-map" property.
> + * "msi-map" or an "msi-parent" property.
> */
> - for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent)
> + for (parent_dev = dev; parent_dev; parent_dev = parent_dev->parent) {
> if (!of_map_id(parent_dev->of_node, id_in, "msi-map",
> "msi-map-mask", msi_np, &id_out))
> break;
> + if (!of_check_msi_parent(parent_dev->of_node, msi_np))
> + break;
> + }
> return id_out;
> }
>
> --
> 2.48.0
>