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 -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html