Re: [PATCH 4/9] of/irq: Introduce of_irq_get()

[Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 1528 bytes --]

On Mon, Sep 16, 2013 at 04:24:47PM -0500, Rob Herring wrote:
> On 09/16/2013 03:32 AM, Thierry Reding wrote:
> > This is a version of irq_of_parse_and_map() that propagates the precise
> > error code instead of returning 0 for all errors. It will be used in
> > subsequent patches to allow further propagation of error codes.
> > 
> > To avoid code duplication, implement irq_of_parse_and_map() as a wrapper
> > around the new of_irq_get().
> 
> *_get typically also implies some reference counting which I don't
> believe this does. I don't think having 2 functions with completely
> different names doing the same thing with only a different calling
> convention is good either. So I would keep the old name and the names
> aligned.

Okay, I'll make the new function __irq_of_parse_and_map().

> >  unsigned int irq_of_parse_and_map(struct device_node *dev, int index)
> >  {
> > -	struct of_irq oirq;
> > +	unsigned int virq;
> >  
> > -	if (of_irq_map_one(dev, index, &oirq))
> > +	if (of_irq_get(dev, index, &virq))
> >  		return 0;
> >  
> > -	return irq_create_of_mapping(oirq.controller, oirq.specifier,
> > -				     oirq.size);
> > +	return virq;
> 
> This can be an inline and written more concisely:
> 
> {
> 	unsigned int virq;
> 	return (of_irq_get(dev, index, &virq) < 0) ? 0 : virq;
> }

I find code such as the above very hard to read. But if you insist I can
move the function into include/of/of_irq.h and make it static inline
with the implementation above.

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]