From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756070AbdC2Kzs (ORCPT ); Wed, 29 Mar 2017 06:55:48 -0400 Received: from foss.arm.com ([217.140.101.70]:60120 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756042AbdC2KzW (ORCPT ); Wed, 29 Mar 2017 06:55:22 -0400 Subject: Re: [PATCH v6 2/5] irqchip/aspeed-i2c-ic: Add I2C IRQ controller for Aspeed To: Brendan Higgins , Benjamin Herrenschmidt References: <20170328051226.21677-1-brendanhiggins@google.com> <20170328051226.21677-3-brendanhiggins@google.com> <49a13bbc-aec3-a349-4323-3c8d2728c62f@arm.com> <1490692375.3177.119.camel@kernel.crashing.org> <91936f1a-0a0d-4091-b981-976503a6f7cd@arm.com> <1490734216.3177.140.camel@kernel.crashing.org> Cc: Wolfram Sang , robh+dt@kernel.org, mark.rutland@arm.com, tglx@linutronix.de, jason@lakedaemon.net, Joel Stanley , Vladimir Zapolskiy , Kachalov Anton , =?UTF-8?Q?C=c3=a9dric_Le_Goater?= , linux-i2c@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, OpenBMC Maillist From: Marc Zyngier Organization: ARM Ltd Message-ID: <75ef2cc1-44d7-0137-96e5-77ba68ee071f@arm.com> Date: Wed, 29 Mar 2017 11:55:17 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.6.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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...