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

[Benjamin Herrenschmidt <benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>]

On Tue, 2017-03-28 at 10:40 +0100, Marc Zyngier wrote:
> On 28/03/17 10:12, Benjamin Herrenschmidt wrote:
> > On Tue, 2017-03-28 at 09:32 +0100, Marc Zyngier wrote:
> > > I'm a bit concerned by this. It means that you can't even mask an
> > > interrupt. Is that really what you intend to do? Or all that the HW can
> > > do? If you cannot mask an interrupt, you're at the mercy of a screaming
> > > device...
> > 
> > This is not really an interrupt controller. It's a "summary" register
> > that reflects the state of the 14 i2c controller interrupts.
> > 
> > This approach does have the advantage of providing separate counters in
> > /proc/interrupts which is rather nice, but it does have overhead. On
> > those shittly little ARMv9 400Mhz cores it can be significant.
> 
> <pedantic>
> s/ARMv9/ARM9/, as we're still on variations of the ARMv8 architecture ;-)
> </pedantic>

It was a typo, I meant ARM9/ARMv5 :-)

The 2 SOC families we are talking about (Aspeed 24xx and 25xx) are
based on a ARM926EJ at 400Mhz and an ARM1176JZFS at 800Mhz
respectively, so cycles do count :-)
 
> A 400MHz ARM9 (which is either ARMv4 or ARMv5) is not too bad (hey, we
> still have a couple of Versatile-ABs here...). Caches are pretty small
> though.

16K/16K, no L2 :) 

> > I would personally have some kind of trick to register a single
> > interrupt handler that calls directly the handlers of the respective
> > i2c busses via a simple indirection for speed, maybe adding my custom
> > sysfs or debugfs statistics. But that's just me trying to suck the last
> > cycle out of the bloody thing ;-)
> 
> I'd hope the irqdomain itself to be pretty light (the revmap should help
> here), but of course you're going to do more work. Counters also come at
> a cost. It'd be interesting to see if Brendan has any overhead data
> about this.

Thankfully, the HW supports buffered sends/receive or even DMA. The
current patch doesn't yet support these but they would be a good way to
alleviate the cost of the interrupts if it becomes a problem.

Cheers,
Ben.

> Cheers,
> 
> 	M.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html