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

Lorenzo Pieralisi posted 1 patch 2 weeks, 2 days ago
drivers/of/irq.c | 38 +++++++++++++++++++++++++++++++++++---
1 file changed, 35 insertions(+), 3 deletions(-)
[PATCH] of/irq: Add msi-parent check to of_msi_xlate()
Posted by Lorenzo Pieralisi 2 weeks, 2 days ago
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.

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
Re: [PATCH] of/irq: Add msi-parent check to of_msi_xlate()
Posted by Rob Herring 1 week, 6 days ago
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
>
Re: [PATCH] of/irq: Add msi-parent check to of_msi_xlate()
Posted by Lorenzo Pieralisi 1 week, 3 days ago
On Thu, Sep 18, 2025 at 08:55:55AM -0500, Rob Herring wrote:
> 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...

Hi Rob,

given where we are in the development cycle and that technically this
is a GICv5 fix I would send a patch to fix the issue in GICv5 MSI
parent code in drivers/irqchip (yes, it is more code handling the
msi-parent property scattered around) and then will resume this
thread for v6.19 - starting the clean-up.

This patch affects more platforms than GICv5 and I am not sure
it is safe to rush it in.

Thanks,
Lorenzo

> > 
> > 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
> >
Re: [PATCH] of/irq: Add msi-parent check to of_msi_xlate()
Posted by Lorenzo Pieralisi 1 week, 6 days ago
On Thu, Sep 18, 2025 at 08:55:55AM -0500, Rob Herring wrote:
> 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...

I am not sure I get what you mean here. Possibly iproc_pcie_msi_enable()
can reuse this patch's code if I extend it and make it a global function,
yes and somehow use that function to carry out the check for an
msi-parent property with no #msi-cells property or an #msi-cells == 0.

Don't get what GICv3 node has to do with that though, sorry.

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

Yes and I have to fix leaks there too (and there are other
of_phandle_with_args() calls in the kernel that don't put the args.np
pointer on success - though not to retrieve msi-parent).
> 
> And looks like pcie-layerscape-gen4 is leaking a node reference...

It looks like it is in good company I am afraid :(

Thanks,
Lorenzo

> > 
> > 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
> >
Re: [PATCH] of/irq: Add msi-parent check to of_msi_xlate()
Posted by Rob Herring 1 week, 6 days ago
On Thu, Sep 18, 2025 at 10:21 AM Lorenzo Pieralisi
<lpieralisi@kernel.org> wrote:
>
> On Thu, Sep 18, 2025 at 08:55:55AM -0500, Rob Herring wrote:
> > 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...
>
> I am not sure I get what you mean here. Possibly iproc_pcie_msi_enable()
> can reuse this patch's code if I extend it and make it a global function,
> yes and somehow use that function to carry out the check for an
> msi-parent property with no #msi-cells property or an #msi-cells == 0.

I meant using of_msi_xlate() (or even of_msi_get_domain()).

> Don't get what GICv3 node has to do with that though, sorry.

Just trace what the code there does after it gets the MSI parent. I
didn't study it too closely, but why is a iProc PCIe parsing GICv3 MSI
stuff itself? There's either some missing feature in the
irqchip/domain APIs or it's being dumb.

Rob
Re: [PATCH] of/irq: Add msi-parent check to of_msi_xlate()
Posted by Lorenzo Pieralisi 1 week, 6 days ago
On Thu, Sep 18, 2025 at 02:44:23PM -0500, Rob Herring wrote:
> On Thu, Sep 18, 2025 at 10:21 AM Lorenzo Pieralisi
> <lpieralisi@kernel.org> wrote:
> >
> > On Thu, Sep 18, 2025 at 08:55:55AM -0500, Rob Herring wrote:
> > > 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...
> >
> > I am not sure I get what you mean here. Possibly iproc_pcie_msi_enable()
> > can reuse this patch's code if I extend it and make it a global function,
> > yes and somehow use that function to carry out the check for an
> > msi-parent property with no #msi-cells property or an #msi-cells == 0.
> 
> I meant using of_msi_xlate() (or even of_msi_get_domain()).

https://lore.kernel.org/all/20240818161816.GA173148-robh@kernel.org/

Isn't of_msi_get_domain() leaking a reference when it finds an IRQ domain and
return ?

Possibly I can generalize it and use it in this patch too.

Thanks,
Lorenzo

> > Don't get what GICv3 node has to do with that though, sorry.
> 
> Just trace what the code there does after it gets the MSI parent. I
> didn't study it too closely, but why is a iProc PCIe parsing GICv3 MSI
> stuff itself? There's either some missing feature in the
> irqchip/domain APIs or it's being dumb.
> 
> Rob
Re: [PATCH] of/irq: Add msi-parent check to of_msi_xlate()
Posted by Lorenzo Pieralisi 1 week, 6 days ago
On Thu, Sep 18, 2025 at 02:44:23PM -0500, Rob Herring wrote:
> On Thu, Sep 18, 2025 at 10:21 AM Lorenzo Pieralisi
> <lpieralisi@kernel.org> wrote:
> >
> > On Thu, Sep 18, 2025 at 08:55:55AM -0500, Rob Herring wrote:
> > > 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...
> >
> > I am not sure I get what you mean here. Possibly iproc_pcie_msi_enable()
> > can reuse this patch's code if I extend it and make it a global function,
> > yes and somehow use that function to carry out the check for an
> > msi-parent property with no #msi-cells property or an #msi-cells == 0.
> 
> I meant using of_msi_xlate() (or even of_msi_get_domain()).
> 
> > Don't get what GICv3 node has to do with that though, sorry.
> 
> Just trace what the code there does after it gets the MSI parent. I
> didn't study it too closely, but why is a iProc PCIe parsing GICv3 MSI
> stuff itself? There's either some missing feature in the
> irqchip/domain APIs or it's being dumb.

I think it is setting up the host bridge to steer MSIs to the GIC ITS.

Platform specific stuff - nothing to do with irqchip/domain.

Lorenzo