Re: [RFC 9/9] of/irq: create interrupts-extended property

[Peter Crosthwaite]

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

Regards,
Peter

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