linux-kernel - Re: [PATCH v2 02/10] irqchip: add irqchip driver for nuc900

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: <16178876.fFBuoBSjbd@wuerfel>
Date:	Fri, 15 Jul 2016 09:00:22 +0200
From:	Arnd Bergmann <arnd@...db.de>
To:	Wan Zongshun <vw@...mu.org>
Cc:	Jason Cooper <jason@...edaemon.net>,
	linux-arm-kernel@...ts.infradead.org,
	Russell King <linux@...linux.org.uk>,
	devicetree@...r.kernel.org, linux-clk@...r.kernel.org,
	Daniel Lezcano <daniel.lezcano@...aro.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	linux-kernel@...r.kernel.org, robh@...nel.org,
	p.zabel@...gutronix.de, Wan Zongshun <mcuos.com@...il.com>
Subject: Re: [PATCH v2 02/10] irqchip: add irqchip driver for nuc900

On Friday, July 15, 2016 1:15:58 PM CEST Wan Zongshun wrote:
> 
> Actually, I have two choice to implement this function:
> 
> option1:
> 
> void __exception_irq_entry aic_handle_irq(struct pt_regs *regs)
> {
>         u32 hwirq;
> 
>         (void)readl(aic_base + REG_AIC_IPER);
>         hwirq = readl(aic_base + REG_AIC_ISNR);
> 
>         handle_IRQ((irq_find_mapping(aic_domain, hwirq)), regs);
> }

(side note: I think you want handle_domain_irq())

> option2:
> 
> void __exception_irq_entry aic_handle_irq(struct pt_regs *regs)
> {
>         u32 hwirq;
> 
>         hwirq = readl(aic_base + REG_AIC_IPER);
>         hwirq <<= 2;
> 
>         handle_IRQ((irq_find_mapping(aic_domain, hwirq)), regs);
> }
> 
> Though the option2 do shift for hwirq, but it seems better than do io 
> operation by readl,so I prefer to option2, agree?

That will only return an irq number that is a multiple of four, which
seems wrong since the numbers are not that. Did you mean to write

	hwirq = ilog2(hwirq);   ?

That assumes that REG_AIC_IPER contains a 32-bit value with one single
bit set to indicate which IRQ was triggered.

If the difference is only in performance, you could try measuring which
of the two ends up being faster.

	Arnd