Re: [PATCH V2] IRQ handling by priority for IMX

[Russell King - ARM Linux <linux@xxxxxxxxxxxxxxxx>]

On Mon, Jul 21, 2008 at 12:13:54PM +0200, Sascha Hauer wrote:
> > +#ifdef CONFIG_IMX_IRQ_PRIOR
> > +void imx_irq_set_priority(unsigned char irq, unsigned char prio)
> > +{
> > +	unsigned int temp;
> > +
> > +	if (irq > 63)
> > +		return;
> > +
> > +	temp = __raw_readl(IMX_AITC_NIPRIORITY(irq / 8));
> > +	temp &= ~0x0F << irq%8*4;
> 
> There are parentheses missing. This should be ~(0xf << irq % 8 * 4)
> 
> > +	temp |= prio&0x0F << irq%8*4;
> 
> And this should be (prio & 0xf) << irq % 8 * 4

Even better would be to realise that the mask is calculated twice, so:

	unsigned int mask = 0x0f << irq % 8 * 4;
...
	temp &= ~mask;
	temp |= prio & mask;

results in clearer code, and avoids the possibility of typos in the
duplicated calculation of mask.

-------------------------------------------------------------------
List admin: http://lists.arm.linux.org.uk/mailman/listinfo/linux-arm-kernel
FAQ:        http://www.arm.linux.org.uk/mailinglists/faq.php
Etiquette:  http://www.arm.linux.org.uk/mailinglists/etiquette.php