Re: DT connectors, thoughts

[Pantelis Antoniou]

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

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.

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

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

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

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

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

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

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

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

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

Regards

â Pantelis