lkml.org 
[lkml]   [2016]   [Jul]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: DT connectors, thoughts
    From
    Date
    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@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

    \
     
     \ /
      Last update: 2016-07-20 23:41    [W:6.044 / U:0.548 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site