Message-Id: <20131112085038.B6A75C421BB@trevor.secretlab.ca>
Date:	Tue, 12 Nov 2013 17:50:38 +0900
From:	Grant Likely <grant.likely@...aro.org>
To:	Peter Crosthwaite <peter.crosthwaite@...inx.com>
Cc:	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
	Rob Herring <rob.herring@...xeda.com>
Subject: Re: [RFC 9/9] of/irq: create interrupts-extended property

On Tue, 12 Nov 2013 17:49:37 +1000, Peter Crosthwaite <peter.crosthwaite@...inx.com> wrote:
> Hi Grant,
> 
> On Tue, Nov 12, 2013 at 4:54 PM, Grant Likely <grant.likely@...aro.org> wrote:
> > On Tue, 12 Nov 2013 08:58:15 +1000, Peter Crosthwaite <peter.crosthwaite@...inx.com> wrote:
> >> Hi Grant,
> >>
> >> On Wed, Oct 16, 2013 at 6:39 AM, Grant Likely <grant.likely@...aro.org> 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 <grant.likely@...aro.org>
> >> > Cc: Rob Herring <rob.herring@...xeda.com>
> >> > ---
> >> >  .../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.

g.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists