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

[Lorenzo Pieralisi <lpieralisi@kernel.org>]

On Tue, Oct 21, 2025 at 06:55:17AM -0500, Rob Herring wrote:
> On Fri, Oct 17, 2025 at 10:47:48AM +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.
> > 
> > Fixes: 57d72196dfc8 ("irqchip/gic-v5: Add GICv5 ITS support")
> > 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(-)
> 
> It all looks good to me other than 1 nit below. How do you propose 
> merging the series? I can take the first 3 for 6.18 and then patches 4 
> and 5 can go via their respective trees for 6.19?

Yep, though patch (3) isn't really a fix so not sure it is v6.18 material
but that's up to you.

I'd leave patch (1) brewing in -next if possible a little bit - it should
not cause any issues but it might.

> > 
> > diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> > index 65c3c23255b7..e67b2041e73b 100644
> > --- a/drivers/of/irq.c
> > +++ b/drivers/of/irq.c
> > @@ -671,6 +671,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 (ret)
> 	return ret;
> 
> And then save a level of indentation.

Fixed - sent v4.

Thanks !
Lorenzo

> > +		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.
> > @@ -678,7 +707,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.
> > @@ -692,12 +721,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.50.1
> >