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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201304221404.31281.arnd@arndb.de>
Date:	Mon, 22 Apr 2013 14:04:31 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	"Uwe Kleine-König" 
	<u.kleine-koenig@...gutronix.de>
Cc:	Thomas Gleixner <tglx@...utronix.de>, kernel@...gutronix.de,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	Jonathan Austin <jonathan.austin@....com>,
	Catalin Marinas <catalin.marinas@....com>,
	Benjamin Herrenschmidt <benh@...nel.crashing.org>,
	Grant Likely <grant.likely@...aro.org>,
	Paul Mundt <lethal@...ux-sh.org>, linux-sh@...r.kernel.org,
	Tomasz Figa <tomasz.figa@...il.com>
Subject: Re: [PATCH v3] irqchip: Add support for ARMv7-M's NVIC

On Monday 22 April 2013, Uwe Kleine-König wrote:
> Hello,
> 
> (for the new readers of this thread: This is about using
> 
> 	u32 mask = 1 << (d->hwirq % 32);
> 
> instead of
> 
> 	u32 mask = 1 << (d->irq - gc->irq_base);

While not technically any different, I would suggest writing the above
as (d->hwirq & 0x1f), which makes it clear to the reader that this is
intended to be a cheap bit mask operation rather than an expensive
division.
> 
> arch/arm/mach-s5p64x0/common.c
> 	-> uses irq_base=101 for irq_alloc_generic_chip

I think there are plans to replace this code with
drivers/pinctrl/pinctrl-samsung.c, but I don't know if patches
exist already.

> arch/arm/plat-orion/gpio.c
> 	-> depends on how orion_gpio_of_init is called. No callers
> 	found.

As of f9e7592230b, this code has been replaced with a pinctrl driver
and can be killed.

> arch/arm/plat-orion/irq.c
> arch/arm/plat-samsung/irq-vic-timer.c
> 	-> used for a single irq that isn't a multiple of 32

Tomasz Figa has a patch set to remove this file, will likely get merged
in 3.11.

> arch/arm/plat-samsung/s5p-irq-gpioint.c
> 	-> would need % 8?

AFAICT this code is the same driver as arch/arm/mach-s5p64x0/common.c
and will meet the same fate.

> arch/sh/boards/mach-se/7343/irq.c
> 	-> uses irq_base = irq_linear_revmap(se7343_irq_domain, 0) where
> 	se7343_irq_domain is a linear domain.
> 	AFAICT this is a bug. (After adding the domain they map all irqs
> 	in increasing order which currently seems to guarantee that it
> 	works. But IMHO it should use a legacy domain.)
> 
> arch/sh/boards/mach-se/7722/irq.c
> 	as above.

Right. I think irq_domain_add_simple() is the right interface to use
here.

> For the uncommented files using %32 instead of -gc->irq_base should
> work.

I'm not sure I understand why this doesn't work for the ones that use
a base that isn't a multiple of 32. Since you are masking the hwirq
rather than the irq number, it will still be zero-based, won't it?

	Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/