[PATCH] of_irq_to_resource now returns the virq

[PATCH] of_irq_to_resource now returns the virq

[Andy Fleming]

Mostly this is to allow for error checking (check the return for NO_IRQ)

Signed-off-by: Andrew Fleming <afleming@freescale.com>
---
For 2.6.20

 include/asm-powerpc/prom.h |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/include/asm-powerpc/prom.h b/include/asm-powerpc/prom.h
index 933ba27..e9226e3 100644
--- a/include/asm-powerpc/prom.h
+++ b/include/asm-powerpc/prom.h
@@ -334,10 +334,17 @@ extern int of_irq_map_one(struct device_
 struct pci_dev;
 extern int of_irq_map_pci(struct pci_dev *pdev, struct of_irq *out_irq);
 
-static inline void of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
+static inline int of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
 {
-	r->start = r->end = irq_of_parse_and_map(dev, index);
-	r->flags = IORESOURCE_IRQ;
+	int irq = irq_of_parse_and_map(dev, index);
+
+	/* Only dereference the resource if the irq is valid. */
+	if (irq != NO_IRQ) {
+		r->start = r->end = irq;
+		r->flags = IORESOURCE_IRQ;
+	}
+
+	return irq;
 }
 
 
-- 
1.4.2.3

Re: [PATCH] of_irq_to_resource now returns the virq

[Benjamin Herrenschmidt]

On Mon, 2006-11-06 at 17:13 -0600, Andy Fleming wrote:
> Mostly this is to allow for error checking (check the return for NO_IRQ)
> 
> Signed-off-by: Andrew Fleming <afleming@freescale.com>
> ---
> For 2.6.20
> 
>  include/asm-powerpc/prom.h |   13 ++++++++++---
>  1 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/include/asm-powerpc/prom.h b/include/asm-powerpc/prom.h
> index 933ba27..e9226e3 100644
> --- a/include/asm-powerpc/prom.h
> +++ b/include/asm-powerpc/prom.h
> @@ -334,10 +334,17 @@ extern int of_irq_map_one(struct device_
>  struct pci_dev;
>  extern int of_irq_map_pci(struct pci_dev *pdev, struct of_irq *out_irq);
>  
> -static inline void of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
> +static inline int of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
>  {
> -	r->start = r->end = irq_of_parse_and_map(dev, index);
> -	r->flags = IORESOURCE_IRQ;
> +	int irq = irq_of_parse_and_map(dev, index);
> +
> +	/* Only dereference the resource if the irq is valid. */
> +	if (irq != NO_IRQ) {

What about if (irq != NO_IRQ && r) ?
 
> +		r->start = r->end = irq;
> +		r->flags = IORESOURCE_IRQ;
> +	}
> +
> +	return irq;
>  }

That way, if somebody doesn't care about the resource, just pass NULL,
though I suppose we should then also change the name of the function to
something like of_map_one_irq() ..

Ben.

Re: [PATCH] of_irq_to_resource now returns the virq

[Andy Fleming]

On Nov 6, 2006, at 18:04, Benjamin Herrenschmidt wrote:

> On Mon, 2006-11-06 at 17:13 -0600, Andy Fleming wrote:
>> Mostly this is to allow for error checking (check the return for  
>> NO_IRQ)
>>
>> Signed-off-by: Andrew Fleming <afleming@freescale.com>
>> ---
>> For 2.6.20
>>
>>  include/asm-powerpc/prom.h |   13 ++++++++++---
>>  1 files changed, 10 insertions(+), 3 deletions(-)
>>
>> diff --git a/include/asm-powerpc/prom.h b/include/asm-powerpc/prom.h
>> index 933ba27..e9226e3 100644
>> --- a/include/asm-powerpc/prom.h
>> +++ b/include/asm-powerpc/prom.h
>> @@ -334,10 +334,17 @@ extern int of_irq_map_one(struct device_
>>  struct pci_dev;
>>  extern int of_irq_map_pci(struct pci_dev *pdev, struct of_irq  
>> *out_irq);
>>
>> -static inline void of_irq_to_resource(struct device_node *dev,  
>> int index, struct resource *r)
>> +static inline int of_irq_to_resource(struct device_node *dev, int  
>> index, struct resource *r)
>>  {
>> -	r->start = r->end = irq_of_parse_and_map(dev, index);
>> -	r->flags = IORESOURCE_IRQ;
>> +	int irq = irq_of_parse_and_map(dev, index);
>> +
>> +	/* Only dereference the resource if the irq is valid. */
>> +	if (irq != NO_IRQ) {
>
> What about if (irq != NO_IRQ && r) ?

Yeah, that's a good idea.  The reason I didn't want to dereference r  
if the IRQ was invalid was originally because I wanted my gianfar  
setup code to do this:

	while (of_irq_to_resource(np, n_res, &r[++n_res]) != NO_IRQ);

That way, I didn't have to do bounds checking on the array, as long  
as I made sure that it could hold all the possible interrupts.  I  
eventually decided that was a bad idea (since I had to hard-code that  
anyway, but that of_irq_to_resource() might as well not fill in a  
resource if the IRQ is bad.  I'm about to submit the code that uses  
this function, but now it does bounds checking.

>
>> +		r->start = r->end = irq;
>> +		r->flags = IORESOURCE_IRQ;
>> +	}
>> +
>> +	return irq;
>>  }
>
> That way, if somebody doesn't care about the resource, just pass NULL,
> though I suppose we should then also change the name of the  
> function to
> something like of_map_one_irq() ..


Well, I think it would just be irq_of_parse_and_map(), then.  :)


>
> Ben.
>

[PATCH] of_irq_to_resource now returns the virq

[Andy Fleming]

Mostly this is to allow for error checking (check the return for NO_IRQ)
Added a check that the resource is non-NULL, too

Signed-off-by: Andrew Fleming <afleming@freescale.com>
---
 include/asm-powerpc/prom.h |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/include/asm-powerpc/prom.h b/include/asm-powerpc/prom.h
index 933ba27..0afee17 100644
--- a/include/asm-powerpc/prom.h
+++ b/include/asm-powerpc/prom.h
@@ -334,10 +334,18 @@ extern int of_irq_map_one(struct device_
 struct pci_dev;
 extern int of_irq_map_pci(struct pci_dev *pdev, struct of_irq *out_irq);
 
-static inline void of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
+static inline int of_irq_to_resource(struct device_node *dev, int index, struct resource *r)
 {
-	r->start = r->end = irq_of_parse_and_map(dev, index);
-	r->flags = IORESOURCE_IRQ;
+	int irq = irq_of_parse_and_map(dev, index);
+
+	/* Only dereference the resource if both the
+	 * resource and the irq are valid. */
+	if (r && irq != NO_IRQ) {
+		r->start = r->end = irq;
+		r->flags = IORESOURCE_IRQ;
+	}
+
+	return irq;
 }
 
 
-- 
1.4.2.3