From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758843Ab3KMI2n (ORCPT ); Wed, 13 Nov 2013 03:28:43 -0500 Received: from mail-pd0-f172.google.com ([209.85.192.172]:44578 "EHLO mail-pd0-f172.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1758733Ab3KMI2a (ORCPT ); Wed, 13 Nov 2013 03:28:30 -0500 From: Grant Likely Subject: Re: [RFC 9/9] of/irq: create interrupts-extended property To: Peter Crosthwaite Cc: devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Rob Herring In-Reply-To: References: <1381869563-16083-1-git-send-email-grant.likely@linaro.org> <1381869563-16083-10-git-send-email-grant.likely@linaro.org> <20131112065405.C75E8C42024@trevor.secretlab.ca> <20131112085038.B6A75C421BB@trevor.secretlab.ca> Date: Wed, 13 Nov 2013 15:14:25 +0900 Message-Id: <20131113061425.667F9C41807@trevor.secretlab.ca> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, 13 Nov 2013 09:17:01 +1000, Peter Crosthwaite wrote: > On Tue, Nov 12, 2013 at 6:50 PM, Grant Likely wrote: > > On Tue, 12 Nov 2013 17:49:37 +1000, Peter Crosthwaite wrote: > >> Hi Grant, > >> > >> On Tue, Nov 12, 2013 at 4:54 PM, Grant Likely wrote: > >> > On Tue, 12 Nov 2013 08:58:15 +1000, Peter Crosthwaite wrote: > >> >> Hi Grant, > >> >> > >> >> On Wed, Oct 16, 2013 at 6:39 AM, Grant Likely wrote: > >> >> > The standard interrupts property in device tree can only handle > >> >> > interrupts coming from a single interrupt parent. If a device is wired > >> >> > to multiple interrupt controllers, then it needs to be attached to a > >> >> > node with an interrupt-map property to demux the interrupt specifiers > >> >> > which is confusing. It would be a lot easier if there was a form of the > >> >> > interrupts property that allows for a separate interrupt phandle for > >> >> > each interrupt specifier. > >> >> > > >> >> > This patch does exactly that by creating a new interrupts-extended > >> >> > property which reuses the phandle+arguments pattern used by GPIOs and > >> >> > other core bindings. > >> >> > > >> >> > Signed-off-by: Grant Likely > >> >> > Cc: Rob Herring > >> >> > --- > >> >> > .../bindings/interrupt-controller/interrupts.txt | 29 +++++++-- > >> >> > arch/arm/boot/dts/testcases/tests-interrupts.dtsi | 16 +++++ > >> >> > arch/arm/boot/dts/versatile-ab.dts | 2 +- > >> >> > arch/arm/boot/dts/versatile-pb.dts | 2 +- > >> >> > drivers/of/irq.c | 16 +++-- > >> >> > drivers/of/selftest.c | 70 ++++++++++++++++++++++ > >> >> > 6 files changed, 122 insertions(+), 13 deletions(-) > >> >> > > >> >> > diff --git a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt > >> >> > index 72a06c0..1486497 100644 > >> >> > --- a/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt > >> >> > +++ b/Documentation/devicetree/bindings/interrupt-controller/interrupts.txt > >> >> > @@ -4,16 +4,33 @@ Specifying interrupt information for devices > >> >> > 1) Interrupt client nodes > >> >> > ------------------------- > >> >> > > >> >> > -Nodes that describe devices which generate interrupts must contain an > >> >> > -"interrupts" property. This property must contain a list of interrupt > >> >> > -specifiers, one per output interrupt. The format of the interrupt specifier is > >> >> > -determined by the interrupt controller to which the interrupts are routed; see > >> >> > -section 2 below for details. > >> >> > +Nodes that describe devices which generate interrupts must contain an either an > >> >> > +"interrupts" property or an "interrupts-extended" property. These properties > >> >> > +contain a list of interrupt specifiers, one per output interrupt. The format of > >> >> > +the interrupt specifier is determined by the interrupt controller to which the > >> >> > +interrupts are routed; see section 2 below for details. > >> >> > + > >> >> > + Example: > >> >> > + interrupt-parent = <&intc1>; > >> >> > + interrupts = <5 0>, <6 0>; > >> >> > > >> >> > The "interrupt-parent" property is used to specify the controller to which > >> >> > interrupts are routed and contains a single phandle referring to the interrupt > >> >> > controller node. This property is inherited, so it may be specified in an > >> >> > -interrupt client node or in any of its parent nodes. > >> >> > +interrupt client node or in any of its parent nodes. Interrupts listed in the > >> >> > +"interrupts" property are always in reference to the node's interrupt parent. > >> >> > + > >> >> > +The "interrupts-extended" property is a special form for use when a node needs > >> >> > +to reference multiple interrupt parents. Each entry in this property contains > >> >> > +both the parent phandle and the interrupt specifier. "interrupts-extended" > >> >> > +should only be used when a device has multiple interrupt parents. > >> >> > + > >> >> > + Example: > >> >> > + interrupts-extended = <&intc1 5 1>, <&intc2 1 0>; > >> >> > + > >> >> > >> >> A slightly different but related problem I am trying to solve with > >> >> interrupt bindings, is to describe connection of a single device > >> >> interrupt line to multiple interrupts controller, whereas this binding > >> >> seems to be describing connection of different interrupt lines to > >> >> different controllers. Can this syntax accommodate my case is any way? > >> >> > >> >> Bascially I think I'm asking for multiple interrupt specifiers for a > >> >> single IRQ output if that's possible. > >> >> > >> >> Should I just be using the interrupt map syntax instead? > >> >> > >> >> interrupt-map-mask = < 0 0 63>; > >> >> interrupt-map = < 0 0 25 &intc0 ... >, < 0 0 25 &intc1 ... >, > >> > > >> > Interrupt map doesn't actually help you here either since the core code > >> > doesn't know what to do with it. It will probably only match and return > >> > one of the options. > >> > > >> > >> Yes I noticed that. Although we are a fair way off Linux patches for > >> this just yet and all I want to solve near-term is the hardware > >> description problem (what core Linux IRQ code is actually supposed to > >> do when given such a wiring setup is probably an open question). > >> > >> > What I would do is describe both in your interrupts property and make > >> > the driver know that the extra interrupt specifier is for the same > >> > interrupt output. > >> > > >> > >> So this is a little annoying, as ideally device drivers should not be > >> aware of their external connections. I'm trying to describe board (or > >> SoC) level wiring in hopefully a way that doesn't require individual > >> driver awareness. It also gets very messy when you have multiple > >> interrupt outputs that are each in themselves connected to multiple > >> interrupt controllers (which happens to be my situation). The meaning > >> of multiple tuples becomes ambiguous. > >> > >> Is the interrupt-map system I proposed acceptable long-term from just > >> a pure binding acceptability standpoint? The driver side solution is > >> also a large change pattern as I'm using this with Xilinx Zynq which > >> has a large number of peripherals, any of which could be > >> multiply-connected to both A9GIC and soft intc's in FPGA fabric. > > > > Well, it isn't actively dangerous, but it really isn't a very good > > description either. There is no information about what the difference > > would be between the different connection options. The kernel certainly > > cannot do anything intelligent with it. > > > > You'd probably be better off with an IRQ mux node and associated driver > > that chooses the appropriate connection and makes the behaviour > > transparent to the device driver. That way you'd have a place to embed > > the knowledge of how to make good decisions. > > > > Hi Grant, > > That sounds good, and it avoids me having to deal with that > interrupt-map crazyness. Here's a first attempt at this form of > syntax: > > foo_irq_mux { > #interrupt-cells = 0; > comaptible = "irq-mux"; > interrupts-extended = < &intc0 0 1 >,< &intc1 2 3 >; > /* descision making info goes here */ > }; > > foo : foo@deadbeef { > compatible = "vendor,foo"; > interrupts-extended = <&foo_irq_mux>; > } > > > It's going to get a little verbose once you start making multiple > connections as you need one mux per wire. Perhaps it could be cleaned > up by making the foo_irq_mux node(s) a child of foo? It could, but then you need some way of attaching a driver to that node, and that would require building knowledge into the driver again. Can you boil it down to a couple of concrete examples? What is a specific example of how the platform should decide which interrupt line to use? g.