On Wed, Nov 13, 2013 at 4:14 PM, Grant Likely <grant.likely@xxxxxxxxxx> wrote:
> On Wed, 13 Nov 2013 09:17:01 +1000, Peter Crosthwaite <peter.crosthwaite@xxxxxxxxxx> wrote:
>> 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?
>
> 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?
>
So i've spent some time playing with this. I now have a booting kernel
with multiple root interrupt controllers and peripheral devices
multiply-connected to both root controllers. But only one on of the
controllers is used by Linux (as linux being able to use multiple
intcs is a non-trivial problem). So the scheme I am using is to have
one of these root intc's marked as disabled via
status = "disabled".
A few small patches to drivers/of/irq.c were needed to get Linux to
ignore these marked-disabled intcs.
It all looks rather silly at first glance, so it might be a good time
to fill you in on why I am doing this. Two big reasons.
1 - Our cad tools generate complete system DTS's that match the
hardware in the system. It is possible to design system with Xilinx
CAD which have these multiply connected interrupts. Its awkward for us
if there is no dts syntax to support the connectivity (despite Linux
being unable to use it) so my thinking here is to generate the
complete DTS then post-annotate (e.g. with tricks like status =
"disabled") RE choices of what hardware Linux should use.
2 - QEMU. I'm using DTS as a syntax for QEMU machine generation. These
QEMU machines should generate the full hardware setup rather than just
the Linux usable peripheral subset (I guess I'm kindof asking to be a
non-linux consumer of dts here).
Working with Michal to get my patches in a list-ready state. Can you
suggest a candidate tree we should base of for contributions to
drivers/of/irq.c given your recent work?
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/
--
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
