diff mbox series

[RFC] of/platform: Drop static setup of IRQ resource from DT core

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

Commit Message

Prabhakar Mahadev Lad March 16, 2022, 8:06 p.m. UTC
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(-)

Comments

Marc Zyngier March 17, 2022, 12:25 p.m. UTC | #1
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.
Rob Herring April 1, 2022, 1:06 p.m. UTC | #2
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 mbox series

Patch

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);