Message ID | 20220316200633.28974-1-prabhakar.mahadev-lad.rj@bp.renesas.com |
---|---|
State | Accepted |
Commit | a1a2b7125e1079cfcc13a116aa3af3df2f9e002b |
Headers | show |
Series | [RFC] of/platform: Drop static setup of IRQ resource from DT core | expand |
On Wed, 16 Mar 2022 20:06:33 +0000, Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> wrote: > > Now that all the DT drivers have switched to platform_get_irq() we can now > safely drop the static setup of IRQ resource from DT core code. > > With the above change hierarchical setup of irq domains is no longer > bypassed and thus allowing hierarchical interrupt domains to describe > interrupts using "interrupts" DT property. > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > --- > Hi All, > > Sending this as RFC as couple of more drivers need to hit -rc yet with > the platform_get_irq() change while that is in progress I wanted to get > some feedback on this patch. > > Cheers, > Prabhakar > --- > drivers/of/platform.c | 14 +++++--------- > 1 file changed, 5 insertions(+), 9 deletions(-) > > diff --git a/drivers/of/platform.c b/drivers/of/platform.c > index 793350028906..6890f7fe556f 100644 > --- a/drivers/of/platform.c > +++ b/drivers/of/platform.c > @@ -114,35 +114,31 @@ 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; > struct resource *res, temp_res; > > dev = platform_device_alloc("", PLATFORM_DEVID_NONE); > if (!dev) > return NULL; > > - /* count the io and irq resources */ > + /* count the io resources */ > while (of_address_to_resource(np, num_reg, &temp_res) == 0) > num_reg++; > - num_irq = of_irq_count(np); > > /* Populate the resource table */ > - if (num_irq || num_reg) { > - res = kcalloc(num_irq + num_reg, sizeof(*res), GFP_KERNEL); > + if (num_reg) { > + res = kcalloc(num_reg, sizeof(*res), GFP_KERNEL); > if (!res) { > platform_device_put(dev); > return NULL; > } > > - dev->num_resources = num_reg + num_irq; > + dev->num_resources = num_reg; > dev->resource = res; > for (i = 0; i < num_reg; i++, res++) { > rc = of_address_to_resource(np, i, res); > WARN_ON(rc); > } > - if (of_irq_to_resource_table(np, res, num_irq) != num_irq) > - pr_debug("not all legacy IRQ resources mapped for %pOFn\n", > - np); > } > > dev->dev.of_node = of_node_get(np); I think this definitely goes in the right direction by not eagerly populating resources without a driver actually needing it. If anything breaks, that should be seen as an opportunity to fix the users of this misfeature. I booted a couple of boxes with this patch, and nothing caught fire, so: Acked-by: Marc Zyngier <maz@kernel.org> Tested-by: Marc Zyngier <maz@kernel.org> M.
On Fri, Apr 1, 2022 at 2:42 AM Lad, Prabhakar <prabhakar.csengg@gmail.com> wrote: > > Hi Rob, > > On Thu, Mar 31, 2022 at 10:02 PM Rob Herring <robh@kernel.org> wrote: > > > > On Wed, Mar 16, 2022 at 08:06:33PM +0000, Lad Prabhakar wrote: > > > Now that all the DT drivers have switched to platform_get_irq() we can now > > > safely drop the static setup of IRQ resource from DT core code. > > > > > > With the above change hierarchical setup of irq domains is no longer > > > bypassed and thus allowing hierarchical interrupt domains to describe > > > interrupts using "interrupts" DT property. > > > > > > Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> > > > --- > > > Hi All, > > > > > > Sending this as RFC as couple of more drivers need to hit -rc yet with > > > the platform_get_irq() change while that is in progress I wanted to get > > > some feedback on this patch. > > > > I just applied this on top of current master and pushed to my > > for-kernelci branch. It should show up in kernelCI in a bit. I did this > > before all the fixes too and there were definitely a couple of test > > regressions. > > > Any chance you can share the regressions or maybe the CI script so > that I can reproduce it locally. It will show up here[1] as 'robh', but it still hasn't built yet which is strange. Rob [1] https://linux.kernelci.org/job/
diff --git a/drivers/of/platform.c b/drivers/of/platform.c index 793350028906..6890f7fe556f 100644 --- a/drivers/of/platform.c +++ b/drivers/of/platform.c @@ -114,35 +114,31 @@ 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; struct resource *res, temp_res; dev = platform_device_alloc("", PLATFORM_DEVID_NONE); if (!dev) return NULL; - /* count the io and irq resources */ + /* count the io resources */ while (of_address_to_resource(np, num_reg, &temp_res) == 0) num_reg++; - num_irq = of_irq_count(np); /* Populate the resource table */ - if (num_irq || num_reg) { - res = kcalloc(num_irq + num_reg, sizeof(*res), GFP_KERNEL); + if (num_reg) { + res = kcalloc(num_reg, sizeof(*res), GFP_KERNEL); if (!res) { platform_device_put(dev); return NULL; } - dev->num_resources = num_reg + num_irq; + dev->num_resources = num_reg; dev->resource = res; for (i = 0; i < num_reg; i++, res++) { rc = of_address_to_resource(np, i, res); WARN_ON(rc); } - if (of_irq_to_resource_table(np, res, num_irq) != num_irq) - pr_debug("not all legacy IRQ resources mapped for %pOFn\n", - np); } dev->dev.of_node = of_node_get(np);
Now that all the DT drivers have switched to platform_get_irq() we can now safely drop the static setup of IRQ resource from DT core code. With the above change hierarchical setup of irq domains is no longer bypassed and thus allowing hierarchical interrupt domains to describe interrupts using "interrupts" DT property. Signed-off-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com> --- Hi All, Sending this as RFC as couple of more drivers need to hit -rc yet with the platform_get_irq() change while that is in progress I wanted to get some feedback on this patch. Cheers, Prabhakar --- drivers/of/platform.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-)