From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris BREZILLON Subject: Re: [RFC PATCH v2 07/10] irqchip: atmel-aic: document new dt properties and children nodes Date: Sun, 30 Mar 2014 15:27:17 +0200 Message-ID: <53381BB5.10609@gmail.com> References: <1396029548-10928-1-git-send-email-b.brezillon.dev@gmail.com> <1396029548-10928-8-git-send-email-b.brezillon.dev@gmail.com> <20140329101903.65848c12@skate> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <20140329101903.65848c12@skate> Sender: linux-doc-owner@vger.kernel.org To: Thomas Petazzoni Cc: Rob Landley , Nicolas Ferre , Jean-Christophe Plagniol-Villard , Thomas Gleixner , devicetree@vger.kernel.org, linux-doc@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org List-Id: devicetree@vger.kernel.org Hello Thomas, Le 29/03/2014 10:19, Thomas Petazzoni a =E9crit : > Dear Boris BREZILLON, > > On Fri, 28 Mar 2014 18:59:05 +0100, Boris BREZILLON wrote: > >> +Optional children nodes: >> +- muxed irq entries: >> + Required properties: >> + * compatible: Shall be >> + "atmel,aic-mux-1reg-irq": irq enable/disable/retrieve-status i= s done by >> + setting/clearing/reading flags in a specific register >> + or >> + "atmel,aic-mux-3reg-irq": irq enable/disable/retrieve-status i= s done >> + by writing/reading flags in specific enable/disable/mask regis= ters >> + * reg: encode the interrupt control register. >> + The first cell encode the irq line. >> + The second cell encode the offset register within its iomem ra= nge >> + The last cell encode the iomem region size (should always be s= et to 0x4). >> + * atmel,aic-mux-reg-mask: define the mask used to disable the in= terrupts >> + generated by the muxed entry. > Can you describe in more details what are these muxed irqs? Are they > interrupts raised to the AIC that may actually be related to several > devices, like a shared interrupt? Exactly, muxed irqs are shared irqs. > If that's the case, then what you want is to implement separate > interrupt controller drivers to handle those shared interrupts, and > demux them into multiple separate interrupts. It may work (I'll take a look), but we may have to modify several=20 drivers (and it may break the DT ABI). > > Note that the way you use the "ranges" property seems wrong to me: > you're using it as a "hack" to define the base address of some > peripherals that are outside the AIC, while the ranges property is > normally used to describe the address translations between a child bu= s > and a parent bus. Which is not what you have here, as far as I can > understand. This is clearly not a real bus, but more a virtual/conceptual bus where= each irq source is a bus device. > So could you give more details about the design of the AIC and these > muxed interrupts, to see if the DT binding you're proposing is actual= ly > the right way of representing the hardware? The AIC controller muxes several peripheral irqs to one irq line. If a given peripheral has a pending irq, and the driver requesting this= =20 irq is not loaded (either because it was not enabled or because it is not=20 loaded yet) it may generate spurious interrupts (or even hang forever), waiting for= =20 someone clearing/reading the interrupt flag(s). See this thread for an example of what can happen if we don't disable=20 all muxed interrupts before enabling an IRQ line: https://lkml.org/lkml/2013/3/8/176 In this series' implementation, I define all the muxed (or shared) inte= rrupt entries (using the DT) and let the AIC driver disable all the interrupt= =20 sources when shutting down an interrupt line or before starting the AIC control= ler. This provides a generic solution to avoid these spurious interrupt issu= es instead of adding several hooks in the machine specific code (one for e= ach impacted peripheral). Please tell me if you see a better solution (but keep in mind, this=20 should be done during early init, because the at91 init timer is using a muxed ir= q=20 line on almost all at91 SoCs). Best Regards, Boris > > Thomas