linux-kernel - Re: [PATCH v6 2/5] irqchip/aspeed-i2c-ic: Add I2C IRQ controller for Aspeed

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: <75ef2cc1-44d7-0137-96e5-77ba68ee071f@arm.com>
Date:   Wed, 29 Mar 2017 11:55:17 +0100
From:   Marc Zyngier <marc.zyngier@....com>
To:     Brendan Higgins <brendanhiggins@...gle.com>,
        Benjamin Herrenschmidt <benh@...nel.crashing.org>
Cc:     Wolfram Sang <wsa@...-dreams.de>, robh+dt@...nel.org,
        mark.rutland@....com, tglx@...utronix.de, jason@...edaemon.net,
        Joel Stanley <joel@....id.au>,
        Vladimir Zapolskiy <vz@...ia.com>,
        Kachalov Anton <mouse@...c.ru>,
        Cédric Le Goater <clg@...d.org>,
        linux-i2c@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        OpenBMC Maillist <openbmc@...ts.ozlabs.org>
Subject: Re: [PATCH v6 2/5] irqchip/aspeed-i2c-ic: Add I2C IRQ controller for
 Aspeed

On 29/03/17 10:59, Brendan Higgins wrote:
> The main reason I took this approach is just because I thought it was
> cleaner from the perspective of the busses which are totally
> independent (except for the fact that they share a single hardware
> interrupt).
> 
> I did not make any measurements, so I doubt that I have anything to
> add that you don't already know. I saw other usages of chained
> interrupts that do the same thing (scan a "status" register and use
> them to make software interrupts) and I thought that is basically what
> the dummy irq chip code is for. The only thing I thought I was doing
> that was novel was actually breaking out the dummy irqchip into its
> own driver; it is not my idea, but I do think makes it a lot cleaner.
> Nevertheless, it should be cheap in terms of number of instructions;
> the most expensive part looks like looking up the mapping. In any
> case, I think the low hanging fruit here is supporting buffering or
> DMA, like Ben suggested.
> 
> To address the comment on being over engineered: outside of the init
> function (which would exist regardless of how we do this, if not here
> then in the I2C driver); the code is actually pretty small and
> generic.
> 
> All that being said, it would not be very hard to do this without
> using the dummy irqchip code and it would definitely be smaller in
> terms of indirection and space used, but I think the code would
> actually be more complicated to read. We would be going back to having
> an I2C controller along with the I2C busses; where all the I2C
> controller does is read the IRQ register and then call the appropriate
> bus irq handler, which looks a lot like a dummy irqchip.

As long as you're happy with the performance and the restrictions that
come attached to the HW, I'm happy to take the irqchip patches.

Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...