Message ID | 20211209001056.29774-1-prabhakar.mahadev-lad.rj@bp.renesas.com (mailing list archive) |
---|---|
State | Under Review |
Delegated to: | Geert Uytterhoeven |
Headers | show |
Series | [RFC] of: platform: Skip mapping of interrupts in of_device_alloc() | expand |
On Wed, Dec 8, 2021 at 6:11 PM Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > of_device_alloc() in early boot stage creates a interrupt mapping if > there exists a "interrupts" property in the node. > > For hierarchical interrupt domains using "interrupts" property in the node > bypassed the hierarchical setup and messed up the irq setup. > > This patch adds a check in of_device_alloc() to skip interrupt mapping if > "not-interrupt-producer" property is present in the node. This allows > nodes to describe the interrupts using "interrupts" property. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > Hi All, > > Spawning from discussion [1], here is simple patch (not the ideal probably > welcome for suggestions) from stopping the OF code from creating a map for > the interrupts when using "interrupts" property. > > [1] https://lore.kernel.org/lkml/87pmqrck2m.wl-maz@kernel.org/ > T/#mbd1e47c1981082aded4b32a52e2c04291e515508 > > Cheers, > Prabhakar > --- > drivers/of/platform.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index b3faf89744aa..629776ca1721 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -114,7 +114,7 @@ struct platform_device *of_device_alloc(struct device_node *np, > struct device *parent) > { > struct platform_device *dev; > - int rc, i, num_reg = 0, num_irq; > + int rc, i, num_reg = 0, num_irq = 0; > struct resource *res, temp_res; > > dev = platform_device_alloc("", PLATFORM_DEVID_NONE); > @@ -124,7 +124,14 @@ struct platform_device *of_device_alloc(struct device_node *np, > /* count the io and irq resources */ > while (of_address_to_resource(np, num_reg, &temp_res) == 0) > num_reg++; > - num_irq = of_irq_count(np); > + > + /* > + * we don't want to map the interrupts of hierarchical interrupt domain > + * into the parent domain yet. This will be the job of the hierarchical > + * interrupt driver code to map the interrupts as and when needed. > + */ > + if (!of_property_read_bool(np, "not-interrupt-producer")) > + num_irq = of_irq_count(np); The property won't fly for sure. A compatible match table could work here, but I don't really want another temporary solution. > /* Populate the resource table */ > if (num_irq || num_reg) { > @@ -140,7 +147,7 @@ struct platform_device *of_device_alloc(struct device_node *np, > rc = of_address_to_resource(np, i, res); > WARN_ON(rc); > } > - if (of_irq_to_resource_table(np, res, num_irq) != num_irq) > + if (num_irq && of_irq_to_resource_table(np, res, num_irq) != num_irq) You might want to look at commit 9ec36cafe43b ("of/irq: do irq resolution in platform_get_irq"). The intent was to remove this code, but looks like the cleanup has a ways to go 7 years on. Primarily, it's convert any platform_get_resource(pdev, IORESOURCE_IRQ, n) call to platform_get_irq(). There's ~169 of those. There are probably some open coded accesses to pdev->resources too, but I didn't spot any. Rob
On 2021-12-09 00:10, Lad Prabhakar wrote: > of_device_alloc() in early boot stage creates a interrupt mapping if > there exists a "interrupts" property in the node. > > For hierarchical interrupt domains using "interrupts" property in the > node > bypassed the hierarchical setup and messed up the irq setup. > > This patch adds a check in of_device_alloc() to skip interrupt mapping > if > "not-interrupt-producer" property is present in the node. This allows > nodes to describe the interrupts using "interrupts" property. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > Hi All, > > Spawning from discussion [1], here is simple patch (not the ideal > probably > welcome for suggestions) from stopping the OF code from creating a map > for > the interrupts when using "interrupts" property. > > [1] https://lore.kernel.org/lkml/87pmqrck2m.wl-maz@kernel.org/ > T/#mbd1e47c1981082aded4b32a52e2c04291e515508 > > Cheers, > Prabhakar > --- > drivers/of/platform.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index b3faf89744aa..629776ca1721 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -114,7 +114,7 @@ struct platform_device *of_device_alloc(struct > device_node *np, > struct device *parent) > { > struct platform_device *dev; > - int rc, i, num_reg = 0, num_irq; > + int rc, i, num_reg = 0, num_irq = 0; > struct resource *res, temp_res; > > dev = platform_device_alloc("", PLATFORM_DEVID_NONE); > @@ -124,7 +124,14 @@ struct platform_device *of_device_alloc(struct > device_node *np, > /* count the io and irq resources */ > while (of_address_to_resource(np, num_reg, &temp_res) == 0) > num_reg++; > - num_irq = of_irq_count(np); > + > + /* > + * we don't want to map the interrupts of hierarchical interrupt > domain > + * into the parent domain yet. This will be the job of the > hierarchical > + * interrupt driver code to map the interrupts as and when needed. > + */ > + if (!of_property_read_bool(np, "not-interrupt-producer")) I don't think this is right. If anything, the expected behaviour should be indicated by the driver and not the device node. > + num_irq = of_irq_count(np); > > /* Populate the resource table */ > if (num_irq || num_reg) { > @@ -140,7 +147,7 @@ struct platform_device *of_device_alloc(struct > device_node *np, > rc = of_address_to_resource(np, i, res); > WARN_ON(rc); > } > - if (of_irq_to_resource_table(np, res, num_irq) != num_irq) > + if (num_irq && of_irq_to_resource_table(np, res, num_irq) != > num_irq) > pr_debug("not all legacy IRQ resources mapped for %pOFn\n", > np); > } The root of the issue is that all the resource allocation is done upfront, way before we even have a driver that could potentially deal with this device. This is a potential waste of resource, and it triggers the issue you noticed. If you delay the resource allocation until there is an actual match with a driver, you could have a per-driver flag telling you whether the IRQ allocation should be performed before the probe() function is called. M.
Hi Rob, Thank you for the review. On Thu, Dec 9, 2021 at 3:08 AM Rob Herring <robh+dt@kernel.org> wrote: > > On Wed, Dec 8, 2021 at 6:11 PM Lad Prabhakar > <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > > > of_device_alloc() in early boot stage creates a interrupt mapping if > > there exists a "interrupts" property in the node. > > > > For hierarchical interrupt domains using "interrupts" property in the node > > bypassed the hierarchical setup and messed up the irq setup. > > > > This patch adds a check in of_device_alloc() to skip interrupt mapping if > > "not-interrupt-producer" property is present in the node. This allows > > nodes to describe the interrupts using "interrupts" property. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > Hi All, > > > > Spawning from discussion [1], here is simple patch (not the ideal probably > > welcome for suggestions) from stopping the OF code from creating a map for > > the interrupts when using "interrupts" property. > > > > [1] https://lore.kernel.org/lkml/87pmqrck2m.wl-maz@kernel.org/ > > T/#mbd1e47c1981082aded4b32a52e2c04291e515508 > > > > Cheers, > > Prabhakar > > --- > > drivers/of/platform.c | 13 ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > > index b3faf89744aa..629776ca1721 100644 > > --- a/drivers/of/platform.c > > +++ b/drivers/of/platform.c > > @@ -114,7 +114,7 @@ struct platform_device *of_device_alloc(struct device_node *np, > > struct device *parent) > > { > > struct platform_device *dev; > > - int rc, i, num_reg = 0, num_irq; > > + int rc, i, num_reg = 0, num_irq = 0; > > struct resource *res, temp_res; > > > > dev = platform_device_alloc("", PLATFORM_DEVID_NONE); > > @@ -124,7 +124,14 @@ struct platform_device *of_device_alloc(struct device_node *np, > > /* count the io and irq resources */ > > while (of_address_to_resource(np, num_reg, &temp_res) == 0) > > num_reg++; > > - num_irq = of_irq_count(np); > > + > > + /* > > + * we don't want to map the interrupts of hierarchical interrupt domain > > + * into the parent domain yet. This will be the job of the hierarchical > > + * interrupt driver code to map the interrupts as and when needed. > > + */ > > + if (!of_property_read_bool(np, "not-interrupt-producer")) > > + num_irq = of_irq_count(np); > > The property won't fly for sure. A compatible match table could work > here, but I don't really want another temporary solution. > Agreed. > > /* Populate the resource table */ > > if (num_irq || num_reg) { > > @@ -140,7 +147,7 @@ struct platform_device *of_device_alloc(struct device_node *np, > > rc = of_address_to_resource(np, i, res); > > WARN_ON(rc); > > } > > - if (of_irq_to_resource_table(np, res, num_irq) != num_irq) > > + if (num_irq && of_irq_to_resource_table(np, res, num_irq) != num_irq) > > You might want to look at commit 9ec36cafe43b ("of/irq: do irq > resolution in platform_get_irq"). The intent was to remove this code, > but looks like the cleanup has a ways to go 7 years on. Primarily, > it's convert any platform_get_resource(pdev, IORESOURCE_IRQ, n) call > to platform_get_irq(). There's ~169 of those. > That's a good point converting all the drivers to use platform_get_irq() so that the resource allocation happens on demand, and the above code can be dropped. > There are probably some open coded accesses to pdev->resources too, > but I didn't spot any. > I'll have a closer look. Cheers, Prabhakar > Rob
Hi Marc, Thank you for the review. On Thu, Dec 9, 2021 at 8:07 AM Marc Zyngier <maz@kernel.org> wrote: > > On 2021-12-09 00:10, Lad Prabhakar wrote: > > of_device_alloc() in early boot stage creates a interrupt mapping if > > there exists a "interrupts" property in the node. > > > > For hierarchical interrupt domains using "interrupts" property in the > > node > > bypassed the hierarchical setup and messed up the irq setup. > > > > This patch adds a check in of_device_alloc() to skip interrupt mapping > > if > > "not-interrupt-producer" property is present in the node. This allows > > nodes to describe the interrupts using "interrupts" property. > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > --- > > Hi All, > > > > Spawning from discussion [1], here is simple patch (not the ideal > > probably > > welcome for suggestions) from stopping the OF code from creating a map > > for > > the interrupts when using "interrupts" property. > > > > [1] https://lore.kernel.org/lkml/87pmqrck2m.wl-maz@kernel.org/ > > T/#mbd1e47c1981082aded4b32a52e2c04291e515508 > > > > Cheers, > > Prabhakar > > --- > > drivers/of/platform.c | 13 ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > > index b3faf89744aa..629776ca1721 100644 > > --- a/drivers/of/platform.c > > +++ b/drivers/of/platform.c > > @@ -114,7 +114,7 @@ struct platform_device *of_device_alloc(struct > > device_node *np, > > struct device *parent) > > { > > struct platform_device *dev; > > - int rc, i, num_reg = 0, num_irq; > > + int rc, i, num_reg = 0, num_irq = 0; > > struct resource *res, temp_res; > > > > dev = platform_device_alloc("", PLATFORM_DEVID_NONE); > > @@ -124,7 +124,14 @@ struct platform_device *of_device_alloc(struct > > device_node *np, > > /* count the io and irq resources */ > > while (of_address_to_resource(np, num_reg, &temp_res) == 0) > > num_reg++; > > - num_irq = of_irq_count(np); > > + > > + /* > > + * we don't want to map the interrupts of hierarchical interrupt > > domain > > + * into the parent domain yet. This will be the job of the > > hierarchical > > + * interrupt driver code to map the interrupts as and when needed. > > + */ > > + if (!of_property_read_bool(np, "not-interrupt-producer")) > > I don't think this is right. If anything, the expected behaviour should > be > indicated by the driver and not the device node. > > > + num_irq = of_irq_count(np); > > > > /* Populate the resource table */ > > if (num_irq || num_reg) { > > @@ -140,7 +147,7 @@ struct platform_device *of_device_alloc(struct > > device_node *np, > > rc = of_address_to_resource(np, i, res); > > WARN_ON(rc); > > } > > - if (of_irq_to_resource_table(np, res, num_irq) != num_irq) > > + if (num_irq && of_irq_to_resource_table(np, res, num_irq) != > > num_irq) > > pr_debug("not all legacy IRQ resources mapped for %pOFn\n", > > np); > > } > > The root of the issue is that all the resource allocation is done > upfront, > way before we even have a driver that could potentially deal with this > device. This is a potential waste of resource, and it triggers the > issue you noticed. > > If you delay the resource allocation until there is an actual match with > a > driver, you could have a per-driver flag telling you whether the IRQ > allocation should be performed before the probe() function is called. > As suggested by Rob, if we switch the drivers to use platform_get_resource(pdev, IORESOURCE_IRQ, n) call with platform_get_irq() this code should go away and with this switch the resource allocation will happen demand. Is this approach OK? Cheers, Prabhakar > M. > -- > Jazz is not dead. It just smells funny...
On Thu, 09 Dec 2021 10:00:44 +0000, "Lad, Prabhakar" <prabhakar.csengg@gmail.com> wrote: > > > The root of the issue is that all the resource allocation is done > > upfront, way before we even have a driver that could potentially > > deal with this device. This is a potential waste of resource, and > > it triggers the issue you noticed. > > > > If you delay the resource allocation until there is an actual > > match with a driver, you could have a per-driver flag telling you > > whether the IRQ allocation should be performed before the probe() > > function is called. > > > As suggested by Rob, if we switch the drivers to use > platform_get_resource(pdev, IORESOURCE_IRQ, n) call with > platform_get_irq() this code should go away and with this switch the > resource allocation will happen demand. Is this approach OK? If you get rid of of_irq_to_resource_table() altogether, then yes, this has a fighting chance to work. M.
Hi Rob and Marc, On Thu, Dec 9, 2021 at 10:33 AM Marc Zyngier <maz@kernel.org> wrote: > > On Thu, 09 Dec 2021 10:00:44 +0000, > "Lad, Prabhakar" <prabhakar.csengg@gmail.com> wrote: > > > > > The root of the issue is that all the resource allocation is done > > > upfront, way before we even have a driver that could potentially > > > deal with this device. This is a potential waste of resource, and > > > it triggers the issue you noticed. > > > > > > If you delay the resource allocation until there is an actual > > > match with a driver, you could have a per-driver flag telling you > > > whether the IRQ allocation should be performed before the probe() > > > function is called. > > > > > As suggested by Rob, if we switch the drivers to use > > platform_get_resource(pdev, IORESOURCE_IRQ, n) call with > > platform_get_irq() this code should go away and with this switch the > > resource allocation will happen demand. Is this approach OK? > > If you get rid of of_irq_to_resource_table() altogether, then yes, > this has a fighting chance to work. > Yes, switching to platform_get_irq() will eventually cause of_irq_to_resource_table() to go away. On second thought, instead of touching all the drivers, if we update platform_get_resource/platform_get_resource_byname to internally call platform_get_irq() internally if it's a IORESOURCE_IRQ resource. Does that sound good or should I just get on changing all the drivers to use platform_get_irq() instead? Cheers, Prabhakar > M. > > > -- > Without deviation from the norm, progress is not possible.
On Thu, Dec 9, 2021 at 5:35 AM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > > Hi Rob and Marc, > > On Thu, Dec 9, 2021 at 10:33 AM Marc Zyngier <maz@kernel.org> wrote: > > > > On Thu, 09 Dec 2021 10:00:44 +0000, > > "Lad, Prabhakar" <prabhakar.csengg@gmail.com> wrote: > > > > > > > The root of the issue is that all the resource allocation is done > > > > upfront, way before we even have a driver that could potentially > > > > deal with this device. This is a potential waste of resource, and > > > > it triggers the issue you noticed. > > > > > > > > If you delay the resource allocation until there is an actual > > > > match with a driver, you could have a per-driver flag telling you > > > > whether the IRQ allocation should be performed before the probe() > > > > function is called. > > > > > > > As suggested by Rob, if we switch the drivers to use > > > platform_get_resource(pdev, IORESOURCE_IRQ, n) call with > > > platform_get_irq() this code should go away and with this switch the > > > resource allocation will happen demand. Is this approach OK? > > > > If you get rid of of_irq_to_resource_table() altogether, then yes, > > this has a fighting chance to work. > > > Yes, switching to platform_get_irq() will eventually cause > of_irq_to_resource_table() to go away. > > On second thought, instead of touching all the drivers, if we update > platform_get_resource/platform_get_resource_byname to internally call > platform_get_irq() internally if it's a IORESOURCE_IRQ resource. Does > that sound good or should I just get on changing all the drivers to > use platform_get_irq() instead? Except that platform_get_irq() already internally calls platform_get_resource()... I think changing the drivers is the right way. Happy to do some if you want to divide it up. Using coccigrep, I think I've found all the places using platform_device.resource directly. A large swath are Sparc drivers which don't matter. The few that do matter I've prepared patches for here[1]. Most of what I found were DT based drivers that copy resources to a child platform device. That case will not work with platform_get_irq() callers either unless the child device has it's DT node set to the parent node which is the change I made. Rob [1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-kernelci
On Thu, Dec 9, 2021 at 8:34 PM Rob Herring <robh+dt@kernel.org> wrote: > > On Thu, Dec 9, 2021 at 5:35 AM Lad, Prabhakar > <prabhakar.csengg@gmail.com> wrote: > > > > Hi Rob and Marc, > > > > On Thu, Dec 9, 2021 at 10:33 AM Marc Zyngier <maz@kernel.org> wrote: > > > > > > On Thu, 09 Dec 2021 10:00:44 +0000, > > > "Lad, Prabhakar" <prabhakar.csengg@gmail.com> wrote: > > > > > > > > > The root of the issue is that all the resource allocation is done > > > > > upfront, way before we even have a driver that could potentially > > > > > deal with this device. This is a potential waste of resource, and > > > > > it triggers the issue you noticed. > > > > > > > > > > If you delay the resource allocation until there is an actual > > > > > match with a driver, you could have a per-driver flag telling you > > > > > whether the IRQ allocation should be performed before the probe() > > > > > function is called. > > > > > > > > > As suggested by Rob, if we switch the drivers to use > > > > platform_get_resource(pdev, IORESOURCE_IRQ, n) call with > > > > platform_get_irq() this code should go away and with this switch the > > > > resource allocation will happen demand. Is this approach OK? > > > > > > If you get rid of of_irq_to_resource_table() altogether, then yes, > > > this has a fighting chance to work. > > > > > Yes, switching to platform_get_irq() will eventually cause > > of_irq_to_resource_table() to go away. > > > > On second thought, instead of touching all the drivers, if we update > > platform_get_resource/platform_get_resource_byname to internally call > > platform_get_irq() internally if it's a IORESOURCE_IRQ resource. Does > > that sound good or should I just get on changing all the drivers to > > use platform_get_irq() instead? > > Except that platform_get_irq() already internally calls > platform_get_resource()... I think changing the drivers is the right > way. Happy to do some if you want to divide it up. > Thank you, I think I'll manage. > Using coccigrep, I think I've found all the places using > platform_device.resource directly. A large swath are Sparc drivers > which don't matter. The few that do matter I've prepared patches for > here[1]. Most of what I found were DT based drivers that copy > resources to a child platform device. That case will not work with > platform_get_irq() callers either unless the child device has it's DT > node set to the parent node which is the change I made. > Thank you for getting this done. Do you want me to include those along with my conversion patches? Any reason why we dont care for Sparc drivers? > Rob > > [1] git://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-kernelci Cheers, Prabhakar
On Thu, Dec 9, 2021 at 7:16 PM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > > On Thu, Dec 9, 2021 at 8:34 PM Rob Herring <robh+dt@kernel.org> wrote: > > > > On Thu, Dec 9, 2021 at 5:35 AM Lad, Prabhakar > > <prabhakar.csengg@gmail.com> wrote: > > > > > > Hi Rob and Marc, > > > > > > On Thu, Dec 9, 2021 at 10:33 AM Marc Zyngier <maz@kernel.org> wrote: > > > > > > > > On Thu, 09 Dec 2021 10:00:44 +0000, > > > > "Lad, Prabhakar" <prabhakar.csengg@gmail.com> wrote: > > > > > > > > > > > The root of the issue is that all the resource allocation is done > > > > > > upfront, way before we even have a driver that could potentially > > > > > > deal with this device. This is a potential waste of resource, and > > > > > > it triggers the issue you noticed. > > > > > > > > > > > > If you delay the resource allocation until there is an actual > > > > > > match with a driver, you could have a per-driver flag telling you > > > > > > whether the IRQ allocation should be performed before the probe() > > > > > > function is called. > > > > > > > > > > > As suggested by Rob, if we switch the drivers to use > > > > > platform_get_resource(pdev, IORESOURCE_IRQ, n) call with > > > > > platform_get_irq() this code should go away and with this switch the > > > > > resource allocation will happen demand. Is this approach OK? > > > > > > > > If you get rid of of_irq_to_resource_table() altogether, then yes, > > > > this has a fighting chance to work. > > > > > > > Yes, switching to platform_get_irq() will eventually cause > > > of_irq_to_resource_table() to go away. > > > > > > On second thought, instead of touching all the drivers, if we update > > > platform_get_resource/platform_get_resource_byname to internally call > > > platform_get_irq() internally if it's a IORESOURCE_IRQ resource. Does > > > that sound good or should I just get on changing all the drivers to > > > use platform_get_irq() instead? > > > > Except that platform_get_irq() already internally calls > > platform_get_resource()... I think changing the drivers is the right > > way. Happy to do some if you want to divide it up. > > > Thank you, I think I'll manage. > > > Using coccigrep, I think I've found all the places using > > platform_device.resource directly. A large swath are Sparc drivers > > which don't matter. The few that do matter I've prepared patches for > > here[1]. Most of what I found were DT based drivers that copy > > resources to a child platform device. That case will not work with > > platform_get_irq() callers either unless the child device has it's DT > > node set to the parent node which is the change I made. > > > Thank you for getting this done. Do you want me to include those along > with my conversion patches? No, I'll send them out. > Any reason why we dont care for Sparc drivers? Sparc does its own thing and doesn't use drivers/of/platform.c to create devices. I'm sure we could modernize a bunch of them, but that's not a blocker. Rob
Hi Marc, On Thu, Dec 9, 2021 at 10:33 AM Marc Zyngier <maz@kernel.org> wrote: > > On Thu, 09 Dec 2021 10:00:44 +0000, > "Lad, Prabhakar" <prabhakar.csengg@gmail.com> wrote: > > > > > The root of the issue is that all the resource allocation is done > > > upfront, way before we even have a driver that could potentially > > > deal with this device. This is a potential waste of resource, and > > > it triggers the issue you noticed. > > > > > > If you delay the resource allocation until there is an actual > > > match with a driver, you could have a per-driver flag telling you > > > whether the IRQ allocation should be performed before the probe() > > > function is called. > > > > > As suggested by Rob, if we switch the drivers to use > > platform_get_resource(pdev, IORESOURCE_IRQ, n) call with > > platform_get_irq() this code should go away and with this switch the > > resource allocation will happen demand. Is this approach OK? > > If you get rid of of_irq_to_resource_table() altogether, then yes, > this has a fighting chance to work. > To clarify, did you mean to get rid of_irq_to_resource_table() completely or just from the drivers/of/platform.c ([0])? [0] https://elixir.bootlin.com/linux/latest/source/drivers/of/platform.c#L143 Cheers, Prabhakar
diff --git a/drivers/of/platform.c b/drivers/of/platform.c index b3faf89744aa..629776ca1721 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -114,7 +114,7 @@ struct platform_device *of_device_alloc(struct device_node *np, struct device *parent) { struct platform_device *dev; - int rc, i, num_reg = 0, num_irq; + int rc, i, num_reg = 0, num_irq = 0; struct resource *res, temp_res; dev = platform_device_alloc("", PLATFORM_DEVID_NONE); @@ -124,7 +124,14 @@ struct platform_device *of_device_alloc(struct device_node *np, /* count the io and irq resources */ while (of_address_to_resource(np, num_reg, &temp_res) == 0) num_reg++; - num_irq = of_irq_count(np); + + /* + * we don't want to map the interrupts of hierarchical interrupt domain + * into the parent domain yet. This will be the job of the hierarchical + * interrupt driver code to map the interrupts as and when needed. + */ + if (!of_property_read_bool(np, "not-interrupt-producer")) + num_irq = of_irq_count(np); /* Populate the resource table */ if (num_irq || num_reg) { @@ -140,7 +147,7 @@ struct platform_device *of_device_alloc(struct device_node *np, rc = of_address_to_resource(np, i, res); WARN_ON(rc); } - if (of_irq_to_resource_table(np, res, num_irq) != num_irq) + if (num_irq && of_irq_to_resource_table(np, res, num_irq) != num_irq) pr_debug("not all legacy IRQ resources mapped for %pOFn\n", np); }
of_device_alloc() in early boot stage creates a interrupt mapping if there exists a "interrupts" property in the node. For hierarchical interrupt domains using "interrupts" property in the node bypassed the hierarchical setup and messed up the irq setup. This patch adds a check in of_device_alloc() to skip interrupt mapping if "not-interrupt-producer" property is present in the node. This allows nodes to describe the interrupts using "interrupts" property. Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- Hi All, Spawning from discussion [1], here is simple patch (not the ideal probably welcome for suggestions) from stopping the OF code from creating a map for the interrupts when using "interrupts" property. [1] https://lore.kernel.org/lkml/87pmqrck2m.wl-maz@kernel.org/ T/#mbd1e47c1981082aded4b32a52e2c04291e515508 Cheers, Prabhakar --- drivers/of/platform.c | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-)