Re: [PATCH v3] irqchip: Add support for ARMv7-M's NVIC

Arnd Bergmann
2013-04-17T20:23:49Z

On Wednesday 17 April 2013, Uwe Kleine-König wrote:

> This patch triggers two checkpatch warnings:
>
> WARNING: Avoid CamelCase: <nvic_do_IRQ>
> WARNING: Avoid CamelCase: <handle_IRQ>
>
> but I think they are OK for consistency?!

You obviously have no choice for handle_IRQ, but I think the common way to
name the first-level interrupt handler would be "nvic_handle_irq" here.

> Moreover sparse tells me:
>
> drivers/irqchip/irq-nvic.c:58:1: warning: symbol 'nvic_do_IRQ' was not declared. Should it be static?
>
> nvic_do_IRQ is called from assembler only, so a declaration couldn't be
> shared and I couldn't find a nice place for a declaration. Suggestions
> welcome.

Can't you make it static and call set_handle_irq() on it from the
probe function?

> + * Each bank handles 32 irqs. Only the 16th (= last) bank handles only
> + * 16 irqs.
> + */
> +#define NVIC_MAX_IRQ ((NVIC_MAX_BANKS - 1) * 32 + 16)

Is this actually inherent to the hardware design, or is the number of irqs
actually customizable? Also, why do you care about the maximum? You only
use it to check against the device tree provided value, but I suppose
you could just as well trust that property to be correct.

Arnd