On 15/12/14 20:58, Stefan Agner wrote: > On 2014-12-15 10:59, Marc Zyngier wrote: >> Hi Stefan, >> >> On 14/12/14 22:09, Stefan Agner wrote: >>> This adds support for Vybrid's interrupt router. On VF6xx models, >>> almost all peripherals can be accessed from either of the two >>> CPU's, from the Cortex-A5 or from the Cortex-M4. The interrupt >>> router routes the peripheral interrupts to the configured CPU. >>> >>> The driver makes use of the irqdomain hierarchy support. The >>> parent is either the ARM GIC or the ARM NVIC interrupt controller >>> depending on which CPU the kernel is executed on. Currently only >>> ARM GIC is supported because the NVIC driver lacks hierarchical >>> irqdomain support as of now. >>> >>> Currently, there is no resource control mechnism implemented to >>> avoid concurrent access of the same peripheral. The user needs >>> to make sure to use device trees which assign the peripherals >>> orthogonally. However, this driver warns the user in case the >>> interrupt is already configured for the other CPU. This provides >>> a poor man's resource controller. >>> >>> Signed-off-by: Stefan Agner <stefan@xxxxxxxx> >>> --- >>> Thanks for the feedback on the initial driver, I'm quite happy >>> with the outcome using the hierarchic irqdomain support. >> >> Great stuff, pleased to see the stacked domain proving to be useful. >> >>> The driver is tested on Vybrid running on the Cortex-A5 CPU. >>> However, to properly support Cortex-M4, some more work will be >>> needed. Beside the hierarchic irqdomain support for NVIC, the >>> different IRQ cell layout need to be solved: NVIC uses only >>> one cell, whereas GIC uses three. I see two possible solutions: >>> - Support two layouts in this driver. Maybe by using IS_ENABLED, >>> since it is not possible to compile a kernel for the A5 and >>> M4. >>> - Define a 3 cell layout as GIC uses it for the MSCM, and pass >>> a syntetic one cell layout to the parent when calling >>> irq_domain_alloc_irqs_parent. This driver would then still >>> need to know what type of interrupt controller the parent is... >>> >>> Ideas/advice welcome... >> >> You shouldn't use the GIC format for the MSCM, as it doesn't mean >> anything for it. Yes, I know that everybody did that, but that's just >> wrong (MSCM itself shouldn't care about SPIs, except when it is actually >> talking to a GIC). The only reason I didn't clean that up in my ongoing >> series is to avoid having to rewrite all the DTs entirely. >> >> My hunch is that you should have a MSCM-specific interrupt description >> (I guess two cells should be enough, one for the interrupt number and >> one for the trigger if necessary), and translate this to the format that >> the backing interrupt controller is using (only the map function should >> be affected). > > Ok, so foremost you suggest to use always the same interrupt > specification, no matter if I use the dt for the A5 or the M4. Hm, just > some weeks ago I extracted the interrupt properties of all peripherals > and made a base device tree without interrupt properties, just so that I > could create a device tree with the interrupt properties for NVIC and > one for GIC (see vf500.dtsi vs the preliminary vf610m4.dtsi from the > Cortex-M4 support patchset). Back then, I did not put much thought into > MSCM etc., and just adjusted the interrupt properties to the needs of > those two interrupt controllers. When having a common definition, I can > merge those interrupt nodes back into the common device tree, which is > much nicer anyway! Indeed. The thing to realize is that from the point of view of the device, the interrupt controller *is* MSCM. That is what the wire is connected to. What the MSCM is connected to is its responsibility. > Regarding format, since I have to touch all the interrupt properties > anyway, it's not much hassle to use a new format in that process. So my > MSCM format would be, as you suggested, two cells with interrupt number > and the trigger specification (IRQ_TYPE... from > ./include/dt-bindings/interrupt-controller/irq.h). > > One open thing: How should I determine the backing interrupt controller? > Maybe by just reading the interrupt-cells property of the parent > interrupt controller, and depending on the cell count create that > format? The way to handle this would be to look at the interrupt-parent (you get a pointer to it anyway), and match the compatible string. You still need to hardcode the knowledge of the format for GIC and NVIC though. [...] >> Otherwise, looks pretty good to me. >> > > The same line adjustment will break the 80 character border... But I > agree, it's ugly the way it is now. Will put them in the same line. Never mind what checkpatch says. Readability trumps it anytime. Thanks, M. -- Jazz is not dead. It just smells funny... -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
