On Mon, Aug 29, 2016 at 2:24 PM, Rob Herring <robh+dt@xxxxxxxxxx> wrote:
> On Mon, Aug 29, 2016 at 6:51 AM, Arnd Bergmann <arnd@xxxxxxxx> 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@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@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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
