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 [off-list ref] 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