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:   Wed, 29 Mar 2017 02:59:53 -0700
From:   Brendan Higgins <brendanhiggins@...gle.com>
To:     Benjamin Herrenschmidt <benh@...nel.crashing.org>
Cc:     Marc Zyngier <marc.zyngier@....com>,
        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

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.

Cheers

Powered by blists - more mailing lists