Patchwork [v3] of/irq: Get rid of NO_IRQ usage

login
register
mail settings
Submitter Anton Vorontsov
Date Dec. 6, 2011, 11:16 p.m.
Message ID <20111206231626.GA31683@oksana.dev.rtsoft.ru>
Download mbox | patch
Permalink /patch/129849/
State Not Applicable
Delegated to: David Miller
Headers show

Comments

Anton Vorontsov - Dec. 6, 2011, 11:16 p.m.
PPC32/64 defines NO_IRQ to zero, so no problems expected.
ARM defines NO_IRQ to -1, but OF code relies on IRQ domains support,
which returns correct ('0') value in 'no irq' case. So everything
should be fine.

Other arches might break if some of their OF drivers rely on NO_IRQ
being not 0. If so, the drivers must be fixed, finally.

Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
---

On Tue, Dec 06, 2011 at 01:25:07PM -0800, Linus Torvalds wrote:
> On Tue, Dec 6, 2011 at 1:22 PM, Rob Herring <robherring2@gmail.com> wrote:
> >
> > This warning code is really ugly. Can we just drop it? In my searching
> > of in kernel dts files, there's only 1 instance I have found (Versatile
> > AB watchdog) that would hit this.
> 
> I do agree. Especially since we never got any input on whether it works or not.
> 
> > If not, you don't need to handle irq_create_of_mapping return as that is
> > already always 0 for no irq or error.
> 
> Yeah, I'd like to just remove NO_IRQ from the OF paths. Afaik, it
> hasn't worked as NO_IRQ before anyway, so even if the rest of the
> drivers end up continuing using NO_IRQ, the OF-enabled ones on ARM
> should just stop. There can't be many of those yet.

Okay, here it goes.

 drivers/of/irq.c |   13 ++++---------
 1 files changed, 4 insertions(+), 9 deletions(-)
Rob Herring - Dec. 7, 2011, 3:51 a.m.
On 12/06/2011 05:16 PM, Anton Vorontsov wrote:
> PPC32/64 defines NO_IRQ to zero, so no problems expected.
> ARM defines NO_IRQ to -1, but OF code relies on IRQ domains support,
> which returns correct ('0') value in 'no irq' case. So everything
> should be fine.
> 
> Other arches might break if some of their OF drivers rely on NO_IRQ
> being not 0. If so, the drivers must be fixed, finally.
> 
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
> ---

Looks fine, but I think we have to wait for microblaze to be fixed or
some confirmation from microblaze folks that no driver would use IRQ0.
There was a patch posted by Grant in back in Feb, but nothing has
happened since:

http://article.gmane.org/gmane.linux.uclinux.microblaze/10121/

Rob

> 
> On Tue, Dec 06, 2011 at 01:25:07PM -0800, Linus Torvalds wrote:
>> On Tue, Dec 6, 2011 at 1:22 PM, Rob Herring <robherring2@gmail.com> wrote:
>>>
>>> This warning code is really ugly. Can we just drop it? In my searching
>>> of in kernel dts files, there's only 1 instance I have found (Versatile
>>> AB watchdog) that would hit this.
>>
>> I do agree. Especially since we never got any input on whether it works or not.
>>
>>> If not, you don't need to handle irq_create_of_mapping return as that is
>>> already always 0 for no irq or error.
>>
>> Yeah, I'd like to just remove NO_IRQ from the OF paths. Afaik, it
>> hasn't worked as NO_IRQ before anyway, so even if the rest of the
>> drivers end up continuing using NO_IRQ, the OF-enabled ones on ARM
>> should just stop. There can't be many of those yet.
> 
> Okay, here it goes.
> 
>  drivers/of/irq.c |   13 ++++---------
>  1 files changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/of/irq.c b/drivers/of/irq.c
> index 791270b..ac6da0d 100644
> --- a/drivers/of/irq.c
> +++ b/drivers/of/irq.c
> @@ -26,11 +26,6 @@
>  #include <linux/string.h>
>  #include <linux/slab.h>
>  
> -/* For archs that don't support NO_IRQ (such as x86), provide a dummy value */
> -#ifndef NO_IRQ
> -#define NO_IRQ 0
> -#endif
> -
>  /**
>   * irq_of_parse_and_map - Parse and map an interrupt into linux virq space
>   * @device: Device node of the device whose interrupt is to be mapped
> @@ -44,7 +39,7 @@ unsigned int irq_of_parse_and_map(struct device_node *dev, int index)
>  	struct of_irq oirq;
>  
>  	if (of_irq_map_one(dev, index, &oirq))
> -		return NO_IRQ;
> +		return 0;
>  
>  	return irq_create_of_mapping(oirq.controller, oirq.specifier,
>  				     oirq.size);
> @@ -345,7 +340,7 @@ int of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
>  
>  	/* Only dereference the resource if both the
>  	 * resource and the irq are valid. */
> -	if (r && irq != NO_IRQ) {
> +	if (r && irq) {
>  		r->start = r->end = irq;
>  		r->flags = IORESOURCE_IRQ;
>  		r->name = dev->full_name;
> @@ -363,7 +358,7 @@ int of_irq_count(struct device_node *dev)
>  {
>  	int nr = 0;
>  
> -	while (of_irq_to_resource(dev, nr, NULL) != NO_IRQ)
> +	while (of_irq_to_resource(dev, nr, NULL))
>  		nr++;
>  
>  	return nr;
> @@ -383,7 +378,7 @@ int of_irq_to_resource_table(struct device_node *dev, struct resource *res,
>  	int i;
>  
>  	for (i = 0; i < nr_irqs; i++, res++)
> -		if (of_irq_to_resource(dev, i, res) == NO_IRQ)
> +		if (!of_irq_to_resource(dev, i, res))
>  			break;
>  
>  	return i;

--
To unsubscribe from this list: send the line "unsubscribe linux-ide" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Wolfram Sang - Dec. 7, 2011, 9:52 a.m.
On Wed, Dec 07, 2011 at 03:16:26AM +0400, Anton Vorontsov wrote:
> PPC32/64 defines NO_IRQ to zero, so no problems expected.
> ARM defines NO_IRQ to -1, but OF code relies on IRQ domains support,
> which returns correct ('0') value in 'no irq' case. So everything
> should be fine.
> 
> Other arches might break if some of their OF drivers rely on NO_IRQ
> being not 0. If so, the drivers must be fixed, finally.
> 
> Signed-off-by: Anton Vorontsov <anton.vorontsov@linaro.org>
> ---

/me likes NO_IRQ removal very much

Acked-by: Wolfram Sang <w.sang@pengutronix.de>

Patch

diff --git a/drivers/of/irq.c b/drivers/of/irq.c
index 791270b..ac6da0d 100644
--- a/drivers/of/irq.c
+++ b/drivers/of/irq.c
@@ -26,11 +26,6 @@ 
 #include <linux/string.h>
 #include <linux/slab.h>
 
-/* For archs that don't support NO_IRQ (such as x86), provide a dummy value */
-#ifndef NO_IRQ
-#define NO_IRQ 0
-#endif
-
 /**
  * irq_of_parse_and_map - Parse and map an interrupt into linux virq space
  * @device: Device node of the device whose interrupt is to be mapped
@@ -44,7 +39,7 @@  unsigned int irq_of_parse_and_map(struct device_node *dev, int index)
 	struct of_irq oirq;
 
 	if (of_irq_map_one(dev, index, &oirq))
-		return NO_IRQ;
+		return 0;
 
 	return irq_create_of_mapping(oirq.controller, oirq.specifier,
 				     oirq.size);
@@ -345,7 +340,7 @@  int of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
 
 	/* Only dereference the resource if both the
 	 * resource and the irq are valid. */
-	if (r && irq != NO_IRQ) {
+	if (r && irq) {
 		r->start = r->end = irq;
 		r->flags = IORESOURCE_IRQ;
 		r->name = dev->full_name;
@@ -363,7 +358,7 @@  int of_irq_count(struct device_node *dev)
 {
 	int nr = 0;
 
-	while (of_irq_to_resource(dev, nr, NULL) != NO_IRQ)
+	while (of_irq_to_resource(dev, nr, NULL))
 		nr++;
 
 	return nr;
@@ -383,7 +378,7 @@  int of_irq_to_resource_table(struct device_node *dev, struct resource *res,
 	int i;
 
 	for (i = 0; i < nr_irqs; i++, res++)
-		if (of_irq_to_resource(dev, i, res) == NO_IRQ)
+		if (!of_irq_to_resource(dev, i, res))
 			break;
 
 	return i;