Re: DT connectors, thoughts

[David Gibson]

On Thu, Jul 21, 2016 at 10:15:36PM +0300, Pantelis Antoniou wrote:
> Hi Rob,
> 
> > On Jul 21, 2016, at 22:09 , Rob Herring <robh+dt@xxxxxxxxxx> wrote:
> > 
> > On Thu, Jul 21, 2016 at 9:14 AM, Pantelis Antoniou
> > <pantelis.antoniou@xxxxxxxxxxxx> wrote:
> >> Hi David,
> >> 
> >>> On Jul 21, 2016, at 16:42 , David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> >>> 
> >>> On Wed, Jul 20, 2016 at 11:59:44PM +0300, Pantelis Antoniou wrote:
> >>>> Hi David,
> >>>> 
> >>>> Spent some time looking at this, and it looks like itâs going to the right direction.
> >>>> 
> >>>> Comments inline.
> >>>> 
> >>>>> On Jul 18, 2016, at 17:20 , David Gibson <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> >>>>> 
> >>>>> Hi,
> >>>>> 
> >>>>> Here's some of my thoughts on how a connector format for the DT could
> >>>>> be done.  Sorry it's taken longer than I hoped - I've been pretty
> >>>>> swamped in my day job.
> >>>>> 
> >>>>> This is pretty early thoughts, but gives an outline of the approach I
> >>>>> prefer.
> > 
> > [...]
> > 
> >>>>>            i2c: i2c@... {
> >>>>>            };
> >>>>>            intc: intc@... {
> >>>>>                    #interrupt-cells = <2>;
> >>>>>            };
> >>>>>    };
> >>>>> 
> >>>>>    connectors {
> >>>>>            widget1 {
> >>>>>                    compatible = "foo,widget-socket";
> >>>>>                    w1_irqs: irqs {
> >>>>>                            interrupt-controller;
> >>>>>                            #address-cells = <0>;
> >>>>>                            #interrupt-cells = <1>;
> >>>>>                            interrupt-map-mask = <0xffffffff>;
> >>>>>                            interrupt-map = <
> >>>>>                                    0 &intc 7 0
> >>>>>                                    1 &intc 8 0
> >>>>>> ;
> >>>>>                    };
> >>>> 
> >>>> This is fine. We need an interrupt controller node.
> >>> 
> >>> Actually I think we only need an interrupt nexus, not an interrupt
> >>> controller (in IEEE1275 terminology).  (An interrupt controller would
> >>> generally require it's own driver, to ack/mask irqs, whereas this just
> >>> demonstrates the routing to an existing interrupt controller).  Which
> >>> makes that example slightly incorrect (it shouldn't have the
> >>> interrupt-controller property).
> >> 
> >> Hmm, as far as I can tell we only have a concept of an interrupt controller
> >> in the kernel. An interrupt nexus is something new. We should get by without
> >> a driver but hacking the interrupt lookup path at DT.
> > 
> > Interrupt nexus is the interrupt-map property which is fully
> > supported. I'd expect we'll end up with a gpio nexus (i.e. gpio-map)
> > for connector gpios, too.
> > 
> 
> Is interrupt-map enough to cover all our cases? On all the cases that I see it

That's the most common use, but it's pretty general.  It basically
maps (source irq desc, source unit address) tuples to (dest interrupt
controller, dest irq desc) tuples.  The interrupt-map-mask lets us
ignore some parts of that input.

One of the weirder uses was in the DTs for the DMA controller used for
the Ethernet on many PPC 4xx chips.  Those had multiple irqs, wired to
different interrupt controllers on some SoCs, but the DT only allows
to specify a single interrupt-parent for a device.  We worked around
that by including an interrupt nexus in the node itself, which mapped
a fake local irq number (just an index) to the right irqs and
controllers.

> Is the example above well defined? As far as I can tell interrupt-controller is not
> needed.

The 'interrupt-controller' property should not be there, that was an
error on my part.  The rest should be ok.

The plugin piece would need to specify the interrupt-parent as w1_irs
on all its nodes, otherwise they'd just inherit from the parent bus
for their fragment, which might have odd results.

We should probably see if we can figure out a way to enforce that.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

Attachment: signature.asc
Description: PGP signature