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:
> > 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.
> >
> > So.. start with an example of a board DT including a widget socket,
> > which contains pins for an MMIO bus, an i2c bus and 2 interrupt lines.
> >
> > /dts-v1/;
> >
> > / {
> > compatible = "foo,oldboard";
> > ranges;
> > soc@... {
> > ranges;
> > mmio: mmio-bus@... {
> > #address-cells = <2>;
> > #size-cells = <2>;
> > ranges;
> > };
> > 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
> > >;
> > };
> > aliases = {
> > i2c = &i2c;
> > intc = &w1_irqs;
>
> 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.
> > 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>;
> > };
> > };
> >
> > Use of the /plugin/ keyword is rather different from existing
> > practice, so we may want a new one instead.
> >
> > The idea is that this would be compiled to something like:
> >
> > /dts-v1/;
> >
> > / {
> > socket-type = "foo,widget-socket";
> > compatible = "foo,whirligig-widget";
> >
> > fragment@0 {
> > target-alias = "i2c";
>
> Yet another way to express the target... Every new feature for
> overlays seems to define a new way.
Well, yes. Frankly I think that's because the original ways of
describing the target were not well thought out. Using a phandle is
awkward because it will always be -1 until fixups are applied, which
seems a bit pointless (why not have the alias/label directly). Using
a full path means that the overlay can overwrite anywhere in the base
tree which doesn't seem like good practice.
> My thinking was the target is a
> connector node and all devices are under it.
I thought about this, and I think it's technically possible, but it
gets really ugly. The trouble is that connectors frequently have pins
onto multiple buses. In order to combine those into a single parent
node, we'd have to combine the address spaces of all those buses.
That can be done using some complex multi-cell encoding, but it won't
be pretty. Then to make the connection point be a single node in the
base tree you'd essentially have to combine all the buses used in the
connector throughout the base board DT, so that ugly multi-cell
encoding will have to be used throughout most of the base DT, not just
in the connector stuff.
> In your case, the
> connector is not part of the hierarchy for any devices in the overlay.
> That may simplify adding OS support, but seems to be a less accurate
> representation of the h/w.
I think its the best we can do. Because connectors usually combine
multiple logically distinct buses, they really don't exist as a
well-defined point in a single heirarchy.
> > __overlay__ {
> > whirligig-controller@... {
> > ...
> > interrupt-parent = <0xffffffff>;
> > interrupts = <0>;
> > };
> > };
> > };
> > __phandle_fixups__ {
> > /* These are (path, property, offset) tuples) */
> > widget-irqs =
> > "/fragment@0/__overlay__/whirligig-controller@...",
> > "interrupt-parent", <0>;
> > };
> > };
>
--
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
