portable device tree connector -- problem statement

On the other hand, I have no previous detailed knowledge of the beagle family.

- for bones with the same pinout:
- the pins are routed to different function blocks on the SOC because different bones may have different SOCs?
- the different functional blocks are compatible or not?

Quoted 2 lines · Frank Rowand

On the other hand, I have no previous detailed knowledge of the beagle family.

This is in no way specific to the BeagleBones, there's plenty of other boards out there with similar setups like the Raspberry Pi and its derivatives.

Quoted 4 lines

- for bones with the same pinout:
- the pins are routed to different function blocks on the SOC because different bones may have different SOCs?
- the different functional blocks are compatible or not?

This is the general case, there will be a substantial level of compatibility between different base boards by virtue of the pinouts being the same but obviously there will be some variation in the specifics (and even where that exists it may not be enough to be visible at the DT level for the most part). That said there will doubtless be some plug in modules that want to rely on the specifics of a given base board rather than remain compatible with general users of the interface.

Quoted 2 lines · Frank Rowand

On the other hand, I have no previous detailed knowledge of the beagle family.

This is in no way specific to the BeagleBones, there's plenty of other boards out there with similar setups like the Raspberry Pi and its derivatives.

Quoted 4 lines

- for bones with the same pinout:
- the pins are routed to different function blocks on the SOC because different bones may have different SOCs?
- the different functional blocks are compatible or not?

This is the general case, there will be a substantial level of compatibility between different base boards by virtue of the pinouts being the same but obviously there will be some variation in the specifics (and even where that exists it may not be enough to be visible at the DT level for the most part). That said there will doubtless be some plug in modules that want to rely on the specifics of a given base board rather than remain compatible with general users of the interface.

Quoted 8 lines · Mark Brown

Quoted 2 lines · Frank Rowand

On the other hand, I have no previous detailed knowledge of the beagle family.

This is in no way specific to the BeagleBones, there's plenty of other boards out there with similar setups like the Raspberry Pi and its derivatives.

Yes, absolutely. I'm just picking on the beaglebones because that is what Pantelis has recently used for examples. (He has mentioned other connector types and expansion boards in his presentations.)

And we need to think beyond beaglebone, pi, arduino, and grove type of connectors.

Some other connectors that are obvious are pci and possibly usb.

Quoted 6 lines

Quoted 5 lines

- for bones with the same pinout:
- the pins are routed to different function blocks on the SOC because different bones may have different SOCs?
- the different functional blocks are compatible or not?

Quoted 8 lines

This is the general case, there will be a substantial level of compatibility between different base boards by virtue of the pinouts being the same but obviously there will be some variation in the specifics (and even where that exists it may not be enough to be visible at the DT level for the most part). That said there will doubtless be some plug in modules that want to rely on the specifics of a given base board rather than remain compatible with general users of the interface.

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;
				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";
		__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>;
	};

};

Suppose then there's a new version of the board. This extends the widget socket in a backwards compatible way, but there are now two interchangeable sockets, and they're wired up to different irqs and i2c lines on the baseboard:

/dts-v1/;

/ {
	compatible = "foo,newboard";
	ranges;
	soc@... {
		ranges;	
		mmio: mmio-bus@... {
			#address-cells = <2>;
			#size-cells = <2>;
			ranges;
		};
		i2c0: i2c@... {
		};
		i2c1: i2c@... {
		};
		intc: intc@... {
		};
	};

	connectors {
		widget1 {
			compatible = "foo,widget-socket-v2", "foo,widget-socket";
			w1_irqs: irqs {
				interrupt-controller;
				#address-cells = <0>;
				#interrupt-cells = <1>;
				interrupt-map-mask = <0xffffffff>;
				interrupt-map = <
					0 &intc 17 0
					1 &intc 8 0
				>;
			};
			aliases = {
				i2c = &i2c0;
				intc = &w1_irqs;
				mmio = &mmio;
			};
		};
		widget2 {
			compatible = "foo,widget-socket-v2", "foo,widget-socket";
			w2_irqs: irqs {
				interrupt-controller;
				#address-cells = <0>;
				#interrupt-cells = <1>;
				interrupt-map-mask = <0xffffffff>;
				interrupt-map = <
					0 &intc 9 0
					1 &intc 10 0
				>;
			};
			aliases = {
				i2c = &i2c1;
				widget-irqs = &w2_irqs;
				mmio = &mmio;
			};
		};
	};
};

A socketed device could also have it's own connectors - the contrived example below has a little 256 byte mmio space (maybe some sort of LPC thingy?):

/dts-v1/;

/plugin/ foo,widget-socket-v2 {
	compatible = "foo,superduper-widget};

	connectors {
		compatible = "foo,super-socket";
		aliases {
			superbus = &superbus;
		};	
	};
};

&mmio {
	superbus: super-bridge@100000000 {
		#address-cells = <1>;
		#size-cells = <1>;
		ranges = <0x0  0xabcd0000 0x12345600  0x100>;
	};
};

&i2c {
	super-controller@... {
		...
	};
	duper-controller@... {
	};
};

Thoughts?

Signature · 5 lines

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

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;
				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";
		__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>;
	};

};

Suppose then there's a new version of the board. This extends the widget socket in a backwards compatible way, but there are now two interchangeable sockets, and they're wired up to different irqs and i2c lines on the baseboard:

/dts-v1/;

/ {
	compatible = "foo,newboard";
	ranges;
	soc@... {
		ranges;	
		mmio: mmio-bus@... {
			#address-cells = <2>;
			#size-cells = <2>;
			ranges;
		};
		i2c0: i2c@... {
		};
		i2c1: i2c@... {
		};
		intc: intc@... {
		};
	};

	connectors {
		widget1 {
			compatible = "foo,widget-socket-v2", "foo,widget-socket";
			w1_irqs: irqs {
				interrupt-controller;
				#address-cells = <0>;
				#interrupt-cells = <1>;
				interrupt-map-mask = <0xffffffff>;
				interrupt-map = <
					0 &intc 17 0
					1 &intc 8 0
				>;
			};
			aliases = {
				i2c = &i2c0;
				intc = &w1_irqs;
				mmio = &mmio;
			};
		};
		widget2 {
			compatible = "foo,widget-socket-v2", "foo,widget-socket";
			w2_irqs: irqs {
				interrupt-controller;
				#address-cells = <0>;
				#interrupt-cells = <1>;
				interrupt-map-mask = <0xffffffff>;
				interrupt-map = <
					0 &intc 9 0
					1 &intc 10 0
				>;
			};
			aliases = {
				i2c = &i2c1;
				widget-irqs = &w2_irqs;
				mmio = &mmio;
			};
		};
	};
};

A socketed device could also have it's own connectors - the contrived example below has a little 256 byte mmio space (maybe some sort of LPC thingy?):

/dts-v1/;

/plugin/ foo,widget-socket-v2 {
	compatible = "foo,superduper-widget};

	connectors {
		compatible = "foo,super-socket";
		aliases {
			superbus = &superbus;
		};	
	};
};

&mmio {
	superbus: super-bridge@100000000 {
		#address-cells = <1>;
		#size-cells = <1>;
		ranges = <0x0  0xabcd0000 0x12345600  0x100>;
	};
};

&i2c {
	super-controller@... {
		...
	};
	duper-controller@... {
	};
};

Thoughts?

Signature · 5 lines

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

Hi David,

Spent some time looking at this, and it looks like it’s going to the right direction.

Comments inline.

Quoted 16 lines · David Gibson

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

Quoted 10 lines

/ {
	compatible = "foo,oldboard";
	ranges;
	soc@... {
		ranges;
		mmio: mmio-bus@... {
			#address-cells = <2>;
			#size-cells = <2>;
			ranges;
		};

MMIO busses are going the way of the dodo and we have serious problems handling them in linux in a connector (and a portable manner). While we have drivers for GPMC devices we don’t have an in kernel framework for handling them.

A single address range does not contain enough information to program a GPMC interface with all the timings and chip select options. It might be possible to declare a pre-define memory window on the connector, but it’s use on a real system might be limited.

I think it’s best we focus on standard busses like i2c/spi/i2s/mmc and gpios and interrupts for now.

Quoted 12 lines

		i2c: i2c@... {
		};
		intc: intc@... {
			#interrupt-cells = <2>;
		};
	};

	connectors {
		widget1 {
			compatible = "foo,widget-socket";
			w1_irqs: irqs {
				interrupt-controller;

Quoted 8 lines

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

In a similar manner we need GPIOs too for every GPIO option on the connector. Could we fold this in the same node?

Quoted 28 lines

			aliases = {
				i2c = &i2c;
				intc = &w1_irqs;
				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";
};

Quoted 8 lines

&i2c {
	whirligig-controller@... {
		...
		interrupt-parent = <&widget-irqs>;
		interrupts = <0>;
	};
};

OK, this is brand new syntax. I’m all for it if it makes things easier.

Quoted 3 lines

Use of the /plugin/ keyword is rather different from existing practice, so we may want a new one instead.

It’s a bit weird looking and is bound to cause confusion. How about something like /expansion/ ?

Quoted 16 lines

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";
		__overlay__ {
			whirligig-controller@... {
				...
				interrupt-parent = <0xffffffff>;
				interrupts = <0>;
			};

Quoted 8 lines

		};
	};
	__phandle_fixups__ {
		/* These are (path, property, offset) tuples) */
		widget-irqs =
			"/fragment@0/__overlay__/whirligig-controller@...",
			"interrupt-parent", <0>;
	};

I’m not quite sure this is going to work for multiple use of widget-irqs handle, but it’s a detail for now.

What is the action undertaken when a bus is activated? Looks like it’s going to be similar to my patch where the target/alias bus is given a status=“okay”; property and activated, after all subnodes that contain i2c devices are copied there.

Quoted 103 lines

};

Suppose then there's a new version of the board. This extends the widget socket in a backwards compatible way, but there are now two interchangeable sockets, and they're wired up to different irqs and i2c lines on the baseboard:

/dts-v1/;

/ {
	compatible = "foo,newboard";
	ranges;
	soc@... {
		ranges;	
		mmio: mmio-bus@... {
			#address-cells = <2>;
			#size-cells = <2>;
			ranges;
		};
		i2c0: i2c@... {
		};
		i2c1: i2c@... {
		};
		intc: intc@... {
		};
	};

	connectors {
		widget1 {
			compatible = "foo,widget-socket-v2", "foo,widget-socket";
			w1_irqs: irqs {
				interrupt-controller;
				#address-cells = <0>;
				#interrupt-cells = <1>;
				interrupt-map-mask = <0xffffffff>;
				interrupt-map = <
					0 &intc 17 0
					1 &intc 8 0
				>;
			};
			aliases = {
				i2c = &i2c0;
				intc = &w1_irqs;
				mmio = &mmio;
			};
		};
		widget2 {
			compatible = "foo,widget-socket-v2", "foo,widget-socket";
			w2_irqs: irqs {
				interrupt-controller;
				#address-cells = <0>;
				#interrupt-cells = <1>;
				interrupt-map-mask = <0xffffffff>;
				interrupt-map = <
					0 &intc 9 0
					1 &intc 10 0
				>;
			};
			aliases = {
				i2c = &i2c1;
				widget-irqs = &w2_irqs;
				mmio = &mmio;
			};
		};
	};
};

A socketed device could also have it's own connectors - the contrived example below has a little 256 byte mmio space (maybe some sort of LPC thingy?):

/dts-v1/;

/plugin/ foo,widget-socket-v2 {
	compatible = "foo,superduper-widget};

	connectors {
		compatible = "foo,super-socket";
		aliases {
			superbus = &superbus;
		};	
	};
};

&mmio {
	superbus: super-bridge@100000000 {
		#address-cells = <1>;
		#size-cells = <1>;
		ranges = <0x0  0xabcd0000 0x12345600  0x100>;
	};
};

&i2c {
	super-controller@... {
		...
	};
	duper-controller@... {
	};
};

Quoted 2 lines

Thoughts?

It’s a step in the right direction, especially if we nail down the syntax.

Quoted 38 lines · Pantelis Antoniou

Hi David,

Spent some time looking at this, and it looks like it’s going to the right direction.

Comments inline.

Quoted 16 lines · David Gibson

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

Quoted 10 lines

/ {
	compatible = "foo,oldboard";
	ranges;
	soc@... {
		ranges;
		mmio: mmio-bus@... {
			#address-cells = <2>;
			#size-cells = <2>;
			ranges;
		};

MMIO busses are going the way of the dodo and we have serious problems handling them in linux in a connector (and a portable manner). While we have drivers for GPMC devices we don’t have an in kernel framework for handling them.

Quoted 4 lines

A single address range does not contain enough information to program a GPMC interface with all the timings and chip select options. It might be possible to declare a pre-define memory window on the connector, but it’s use on a real system might be limited.

Ok. I think the example has some value in showing how MMIO ranges and mapping could be expressed even if it's only part of something more complex than a simple MMIO bus.

For example I could imagine a connector which includes PCI and some irq lines. The PCI part is probable, of course, but a PCI device wired to one of the hard interrupt lines instead of a PCI interrupt line would need some DT information. Of course non-Express, non-MSI PCI is pretty much extinct too, but it's not much of a stretch to imagine that something which requires some portion of MMIO mapping is out there or will come along.

Quoted 25 lines

I think it’s best we focus on standard busses like i2c/spi/i2s/mmc and gpios and interrupts for now.

Quoted 20 lines

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

Quoted 1 line

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

Quoted 3 lines

In a similar manner we need GPIOs too for every GPIO option on the connector. Could we fold this in the same node?

IIRC the GPIO binding is pretty much modeled on the interrupt binding and has a similar "nexus" concept. I was expecting the same thing for GPIO. It's expressed with different properties to those for irqs, obviously, so I guess it could be in the same node. Whether it's clearer to have them in the same or separate nodes I suspect would depend on the specifics of the board.

Quoted 37 lines

Quoted 28 lines

			aliases = {
				i2c = &i2c;
				intc = &w1_irqs;
				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";
};

Quoted 8 lines

&i2c {
	whirligig-controller@... {
		...
		interrupt-parent = <&widget-irqs>;
		interrupts = <0>;
	};
};

Quoted 8 lines

OK, this is brand new syntax. I’m all for it if it makes things easier.

Quoted 3 lines

Use of the /plugin/ keyword is rather different from existing practice, so we may want a new one instead.

It’s a bit weird looking and is bound to cause confusion. How about something like /expansion/ ?

That could work.

Quoted 25 lines

Quoted 24 lines

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";
		__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>;
	};

Quoted 2 lines

I’m not quite sure this is going to work for multiple use of widget-irqs handle, but it’s a detail for now.

Just concatenate all the tuples, so path, property, offset, path, property, offset, etc..

Quoted 3 lines

What is the action undertaken when a bus is activated? Looks like it’s going to be similar to my patch where the target/alias bus is given a status=“okay”; property and activated, after all subnodes that contain i2c devices are copied there.

Erm.. what exactly do you mean by "activated"? At the moment you could put a status="okay" in the plugin component, and that would be applied (as long as it goes in one of the accessible attachment points).

Which does bring up a point. I did wonder if the approach above allows the plugin to do too much - e.g. overriding properties in the i2c controller node, rather than just adding children. So I did wonder if we wanted a restriction that only new nodes can be added at the top level of the plugin fragment.

Alternatively that might be achievable by (as a recommended / best practice) putting a "container" subnode under each attachable bus on the master dt and pointing the aliases at that instead of the actual base bus controller. With the right 'ranges' etc. that might accomplish what's needed without extra semantics, but I'm not certain.

Ah.. which makes me think of another point. In this proposal the aliases is used to control both where fragments can be attached, and what nodes can be referenced by phandle. But we probably want to split those concepts: e.g. the plugin will need to reference the interrupt controller / nexus, but probably shouldn't be allowed to override its properties.

Quoted 103 lines

Quoted 103 lines

};

Suppose then there's a new version of the board. This extends the widget socket in a backwards compatible way, but there are now two interchangeable sockets, and they're wired up to different irqs and i2c lines on the baseboard:

/dts-v1/;

/ {
	compatible = "foo,newboard";
	ranges;
	soc@... {
		ranges;	
		mmio: mmio-bus@... {
			#address-cells = <2>;
			#size-cells = <2>;
			ranges;
		};
		i2c0: i2c@... {
		};
		i2c1: i2c@... {
		};
		intc: intc@... {
		};
	};

	connectors {
		widget1 {
			compatible = "foo,widget-socket-v2", "foo,widget-socket";
			w1_irqs: irqs {
				interrupt-controller;
				#address-cells = <0>;
				#interrupt-cells = <1>;
				interrupt-map-mask = <0xffffffff>;
				interrupt-map = <
					0 &intc 17 0
					1 &intc 8 0
				>;
			};
			aliases = {
				i2c = &i2c0;
				intc = &w1_irqs;
				mmio = &mmio;
			};
		};
		widget2 {
			compatible = "foo,widget-socket-v2", "foo,widget-socket";
			w2_irqs: irqs {
				interrupt-controller;
				#address-cells = <0>;
				#interrupt-cells = <1>;
				interrupt-map-mask = <0xffffffff>;
				interrupt-map = <
					0 &intc 9 0
					1 &intc 10 0
				>;
			};
			aliases = {
				i2c = &i2c1;
				widget-irqs = &w2_irqs;
				mmio = &mmio;
			};
		};
	};
};

A socketed device could also have it's own connectors - the contrived example below has a little 256 byte mmio space (maybe some sort of LPC thingy?):

/dts-v1/;

/plugin/ foo,widget-socket-v2 {
	compatible = "foo,superduper-widget};

	connectors {
		compatible = "foo,super-socket";
		aliases {
			superbus = &superbus;
		};	
	};
};

&mmio {
	superbus: super-bridge@100000000 {
		#address-cells = <1>;
		#size-cells = <1>;
		ranges = <0x0  0xabcd0000 0x12345600  0x100>;
	};
};

&i2c {
	super-controller@... {
		...
	};
	duper-controller@... {
	};
};

Quoted 5 lines

Quoted 2 lines

Thoughts?

It’s a step in the right direction, especially if we nail down the syntax.

Excellent.

Signature · 4 lines

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

Hi David,

Quoted 19 lines · David Gibson

Quoted 16 lines · Pantelis Antoniou

Hi David,

Spent some time looking at this, and it looks like it’s going to the right direction.

Comments inline.

Quoted 10 lines · David Gibson

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.

Quoted 21 lines

Quoted 21 lines

Quoted 20 lines

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

Quoted 8 lines

Quoted 1 line

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.

Quoted 21 lines · Pantelis Antoniou

Hi David,

Quoted 19 lines · David Gibson

Quoted 16 lines · Pantelis Antoniou

Hi David,

Spent some time looking at this, and it looks like it’s going to the right direction.

Comments inline.

Quoted 10 lines · David Gibson

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.

[...]

Quoted 30 lines

Quoted 29 lines

Quoted 18 lines

Quoted 18 lines

           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

Quoted 4 lines

Quoted 2 lines

Quoted 1 line

;

                   };

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

Quoted 3 lines

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.

Rob

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;

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

                __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>;
        };
};

Hi Rob,

Quoted 62 lines · Rob Herring

Quoted 21 lines · Pantelis Antoniou

Hi David,

Quoted 19 lines · David Gibson

Quoted 16 lines · Pantelis Antoniou

Hi David,

Spent some time looking at this, and it looks like it’s going to the right direction.

Comments inline.

Quoted 10 lines · David Gibson

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.

[...]

Quoted 33 lines

Quoted 21 lines

Quoted 21 lines

Quoted 20 lines

           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

Quoted 1 line

;

                   };

Quoted 8 lines

Quoted 1 line

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.

Quoted 4 lines

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 used is in the context of PCI or some sort of bus.

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

Hi David,

Quoted 41 lines · David Gibson

Quoted 38 lines · Pantelis Antoniou

Hi David,

Spent some time looking at this, and it looks like it’s going to the right direction.

Comments inline.

Quoted 16 lines · David Gibson

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

Quoted 10 lines

/ {
	compatible = "foo,oldboard";
	ranges;
	soc@... {
		ranges;
		mmio: mmio-bus@... {
			#address-cells = <2>;
			#size-cells = <2>;
			ranges;
		};

MMIO busses are going the way of the dodo and we have serious problems handling them in linux in a connector (and a portable manner). While we have drivers for GPMC devices we don’t have an in kernel framework for handling them.

Quoted 9 lines

Quoted 4 lines

A single address range does not contain enough information to program a GPMC interface with all the timings and chip select options. It might be possible to declare a pre-define memory window on the connector, but it’s use on a real system might be limited.

Ok. I think the example has some value in showing how MMIO ranges and mapping could be expressed even if it's only part of something more complex than a simple MMIO bus.

Yep.

Quoted 7 lines

For example I could imagine a connector which includes PCI and some irq lines. The PCI part is probable, of course, but a PCI device wired to one of the hard interrupt lines instead of a PCI interrupt line would need some DT information. Of course non-Express, non-MSI PCI is pretty much extinct too, but it's not much of a stretch to imagine that something which requires some portion of MMIO mapping is out there or will come along.

Yes, this is actually a real system we’re developing against. Non PCI-E and non-MSI PCI is not extinct, it’s still kicking about, although not in server/desktop class machines.

We should be able to figure things out.

Quoted 25 lines

Quoted 25 lines

I think it’s best we focus on standard busses like i2c/spi/i2s/mmc and gpios and interrupts for now.

Quoted 20 lines

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

Quoted 8 lines

Quoted 1 line

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.

Quoted 9 lines

Quoted 2 lines

In a similar manner we need GPIOs too for every GPIO option on the connector. Could we fold this in the same node?

IIRC the GPIO binding is pretty much modeled on the interrupt binding and has a similar "nexus" concept. I was expecting the same thing for GPIO. It's expressed with different properties to those for irqs, obviously, so I guess it could be in the same node. Whether it's clearer to have them in the same or separate nodes I suspect would depend on the specifics of the board.

Agreed. I should note that it’s pretty standard for a gpio controller to advertise itself as an interrupt controller too.

Quoted 73 lines

Quoted 37 lines

Quoted 28 lines

			aliases = {
				i2c = &i2c;
				intc = &w1_irqs;
				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";
};

Quoted 8 lines

&i2c {
	whirligig-controller@... {
		...
		interrupt-parent = <&widget-irqs>;
		interrupts = <0>;
	};
};

Quoted 8 lines

OK, this is brand new syntax. I’m all for it if it makes things easier.

Quoted 3 lines

Use of the /plugin/ keyword is rather different from existing practice, so we may want a new one instead.

It’s a bit weird looking and is bound to cause confusion. How about something like /expansion/ ?

That could work.

Quoted 25 lines

Quoted 24 lines

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";
		__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>;
	};

Quoted 6 lines

Quoted 2 lines

I’m not quite sure this is going to work for multiple use of widget-irqs handle, but it’s a detail for now.

Just concatenate all the tuples, so path, property, offset, path, property, offset, etc..

Note that parsing that property is going to be really awkward.

It’s [string] [string] [cell], …

We don’t have accessors for something like this.

Quoted 9 lines

Quoted 3 lines

What is the action undertaken when a bus is activated? Looks like it’s going to be similar to my patch where the target/alias bus is given a status=“okay”; property and activated, after all subnodes that contain i2c devices are copied there.

Erm.. what exactly do you mean by "activated"? At the moment you could put a status="okay" in the plugin component, and that would be applied (as long as it goes in one of the accessible attachment points).

I mean that the bus is by default non-activated. When a portable connector overlay is applied and the bus is referenced then the board level bus must be enabled.

Quoted 6 lines

Which does bring up a point. I did wonder if the approach above allows the plugin to do too much - e.g. overriding properties in the i2c controller node, rather than just adding children. So I did wonder if we wanted a restriction that only new nodes can be added at the top level of the plugin fragment.

My RFC patch handles those cases. A connector device declares what kind of properties are allowed to be copied to the board level bus node (i.e. clock-freq, speed etc), and whether subnodes are supposed to be copied there (for i2c client devices etc).

Quoted 5 lines

Alternatively that might be achievable by (as a recommended / best practice) putting a "container" subnode under each attachable bus on the master dt and pointing the aliases at that instead of the actual base bus controller. With the right 'ranges' etc. that might accomplish what's needed without extra semantics, but I'm not certain.

Err, I would need an example to grok this.

Quoted 6 lines

Ah.. which makes me think of another point. In this proposal the aliases is used to control both where fragments can be attached, and what nodes can be referenced by phandle. But we probably want to split those concepts: e.g. the plugin will need to reference the interrupt controller / nexus, but probably shouldn't be allowed to override its properties.

Yep.

Quoted 103 lines

Quoted 103 lines

Quoted 103 lines

};

Suppose then there's a new version of the board. This extends the widget socket in a backwards compatible way, but there are now two interchangeable sockets, and they're wired up to different irqs and i2c lines on the baseboard:

/dts-v1/;

/ {
	compatible = "foo,newboard";
	ranges;
	soc@... {
		ranges;	
		mmio: mmio-bus@... {
			#address-cells = <2>;
			#size-cells = <2>;
			ranges;
		};
		i2c0: i2c@... {
		};
		i2c1: i2c@... {
		};
		intc: intc@... {
		};
	};

	connectors {
		widget1 {
			compatible = "foo,widget-socket-v2", "foo,widget-socket";
			w1_irqs: irqs {
				interrupt-controller;
				#address-cells = <0>;
				#interrupt-cells = <1>;
				interrupt-map-mask = <0xffffffff>;
				interrupt-map = <
					0 &intc 17 0
					1 &intc 8 0
				>;
			};
			aliases = {
				i2c = &i2c0;
				intc = &w1_irqs;
				mmio = &mmio;
			};
		};
		widget2 {
			compatible = "foo,widget-socket-v2", "foo,widget-socket";
			w2_irqs: irqs {
				interrupt-controller;
				#address-cells = <0>;
				#interrupt-cells = <1>;
				interrupt-map-mask = <0xffffffff>;
				interrupt-map = <
					0 &intc 9 0
					1 &intc 10 0
				>;
			};
			aliases = {
				i2c = &i2c1;
				widget-irqs = &w2_irqs;
				mmio = &mmio;
			};
		};
	};
};

A socketed device could also have it's own connectors - the contrived example below has a little 256 byte mmio space (maybe some sort of LPC thingy?):

/dts-v1/;

/plugin/ foo,widget-socket-v2 {
	compatible = "foo,superduper-widget};

	connectors {
		compatible = "foo,super-socket";
		aliases {
			superbus = &superbus;
		};	
	};
};

&mmio {
	superbus: super-bridge@100000000 {
		#address-cells = <1>;
		#size-cells = <1>;
		ranges = <0x0  0xabcd0000 0x12345600  0x100>;
	};
};

&i2c {
	super-controller@... {
		...
	};
	duper-controller@... {
	};
};

Quoted 8 lines

Quoted 5 lines

Quoted 2 lines

Thoughts?

It’s a step in the right direction, especially if we nail down the syntax.

Excellent.

Regards

— Pantelis

Quoted 21 lines · Pantelis Antoniou

Hi David,

Quoted 19 lines · David Gibson

Quoted 16 lines · Pantelis Antoniou

Hi David,

Spent some time looking at this, and it looks like it’s going to the right direction.

Comments inline.

Quoted 10 lines · David Gibson

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.

[...]

Quoted 30 lines

Quoted 29 lines

Quoted 21 lines

Quoted 12 lines

            i2c: i2c@... {
            };
            intc: intc@... {
                    #interrupt-cells = <2>;
            };
    };

    connectors {
            widget1 {
                    compatible = "foo,widget-socket";
                    w1_irqs: irqs {
                            interrupt-controller;

Quoted 8 lines

                            #address-cells = <0>;
                            #interrupt-cells = <1>;
                            interrupt-map-mask = <0xffffffff>;
                            interrupt-map = <
                                    0 &intc 7 0
                                    1 &intc 8 0
                            >;
                    };

Quoted 1 line

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

Quoted 3 lines

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.

Hi Rob,

Quoted 62 lines · Rob Herring

Quoted 21 lines · Pantelis Antoniou

Hi David,

Quoted 19 lines · David Gibson

Quoted 16 lines · Pantelis Antoniou

Hi David,

Spent some time looking at this, and it looks like it’s going to the right direction.

Comments inline.

Quoted 10 lines · David Gibson

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.

[...]

Quoted 33 lines

Quoted 21 lines

Quoted 21 lines

Quoted 20 lines

           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

Quoted 1 line

;

                   };

Quoted 8 lines

Quoted 1 line

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.

Quoted 4 lines

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

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

Quoted 39 lines · David Gibson

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

Quoted 8 lines

                                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.

What does using aliases here buy us vs. just properties with a phandle?

Quoted 44 lines

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

Quoted 2 lines

        fragment@0 {
                target-alias = "i2c";

Yet another way to express the target... Every new feature for overlays seems to define a new way.

My thinking was the target is a
connector node and all devices are under it.

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.

Quoted 15 lines

                __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>;
        };
};

Quoted 6 lines

On Mon, Jul 18, 2016 at 9:20 AM, David Gibson
<[email protected]> 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.

Quoted 2 lines

What does using aliases here buy us vs. just properties with a phandle?

Um.. I'm not sure what you mean.

Quoted 26 lines

Quoted 26 lines

                                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";
};

Quoted 7 lines

Quoted 7 lines

&i2c {
        whirligig-controller@... {
                ...
                interrupt-parent = <&widget-irqs>;
                interrupts = <0>;
        };
};

Quoting David Gibson (2016-07-21 21:25:56)

Quoted 13 lines · Rob Herring

Quoted 6 lines

On Mon, Jul 18, 2016 at 9:20 AM, David Gibson
<[email protected]> 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.

Quoted 4 lines

Quoted 2 lines

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:

	property = &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";

?

Quoted 26 lines

Quoted 26 lines

Quoted 26 lines

                                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";
};

Quoted 7 lines

Quoted 7 lines

Quoted 7 lines

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

Quoted 19 lines

Quoting David Gibson (2016-07-21 21:25:56)

Quoted 17 lines · Rob Herring

Quoted 6 lines

On Mon, Jul 18, 2016 at 9:20 AM, David Gibson
<[email protected]> 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.

Quoted 2 lines

What does using aliases here buy us vs. just properties with a phandle?

Um.. I'm not sure what you mean.

Quoted 1 line

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.

Quoted 7 lines

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.

Quoting David Gibson (2016-08-29 06:45:11)

Quoted 20 lines · Stephen Boyd

Quoted 19 lines

Quoting David Gibson (2016-07-21 21:25:56)

Quoted 17 lines · Rob Herring

Quoted 6 lines

On Mon, Jul 18, 2016 at 9:20 AM, David Gibson
<[email protected]> 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.

Quoted 2 lines

What does using aliases here buy us vs. just properties with a phandle?

Um.. I'm not sure what you mean.

Quoted 7 lines

Quoted 1 line

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.

Agreed. Putting them into a subnode will prevent any collisions, but what sorts of collisions would there even be? Presumably the one making up the connector binding will be choosing the phandles they want to export with specific properties, and during that time they can also choose to have other properties that don't conflict?

Quoted 6 lines

Quoted 5 lines

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.

Quoted 7 lines

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.

Yes it would be nice to design for this case as well.

Quoted 28 lines

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 ..
};

Quoted 7 lines

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

I suppose we can distribute the realias nodes when we compile the plugin into overlay fragments. The socket matching is a little vague though. How would we know which socket to apply to when we have two identical looking sockets? I'm thinking we could put some of that information into the fragment itself.

/{
	compatible = "foo,whirlgig-widget";

	fragment@0 { /* corresponds to i2c in example above */
		target-socket = "foo,widget-socket-a";
		target-alias = "i2c";
		__overlay__ {
			....
		};
	};

	fragment@1 { /* corresponds to i2c-b in example above */
		target-socket = "foo,widget-socket-b";
		target-alias = "i2c";
		__overlay__ {
			...
		};
	};
};

If we have two identical connectors maybe we'll have to enforce that the connectors have some more specific unique compatible string so that we can match up the right socket. But I don't see how we can require that the overlays know this detail if they only care about one socket and could go into either one of them. In that case we should have the loader ask the user which socket they connected this extension board to?

I was also thinking it would be better to leave the gpio-map and interrupt-map properties at the connector level. For example:

	widget1 {
		compatible = "foo,widget-socket";
		interrupt-map-mask = <0xffffffff>;
		interrupt-map = <0 &intc 7 0>,
				<1 &intc 8 0>;
	};

and then we could put a label on the plugin/expansion syntax so we can reference the connector as a whole:

	/plugin/ connector: foo,widget-socket {
		compatible = "foo,whirlgig-widget";
	};

	&i2c {
		device@40 {
			interrupt-parent = <&connector>;
			interrupts = <1>;
		};
	};

I also thought about making another alias inside the connector node to point to itself, but that fails when you get into the situation of two connectors and collisions, unless you rename them of course. It felt better to leave that choice to the overlay though.

In conclusion, I see a few topics/patterns emerging:

1) Expose phandles through the connectors in some way that allows us to limit what the plugin/expansion boards can modify or use

2) Have some flexible syntax to remap cell sizes from the baseboard through the connector to provide a consistent connector size (i.e. remap interrupts and gpios from multiple sources, etc. into a fixed number of cells)

3) Allow plugin/expansion boards to use multiple connectors from the baseboard in a consistent way

4) Attempt to maintain almost all of the current overlay syntax with syntactic sugar

5) Connectors on expansion boards should be supported

Is there anything else we should add to this list?

Quoted 28 lines

Quoting David Gibson (2016-08-29 06:45:11)

Quoted 19 lines

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 ..
};

Quoted 7 lines

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

Quoted 5 lines

I suppose we can distribute the realias nodes when we compile the plugin into overlay fragments. The socket matching is a little vague though. How would we know which socket to apply to when we have two identical looking sockets? I'm thinking we could put some of that information into the fragment itself.

So, my assumption in this example was that the plugin could plug into *any* two widget sockets. If it needs to connect to specific ones, then pretty much by definition, the sockets aren't really of indistinguishable type.

Quoted 13 lines

/{
      compatible = "foo,whirlgig-widget";

      fragment@0 { /* corresponds to i2c in example above */
              target-socket = "foo,widget-socket-a";
              target-alias = "i2c";
              __overlay__ {
                      ....
              };
      };

Quoted 8 lines

      fragment@1 { /* corresponds to i2c-b in example above */
              target-socket = "foo,widget-socket-b";
              target-alias = "i2c";
              __overlay__ {
                      ...
              };
      };
};

We don't need any new construct here. In this case the sockets aren't 100% compatible, which we can notate with

        compatible = "widget-socket-a", "widget-socket";

In the base board.

Devices which can plug into any widget socket will use target-socket = "widget-socket", those which require a specific one (including requiring both) can specifically list "widget-socket-a" and/or "widget-socket-b".

Quoted 11 lines

If we have two identical connectors maybe we'll have to enforce that the connectors have some more specific unique compatible string so that we can match up the right socket. But I don't see how we can require that the overlays know this detail if they only care about one socket and could go into either one of them. In that case we should have the loader ask the user which socket they connected this extension board to?

I was also thinking it would be better to leave the gpio-map and interrupt-map properties at the connector level. For example:

Quoted 6 lines

      widget1 {
              compatible = "foo,widget-socket";
              interrupt-map-mask = <0xffffffff>;
              interrupt-map = <0 &intc 7 0>,
                              <1 &intc 8 0>;
      };

That could work - but we should (and implicitly, do) support either way. Using subnodes might be useful for particularly complex irq or gpio mappings.

Quoted 20 lines

and then we could put a label on the plugin/expansion syntax so we can reference the connector as a whole:

      /plugin/ connector: foo,widget-socket {
              compatible = "foo,whirlgig-widget";
      };

      &i2c {
              device@40 {
                      interrupt-parent = <&connector>;
                      interrupts = <1>;
              };
      };

I also thought about making another alias inside the connector node to point to itself, but that fails when you get into the situation of two connectors and collisions, unless you rename them of course. It felt better to leave that choice to the overlay though.

Quoted 4 lines

In conclusion, I see a few topics/patterns emerging:

1) Expose phandles through the connectors in some way that allows us to limit what the plugin/expansion boards can modify or use

Yes, I definitely think we want that.

Quoted 4 lines

2) Have some flexible syntax to remap cell sizes from the baseboard through the connector to provide a consistent connector size (i.e. remap interrupts and gpios from multiple sources, etc. into a fixed number of cells)

I don't think we need any new constructs here. If there are mismatches we can put dummy bridges with appropriate ranges properties on one side or the other.

The only thing I see that might want some help is that the connector type should certainly imply a specific set of cell widths for all the included buses. So possibly we should supply some stuff to help enforce that.

Quoted 3 lines

3) Allow plugin/expansion boards to use multiple connectors from the baseboard in a consistent way

Seems reasonable.

Quoted 2 lines

4) Attempt to maintain almost all of the current overlay syntax with syntactic sugar

I'm not really sure what you mean by that.

Quoting David Gibson (2016-08-30 16:55:23)

Quoted 35 lines · Stephen Boyd

Quoted 33 lines

Quoting David Gibson (2016-08-29 06:45:11)

Quoted 19 lines

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 ..
};

Quoted 7 lines

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

I suppose we can distribute the realias nodes when we compile the plugin into overlay fragments. The socket matching is a little vague though. How would we know which socket to apply to when we have two identical looking sockets? I'm thinking we could put some of that information into the fragment itself.

Quoted 4 lines

So, my assumption in this example was that the plugin could plug into *any* two widget sockets. If it needs to connect to specific ones, then pretty much by definition, the sockets aren't really of indistinguishable type.

Ah ok. It wasn't clear on how we target where the extension board connects into the full tree. It sounds like the decision of where to plug in the extension board falls to the overlay manager then?

Quoted 22 lines

Quoted 11 lines

/{
      compatible = "foo,whirlgig-widget";

      fragment@0 { /* corresponds to i2c in example above */
              target-socket = "foo,widget-socket-a";
              target-alias = "i2c";
              __overlay__ {
                      ....
              };
      };

Quoted 8 lines

      fragment@1 { /* corresponds to i2c-b in example above */
              target-socket = "foo,widget-socket-b";
              target-alias = "i2c";
              __overlay__ {
                      ...
              };
      };
};

We don't need any new construct here. In this case the sockets aren't 100% compatible, which we can notate with

Quoted 7 lines

        compatible = "widget-socket-a", "widget-socket";

In the base board.

Devices which can plug into any widget socket will use target-socket = "widget-socket", those which require a specific one (including requiring both) can specifically list "widget-socket-a" and/or "widget-socket-b".

Agreed. How do we express the realias node in the overlay format though? I was trying to come up with some way to make that work by redistributing the alias to the fragments but that doesn't work so well.

Quoted 17 lines

Quoted 10 lines

If we have two identical connectors maybe we'll have to enforce that the connectors have some more specific unique compatible string so that we can match up the right socket. But I don't see how we can require that the overlays know this detail if they only care about one socket and could go into either one of them. In that case we should have the loader ask the user which socket they connected this extension board to?

I was also thinking it would be better to leave the gpio-map and interrupt-map properties at the connector level. For example:

Quoted 6 lines

      widget1 {
              compatible = "foo,widget-socket";
              interrupt-map-mask = <0xffffffff>;
              interrupt-map = <0 &intc 7 0>,
                              <1 &intc 8 0>;
      };

Quoted 3 lines

That could work - but we should (and implicitly, do) support either way. Using subnodes might be useful for particularly complex irq or gpio mappings.

Sounds good.

Quoted 32 lines

Quoted 20 lines

and then we could put a label on the plugin/expansion syntax so we can reference the connector as a whole:

      /plugin/ connector: foo,widget-socket {
              compatible = "foo,whirlgig-widget";
      };

      &i2c {
              device@40 {
                      interrupt-parent = <&connector>;
                      interrupts = <1>;
              };
      };

I also thought about making another alias inside the connector node to point to itself, but that fails when you get into the situation of two connectors and collisions, unless you rename them of course. It felt better to leave that choice to the overlay though.

Quoted 4 lines

In conclusion, I see a few topics/patterns emerging:

1) Expose phandles through the connectors in some way that allows us to limit what the plugin/expansion boards can modify or use

Yes, I definitely think we want that.

Quoted 4 lines

2) Have some flexible syntax to remap cell sizes from the baseboard through the connector to provide a consistent connector size (i.e. remap interrupts and gpios from multiple sources, etc. into a fixed number of cells)

Quoted 8 lines

I don't think we need any new constructs here. If there are mismatches we can put dummy bridges with appropriate ranges properties on one side or the other.

The only thing I see that might want some help is that the connector type should certainly imply a specific set of cell widths for all the included buses. So possibly we should supply some stuff to help enforce that.

I'm specifically thinking that anything that has a #<name>-cells associated with it (#gpio-cells, #clock-cells, #power-domain-cells, etc.) would need to have an associated <name>-map property and associated parsing code so that we can make a consistent cell width for the connector. If we have existing ways to make this work, e.g. interrupt-map or ranges, then we don't need.

Quoted 10 lines

Quoted 4 lines

3) Allow plugin/expansion boards to use multiple connectors from the baseboard in a consistent way

Seems reasonable.

Quoted 2 lines

4) Attempt to maintain almost all of the current overlay syntax with syntactic sugar

Quoted 2 lines

I'm not really sure what you mean by that.

I mean that we're not really trying to change the general structure of the DT overlay syntax. At least from what I can tell we're planning to convert this /plugin/ format into a DT overlay that gets compiled into binary all inside DTC.

Quoted 6 lines

Quoting David Gibson (2016-08-30 16:55:23)

Quoted 4 lines

So, my assumption in this example was that the plugin could plug into *any* two widget sockets. If it needs to connect to specific ones, then pretty much by definition, the sockets aren't really of indistinguishable type.

Quoted 3 lines

Ah ok. It wasn't clear on how we target where the extension board connects into the full tree. It sounds like the decision of where to plug in the extension board falls to the overlay manager then?

Yes. The whole point of connectors is that they're interchangeable, so neither the "plug" side nor the "socket" side should specify which instance of the other part they're connected to.

So to resolve the whole tree in this case, extra information would be needed. Ideally it would come from probing (even something ad-hoc like an ID register), failing that it would need to be admin configured.

Quoted 5 lines

Agreed. How do we express the realias node in the overlay format though? I was trying to come up with some way to make that work by redistributing the alias to the fragments but that doesn't work so well.

So, I'm thinking of the overlay dt info in 3 pieces

1) Global metadata

Not much here yet, but it seems like it could be useful.

2) Per-connector metadata

This specifies what sort of "socket" each "plug" on the overlay needs to connect to, and any other information we need that's specific to a particular "plug"

3) DT fragments

The actual device info applied once we've resolved all the aliases and whatnot.

The realias information would need to be in part (2)

How we encode those pieces into the final overlay dt is pretty close to an arbitrary choice.

Quoted 10 lines

Quoted 9 lines

I don't think we need any new constructs here. If there are mismatches we can put dummy bridges with appropriate ranges properties on one side or the other.

The only thing I see that might want some help is that the connector type should certainly imply a specific set of cell widths for all the included buses. So possibly we should supply some stuff to help enforce that.

Quoted 6 lines

I'm specifically thinking that anything that has a #<name>-cells associated with it (#gpio-cells, #clock-cells, #power-domain-cells, etc.) would need to have an associated <name>-map property and associated parsing code so that we can make a consistent cell width for the connector. If we have existing ways to make this work, e.g. interrupt-map or ranges, then we don't need.

Yes, I agree. All the "interrupt tree like" things need interrupt tree like nexus nodes, with a map and map-mask.

However, we also need some way of ensuring that the #address-cells and #size-cells for any buses we expose match properly.

Or maybe we need to invent something like a cross between 'ranges' and 'interrupt-map' that can remap ranges on one address space to the address space attached to a different node.

Quoted 10 lines

Quoted 5 lines

Quoted 2 lines

4) Attempt to maintain almost all of the current overlay syntax with syntactic sugar

I'm not really sure what you mean by that.

I mean that we're not really trying to change the general structure of the DT overlay syntax. At least from what I can tell we're planning to convert this /plugin/ format into a DT overlay that gets compiled into binary all inside DTC.

Roughly speaking, yes. However I think we should take the opportunity to address design flaws in the current overlay encoding as they come up (e.g. the fact that it uses phandles which *always* have to be resolved - better to directly use an alias / symbol / label).