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

On Mon, Aug 29, 2016 at 2:24 PM, Rob Herring [off-list ref] wrote:

On Mon, Aug 29, 2016 at 6:51 AM, Arnd Bergmann [off-list ref] wrote:

quoted

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@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

quoted

quoted

ebi2@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@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@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@1a100000 {
    compatible = "qcom,apq8060-ebi2";
    #address-cells = <1>;
    #size-cells = <1>;
    reg = <0x1a100000 0x1000>, <0x1a110000 0x1000>;
    reg-names = "ebi2", "xmem";

   cs@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@0 {
           compatible = "foo";
           reg = <0x0 0x100>;
           (...)
       };

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