[PATCH 1/4] soc: qcom: add an EBI2 device tree bindings

[Linus Walleij]

On Mon, Aug 29, 2016 at 2:24 PM, Rob Herring <robh+dt at kernel.org> wrote:
> On Mon, Aug 29, 2016 at 6:51 AM, Arnd Bergmann <arnd at arndb.de> wrote:

>> I would have one node per chip-select and then put the devices on
>> that CS below it, with #address-cells=1 again.
>
> Actually, how I've been guiding folks is 2 levels of nodes.
>
> eim {
>   #address-cells = <3>;
>   cs at 0,0 {
>     compatible = "bar,some-device";
>     foo,eim-timing-prop = <0>;
>   };
>
> This is how most bindings have been done. There's not really any point
> to 3 levels other than keeping CS properties separate, but they don't
> need to be if they are properly prefixed. The timing for a device is a
> property of the device even though we express them in terms of the
> controller.

This is what I'm doing in the most recent version from aug 24 I think.
http://marc.info/?l=linux-arm-kernel&m=147202889622540&w=2

>>> ebi2 at 1a100000 {
>>>        compatible = "qcom,msm8660-ebi2";
>>>        #address-cells = <2>;
>>>        #size-cells = <1>;
>>>        qcom,xmem-recovery-cycles = <0>, <0>, <5>, <0>, <0>, <0>;
>>
>> No, better put the settings into one device per cs.
>
> Yes.

Gnah. Each chipselect can (in theory) house several memory-mapped
devices at different offsets. Like two ethernet controllers. It gets a bit
weird.

This is how it currently looks (v2):

ebi2 at 1a100000 {
    compatible = "qcom,apq8060-ebi2";
    #address-cells = <2>;
    #size-cells = <1>;
    ranges = <0 0x0 0x1a800000 0x00800000>,
        <1 0x0 0x1b000000 0x00800000>,
        <2 0x0 0x1b800000 0x00800000>,
        <3 0x0 0x1d000000 0x00800000>,
        <4 0x0 0x1c800000 0x00800000>,
        <5 0x0 0x1c000000 0x00800000>;
    reg = <0x1a100000 0x1000>, <0x1a110000 0x1000>;
    reg-names = "ebi2", "xmem";
    qcom,xmem-recovery-cycles = <0>, <0>, <0>, <0>, <0>, <0>;
    qcom,xmem-write-hold-cycles = <0>, <0>, <3>, <0>, <0>, <0>;
    qcom,xmem-write-delta-cycles = <0>, <0>, <31>, <0>, <0>, <0>;
    qcom,xmem-read-delta-cycles = <0>, <0>, <28>, <0>, <0>, <0>;
    qcom,xmem-write-wait-cycles = <0>, <0>, <9>, <0>, <0>, <0>;
    qcom,xmem-read-wait-cycles = <0>, <0>, <9>, <0>, <0>, <0>;

    foo-ebi2 at 1b800000 {
        compatible = "foo";
        reg = <2 0x0 0x100>;
        (...)
    };
};

That is very close to how
Documentation/devicetree/bindings/bus/imx-weim.txt
drivers/bus/imx-weim.c
does things, just with the additional property arrays.

By looping over the subnodes and inspecting the first address cell of
them I can figure out what chipselects need to be turned on, and
the settings for those chipselects I need to activate are taken from
those 6 qcom,xmem* arrays of timings.

So if I reintroduce the cs nodes how do you imagine it'd look? Like this?

ebi2 at 1a100000 {
    compatible = "qcom,apq8060-ebi2";
    #address-cells = <1>;
    #size-cells = <1>;
    reg = <0x1a100000 0x1000>, <0x1a110000 0x1000>;
    reg-names = "ebi2", "xmem";

   cs at 2 {
       compatible = "bar,some-device";
       #address-cells = <1>;
       #size-cells = <1>;
       ranges = <0x0 0x1b800000 0x00800000>;
       chipselect = <2>;
       foo,eim-timing-prop = <0>;
       qcom,xmem-recovery-cycles = <0>;
       qcom,xmem-write-hold-cycles = <3>;
       qcom,xmem-write-delta-cycles = <31>;
       qcom,xmem-read-delta-cycles = <28>;
       qcom,xmem-write-wait-cycles = <9>;
       qcom,xmem-read-wait-cycles = <9>;

       foo-ebi2 at 0 {
           compatible = "foo";
           reg = <0x0 0x100>;
           (...)
       };

       bar-ebi2 at 100 {
           compatible = "bar";
           reg = <0x100 0x100>;
           (...)
       };

    };
};

Note: the chipselect goes out of the address cell, into the old
chipselect = <n> property, because we have no other way to know
which chipselect the settings apply to! (It's not like we can parse
the subnodes and make a majority decision...)

I kind of think both are non-perfect but the first one is the lesser
evil.

Yours,
Linus Walleij