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 <da...@gibson.dropbear.id.au> 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
