linux-kernel - Re: [PATCH V3 06/11] irqchip: bcm7120-l2: Change DT binding to allow non-contiguous IRQEN/IRQSTAT

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]

Date:	Mon, 24 Nov 2014 15:31:27 +0100
From:	Jonas Gorski <jogo@...nwrt.org>
To:	Kevin Cernekee <cernekee@...il.com>
Cc:	Ralf Baechle <ralf@...ux-mips.org>,
	Florian Fainelli <f.fainelli@...il.com>,
	Jon Fraser <jfraser@...adcom.com>,
	Dmitry Torokhov <dtor@...omium.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Jason Cooper <jason@...edaemon.net>,
	Arnd Bergmann <arnd@...db.de>, computersforpeace@...il.com,
	MIPS Mailing List <linux-mips@...ux-mips.org>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V3 06/11] irqchip: bcm7120-l2: Change DT binding to allow
 non-contiguous IRQEN/IRQSTAT

On Mon, Nov 24, 2014 at 3:40 AM, Kevin Cernekee <cernekee@...il.com> wrote:
> To date, all supported controllers have had the IRQEN register at offset
> 0x00 and the IRQSTAT register at 0x04.  So in DT we would typically see
> something like:
>
>     reg = <0xf0406800 0x8>;
>
> We still want to support this format, but we also need to support cases
> where IRQEN and IRQSTAT aren't adjacent.  So, we will amend the driver to
> accept an alternate format that looks like this:
>
>     reg = <0xf0406800 0x4 0xf0406804 0x4>;
>
> i.e. if the first resource_size() == 4, assume that the first resource
> points to IRQEN and that the next resource points to IRQSTAT.  If the
> first resource_size() == 8, retain the current behavior.
>
> Signed-off-by: Kevin Cernekee <cernekee@...il.com>

Hmm ... the more I think about this, the less I like it.

Using the amount and size of the reg-properties to infer a certain
layout seems rather hackish and dirty to me. Maybe we should just use
different compatible match ids for that? E.g. brcm,bm7120-l2-intc for
the 32-bit en/stat pairs, and e.g. brcm,bcm6368-l2-intc for the 64-bit
wide one. Or maybe make the bcm63xx one a separate driver and let it
share code with the bcm7120-l2-intc driver.

This would avoid having to specify a lot of regs (let's assume we also
add support for affinity), and cause a lot of io(re)map calls - the
bcm63268 one would currently look like:

        reg = <0x1000002c 0x4 0x1000003c 0x4>, /* irq 0..31 -> mips irq 2 */
               <0x10000028 0x4 0x10000038 0x4>,  /* irq 32..63 -> mips irq 2 */
               <0x10000024 0x4 0x10000034 0x4>, /* irq 64 .. 95 -> mips irq 2 */
               <0x10000020 0x4 0x10000030 0x4>, /* irq 96 .. 127 ->
mips irq 2 */
               <0x1000004c 0x4 0x1000005c 0x4>, /* irq 0.. 31 -> mips irq 3 */
               <0x10000048 0x4 0x10000058 0x4>, /* irq 32 .. 63 -> mips irq 3 */
               <0x10000044 0x4 0x10000054 0x4>, /* irq 64 ... 95 ->
mips irq 3 */
              <0x10000040 0x4 0x10000050 0x4>; /* irq 96 ... 127 ->
mips irq 3 */

where as with a different match id, we could rather allow something like

        reg = <0x10000020 0x20>, /* irq 0..127 -> mips irq 2 */
               <0x10000040 0x20>;   /* irq 0..127 -> mips irq 3 */


This would make the dts(i) files quite a bit more readable IMHO, and
make it more likely that newer dts(i) files work with older kernels,
e.g. where the mips irq3 routed registers were added - in the current
style, the kernel would interpret these as additional irq banks. Not
that I think this is expected/required to work, but it wouldn't hurt
having at least a bit of backward compatibility for bisecting on a
device that provides a newer dtb through the bootloader.


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