Re: DT connectors, thoughts

[David Gibson]

On Thu, Aug 25, 2016 at 06:38:41PM -0700, Stephen Boyd wrote:
> Quoting David Gibson (2016-07-21 21:25:56)
> > On Thu, Jul 21, 2016 at 02:15:57PM -0500, Rob Herring wrote:
> > > On Mon, Jul 18, 2016 at 9:20 AM, David Gibson
> > > <david@xxxxxxxxxxxxxxxxxxxxx> wrote:
> > > 
> > > I understand how you are using i2c alias, but not the intc. It would
> > > help if the same names were not used in multiple places unless they
> > > are the same thing.
> > 
> > Yes, sorry.  We have both the /soc/intc node which is the base board's
> > master interrupt controller.  Then we have the connector local 'intc'
> > alias which describes the local interrupt space for just the
> > connector.
> > 
> > > What does using aliases here buy us vs. just properties with a
> > > phandle?
> > 
> > Um.. I'm not sure what you mean.
> 
> I think Rob means drop the aliases node and just have:

Oh, ok.  The reason for the aliases node is that putting the aliases
(or whatever you want to call them) in the top level connector node
limits what potential extensions we can make to the connector format.
The aliases can essentially have any property name, so they could
collide with additional "metadata" properties we might want to add.

> 	property = &phandle;

Note that the example above is misleading.  Without the < >, the label
reference will expand to a path string, not a phandle.

> In this example:
> 
> 	i2c = &i2c;
> 	intc = &w1_irqs;
> 	mmio = &mmio;
> 
> or perhaps to have a list of phandles and names that map to them?
> 
> 	targets = <&i2c>, <&w1_irqs>, <&mmio>;
> 	target-names = "i2c", "intc", "mmio";

That would work, but a) using phandles instead of paths introduces an
additional lookup for no particular benegit and b) it seems harder to
parse.  In addition the aliases format suggested matches the global
aliases property already defined by IEEE1275, so it seems a good place
to start.

> 
> ?
> 
> > 
> > > >                                 mmio = &mmio;
> > > >                         };
> > > >                 };
> > > >         };
> > > > };
> > > >
> > > > Note that the symbols are local to the connector, and explicitly
> > > > listed, rather than including all labels in the tree.  This is to
> > > > enforce (or at the very least encourage) plugins to only access those
> > > > parts of the base tree.
> > > >
> > > > Note also the use of an interrupt nexus node contained within the
> > > > connector to control which irqs the socketed device can use.  I think
> > > > this needs some work to properly handle unit addresses, but hope
> > > > that's enough to give the rough idea.
> > > >
> > > > So, what does the thing that goes in the socket look like?  I'm
> > > > thinking some new dts syntax like this:
> > > >
> > > > /dts-v1/;
> > > >
> > > > /plugin/ foo,widget-socket {
> > > >         compatible = "foo,whirligig-widget";
> > > > };
> > > >
> > > > &i2c {
> > > >         whirligig-controller@... {
> > > >                 ...
> > > >                 interrupt-parent = <&widget-irqs>;
> > > >                 interrupts = <0>;
> > > >         };
> > > > };
> 
> How would we support an expansion board that goes onto two
> sockets/connectors provided by the baseboard when the connectors
> "export" the same phandle aliases? From what I can tell with this design
> we'll be unable to describe a device on the expansion board that is
> wired to properties provided by the two connectors.

Ok, so there are two parts to this.

1) Allowing a plugin to use multiple connectors.

I thought a bit about this case, but didn't address it for
simplicity.  That would require a different syntax, so we can rethink
this if it's a use case we think we need.

2) Dealing with alias collisions between connector types

This one is fairly straightforward to handle.  By default, we'll use
labels from connectors we plug into "as is".  However, we can add a
syntax that allows us to locally rename labels from a connector (for
those familiar with Python, think "import foo from bar as baz").


So, combining those thoughts together, I'm thinking dtc format for
something connecting to two different widget sockets (pretty much the
worst case) would look something like:

/plugin/ foo,widget-socket {
};

/plugin/ foo,widget-socket {
	realias {
		i2c-b = "i2c";
		intc-b = "intc";
		mmio-b = "mmio";
	};
};

&i2c {
	.. devices on the i2c from the first plug ..
};

&i2c-b {
	.. devices on the i2c from the second plug ..
};

Obviously we'd also need to devise an encoding for this to compile
into, since the one I proposed previously won't work in this case.

-- 
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