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

[Sascha Hauer <s.hauer@xxxxxxxxxxxxxx>]

On Wed, Jul 09, 2008 at 02:18:45PM +0300, Darius wrote:
> All Sascha comments fixed. Checked and tested.
> Should I add this patch to Russell patch system?
>
> Signed-off-by: Darius Augulis <augulis.darius@xxxxxxxxx>
> ---

> Index: linux-2.6.26-rc9/arch/arm/mach-imx/Kconfig
> ===================================================================
> --- linux-2.6.26-rc9.orig/arch/arm/mach-imx/Kconfig
> +++ linux-2.6.26-rc9/arch/arm/mach-imx/Kconfig
> @@ -9,3 +9,16 @@ config ARCH_MX1ADS
>  	  Say Y here if you are using the Motorola MX1ADS board
>  
>  endmenu
> +
> +config IMX_IRQ_PRIOR
> +	bool "Use IRQ priority"
> +	depends on ARCH_IMX
> +	help
> +	  Select this if you want to use prioritized IRQ handling.
> +	  This feature prevents higher priority ISR to be interrupted
> +	  by lower priority IRQ even IRQF_DISABLED flag is not set.
> +	  This may be useful in embedded applications, where are strong
> +	  requirements for timing.
> +	  Most users must say N there.
> +	  Say Y only if you know what you are doing.
> +
> Index: linux-2.6.26-rc9/arch/arm/mach-imx/irq.c
> ===================================================================
> --- linux-2.6.26-rc9.orig/arch/arm/mach-imx/irq.c
> +++ linux-2.6.26-rc9/arch/arm/mach-imx/irq.c
> @@ -23,8 +23,8 @@
>   *               interrupt handler
>   */
>  
> +#include <linux/module.h>
>  #include <linux/init.h>
> -#include <linux/list.h>
>  #include <linux/timer.h>
>  
>  #include <asm/hardware.h>
> @@ -63,7 +63,7 @@
>  #define FIPNDH_OFF                0x60
>  #define FIPNDL_OFF                0x64
>  
> -#define VA_AITC_BASE              IO_ADDRESS(IMX_AITC_BASE)
> +#define VA_AITC_BASE             IO_ADDRESS(IMX_AITC_BASE)
>  #define IMX_AITC_INTCNTL         (VA_AITC_BASE + INTCNTL_OFF)
>  #define IMX_AITC_NIMASK          (VA_AITC_BASE + NIMASK_OFF)
>  #define IMX_AITC_INTENNUM        (VA_AITC_BASE + INTENNUM_OFF)
> @@ -250,6 +250,22 @@ static struct irq_chip imx_gpio_chip = {
>  	.set_type = imx_gpio_irq_type,
>  };
>  
> +#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

Operator precedence in C does not work by the count of spaces between
operator and operands.
And please read Documentation/CondingStyle, especially the the paragraph
about operators.

Sascha

-- 
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

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