ARM: tegra: emc: device tree bindings

First cut at device tree bindings for the EMC tables on tegra.

Note that I have a prerequisite patch that changes the tegra2_emc code to be a platform driver; but I wanted to do a sanity-check of my device tree usage here before posting the whole series.

Signed-off-by Olof Johansson <[email protected]>

---

?.../devicetree/bindings/arm/tegra/emc.txt ? ? ? ? ?| ? 77 ++++++++++++++++++++
?arch/arm/boot/dts/tegra-seaboard.dts ? ? ? ? ? ? ? | ? 36 +++++++++
?arch/arm/boot/dts/tegra20.dtsi ? ? ? ? ? ? ? ? ? ? | ? ?7 ++

?3 files changed, 120 insertions(+), 0 deletions(-)
?create mode 100644 Documentation/devicetree/bindings/arm/tegra/emc.txt

Documentation/devicetree/bindings/arm/tegra/emc.txt new file +40 -0

new file mode 100644
index 0000000..dd7f845
@@ -0,0 +1,77 @@
+Embedded Memory Controller
+
+Properties:
+- name : Should be emc
+- #address-cells : Should be 1
+- #size-cells : Should be 0
+- compatible : should contain "nvidia,tegra20-emc".
+- reg : Offset and length of the register set for the device
+- nvidia,use-ram-code : If present, the sub-nodes will be addressed
+ ?and chosen using the ramcode board selector. If omitted, only one
+ ?set of tables can be present and said tables will be used
+ ?irrespective of ram-code configuration.
+
+Child device nodes describe the memory settings for different configurations and clock rates.
+
+Example:
+
+ ? ? ? emc at 7000f400 {
+ ? ? ? ? ? ? ? #address-cells = < 1 >;
+ ? ? ? ? ? ? ? #size-cells = < 0 >;
+ ? ? ? ? ? ? ? compatible = "nvidia,tegra20-emc";
+ ? ? ? ? ? ? ? reg = <0x7000f4000 0x200>;
+ ? ? ? }
+
+
+Embedded Memory Controller configuration table
+
+This is a table containing the EMC register settings for the various
+operating speeds of the memory controller. They are always located as
+subnodes of the emc controller node.
+
+There are two ways of specifying which tables to use:
+
+* The simplest is if there is just one set of tables in the device tree,
+ ?and they will always be used (based on which frequency is used).
+ ?This is the preferred method, especially when firmware can fill in
+ ?this information based on the specific system information and just
+ ?pass it on to the kernel.
+
+* The slightly more complex one is when more than one memory configuration

Documentation/devicetree/bindings/arm/tegra/emc.txt new file +27 -0

+ ?parent. If so, the numerical representation of the selected ram code
+ ?as reported by the strap option APB misc register.
+- clock-frequency : the clock frequency for the EMC at which this
+ ?table should be used (in kHz).
+- nvidia,emc-registers : a 46 word array of EMC registers to be programmed
+ ?for operation at the 'clock-frequency' setting.
+ ?The order and contents of the registers are defined in the Tegra TRM.
+
+ ? ? ? ? ? ? ? emc-table-333mhz at 0 {
+ ? ? ? ? ? ? ? ? ? ? ? reg = <0>;
+ ? ? ? ? ? ? ? ? ? ? ? compatible = "nvidia,tegra20-emc-table";
+ ? ? ? ? ? ? ? ? ? ? ? clock-frequency = < 333000 >;
+ ? ? ? ? ? ? ? ? ? ? ? nvidia,emc-registers = < 0 0 0 0 0 0 0 0 0 0 0 0 0 0
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?0 0 0 0 0 0 0 0 0 0 0 0 0 0
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?0 0 0 0 0 0 0 0 0 0 0 0 0 0
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?0 0 0 0 >;
+ ? ? ? ? ? ? ? };
+
+ ? ? ? ? ? ? ? emc-table-666mhz at 0 {
+ ? ? ? ? ? ? ? ? ? ? ? reg = <0>;
+ ? ? ? ? ? ? ? ? ? ? ? compatible = "nvidia,tegra20-emc-table";
+ ? ? ? ? ? ? ? ? ? ? ? clock-frequency = < 666000 >;
+ ? ? ? ? ? ? ? ? ? ? ? nvidia,emc-registers = < 0 0 0 0 0 0 0 0 0 0 0 0 0 0
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?0 0 0 0 0 0 0 0 0 0 0 0 0 0
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?0 0 0 0 0 0 0 0 0 0 0 0 0 0
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?0 0 0 0 >;
+ ? ? ? ? ? ? ? };

arch/arm/boot/dts/tegra-seaboard.dts +36 -0

index a72299b..2d60b5ff 100644
@@ -20,6 +20,42 @@
 ? ? ? ? ? ? ? ?clock-frequency = < 216000000 >;
 ? ? ? ?};
 
+ ? ? ? emc {
+ ? ? ? ? ? ? ? emc-table-166 {
+ ? ? ? ? ? ? ? ? ? ? ? compatible = "nvidia,tegra20-emc-table";
+ ? ? ? ? ? ? ? ? ? ? ? clock-frequency = < 166500 >;
+ ? ? ? ? ? ? ? ? ? ? ? nvidia,emc-registers = < 0x0000000a 0x00000021
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000008 0x00000003 0x00000004 0x00000004
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000002 0x0000000c 0x00000003 0x00000003
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000002 0x00000001 0x00000004 0x00000005
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000004 0x00000009 0x0000000d 0x000004df
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000000 0x00000003 0x00000003 0x00000003
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000003 0x00000001 0x0000000a 0x000000c8
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000003 0x00000006 0x00000004 0x0000000f
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000002 0x00000000 0x00000000 0x00000002
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000000 0x00000000 0x00000083 0xa04004ae
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x007fd010 0x00000000 0x00000000 0x00000000
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000000 0x00000000 0x00000000 0x00000000 >;
+ ? ? ? ? ? ? ? };
+
+ ? ? ? ? ? ? ? emc-table-333 {
+ ? ? ? ? ? ? ? ? ? ? ? compatible = "nvidia,tegra20-emc-table";
+ ? ? ? ? ? ? ? ? ? ? ? clock-frequency = < 333000 >;
+ ? ? ? ? ? ? ? ? ? ? ? nvidia,emc-registers = < 0x00000014 0x00000041
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x0000000f 0x00000005 0x00000004 0x00000005
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000003 0x0000000c 0x00000005 0x00000005
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000003 0x00000001 0x00000004 0x00000005
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000004 0x00000009 0x0000000d 0x000009ff
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000000 0x00000003 0x00000003 0x00000005
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000005 0x00000001 0x0000000f 0x000000c8
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000003 0x0000000c 0x00000006 0x0000000f
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000002 0x00000000 0x00000000 0x00000002
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000000 0x00000000 0x00000083 0xe034048b
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x007e8010 0x00000000 0x00000000 0x00000000
+ ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0x00000000 0x00000000 0x00000000 0x00000000 >;
+ ? ? ? ? ? ? ? };
+ ? ? ? };
+
 ? ? ? ?sdhci at c8000400 {

? ? ? ? ? ? ? ?cd-gpios = <&gpio 69 0>; /* gpio PI5 */ ? ? ? ? ? ? ? ?wp-gpios = <&gpio 57 0>; /* gpio PH1 */

arch/arm/boot/dts/tegra20.dtsi +7 -1

index 65d7e6a..f4dc8d3 100644
@@ -120,6 +120,13 @@
 ? ? ? ? ? ? ? ?interrupts = < 123 >;
 ? ? ? ?};
 
+ ? ? ? emc at 7000f400 {
+ ? ? ? ? ? ? ? #address-cells = <1>;
+ ? ? ? ? ? ? ? #size-cells = <0>;
+ ? ? ? ? ? ? ? compatible = "nvidia,tegra20-emc";
+ ? ? ? ? ? ? ? reg = <0x7000f400 0x200>;
+ ? ? ? };
+
 ? ? ? ?sdhci at c8000000 {
 ? ? ? ? ? ? ? ?compatible = "nvidia,tegra20-sdhci";
 ? ? ? ? ? ? ? ?reg = <0xc8000000 0x200>;
--

1.7.4.1

_______________________________________________
devicetree-discuss mailing list
devicetree-discuss at lists.ozlabs.org
https://lists.ozlabs.org/listinfo/devicetree-discuss

First cut at device tree bindings for the EMC tables on tegra.

Note that I have a prerequisite patch that changes the tegra2_emc code to be a platform driver; but I wanted to do a sanity-check of my device tree usage here before posting the whole series.

+++ b/Documentation/devicetree/bindings/arm/tegra/emc.txt

+Embedded Memory Controller configuration table

Olof Johansson wrote at Monday, October 17, 2011 11:53 AM:

Quoted 5 lines

First cut at device tree bindings for the EMC tables on tegra.

Note that I have a prerequisite patch that changes the tegra2_emc code to be a platform driver; but I wanted to do a sanity-check of my device tree usage here before posting the whole series.

...

Quoted 1 line

+++ b/Documentation/devicetree/bindings/arm/tegra/emc.txt

...

Documentation/devicetree/bindings/arm/tegra/emc.txt line 6

Quoted 6 lines

Documentation/devicetree/bindings/arm/tegra/emc.txt new file +6 -0

+Embedded Memory Controller
+
+Properties:
+- name : Should be emc
+- #address-cells : Should be 1
+- #size-cells : Should be 0

The address/size-cells properties define the require number of cells for child nodes, where the child nodes are addressable objects in a bus. For the EMC table child nodes, they're more configuration than nodes on a bus, so I don't think it's appropriate for the EMC controller to define the address/size-cells properties. Those properties would be appropriate if we ever have child nodes for the individual SDRAM chips, but I suspect we won't ever do that?

...

Quoted 1 line

+Embedded Memory Controller configuration table

...

Quoted 7 lines

+Properties:
+- name : Should start with emc-table

+- compatible : should contain "nvidia,tegra20-emc-table".
+- reg : only needed if nvidia,use-ram-code is present in the

+ ?parent. If so, the numerical representation of the selected ram code
+ ?as reported by the strap option APB misc register.

I don't think the compatible or reg properties are needed; the EMC tables aren't addressable objects on a bus, so no need for reg. I could see an argument that we'd want to version the emc-table format, and so the compatible flag might be useful, yet AIUI, compatible is more for defining HW compatibility (and hence driver instantiation), rather than a property of some configuration node.

Quoted 5 lines

+- clock-frequency : the clock frequency for the EMC at which this
+ ?table should be used (in kHz).
+- nvidia,emc-registers : a 46 word array of EMC registers to be programmed
+ ?for operation at the 'clock-frequency' setting.
+ ?The order and contents of the registers are defined in the Tegra TRM.

That's not a very semantic representation. Still, I guess there isn't any benefit representing this at a higher level.

Review line 23

Quoted 6 lines

+++ b/arch/arm/boot/dts/tegra-seaboard.dts

(quoted diff) +1 -0

@@ -20,6 +20,42 @@
 ? ? ? ? ? ? ? clock-frequency = < 216000000 >;
 ? ? ? };
 
+ ? ? emc {

I think that should be emc at 7000f400, so it matches tegra20.dtsi?

Quoted 12 lines

Olof Johansson wrote at Monday, October 17, 2011 11:53 AM:
...

Quoted 1 line

+Embedded Memory Controller configuration table

...

Quoted 7 lines

+Properties:
+- name : Should start with emc-table

+- compatible : should contain "nvidia,tegra20-emc-table".
+- reg : only needed if nvidia,use-ram-code is present in the

+ ?parent. If so, the numerical representation of the selected ram code
+ ?as reported by the strap option APB misc register.

Quoted 6 lines

I don't think the compatible or reg properties are needed; the EMC tables aren't addressable objects on a bus, so no need for reg. I could see an argument that we'd want to version the emc-table format, and so the compatible flag might be useful, yet AIUI, compatible is more for defining HW compatibility (and hence driver instantiation), rather than a property of some configuration node.

I was struggling with a good way to specify the selection of the modules. I can definitely use a nvidia,ram-code property instead of reg (with a similar definition to how reg was used here).

Compatible is still needed, in my opinion -- otherwise there will be no way to tell if the node is there to describe emc timings or if it's some new node used to describe something else (such as SDRAM chips as mentioned above).

Olof Johansson wrote at Tuesday, October 18, 2011 12:43 PM:

Quoted 20 lines · Stephen Warren

Quoted 18 lines

Olof Johansson wrote at Monday, October 17, 2011 11:53 AM:
...

Quoted 1 line

+Embedded Memory Controller configuration table

...

Quoted 7 lines

+Properties:
+- name : Should start with emc-table

+- compatible : should contain "nvidia,tegra20-emc-table".
+- reg : only needed if nvidia,use-ram-code is present in the

+ ?parent. If so, the numerical representation of the selected ram code
+ ?as reported by the strap option APB misc register.

I don't think the compatible or reg properties are needed; the EMC tables aren't addressable objects on a bus, so no need for reg. I could see an argument that we'd want to version the emc-table format, and so the compatible flag might be useful, yet AIUI, compatible is more for defining HW compatibility (and hence driver instantiation), rather than a property of some configuration node.

Quoted 3 lines

I was struggling with a good way to specify the selection of the modules. I can definitely use a nvidia,ram-code property instead of reg (with a similar definition to how reg was used here).

Ah, so reg is the ram-code value. I missed that. Using an explicitly named property for this seems better to me, but I'll defer to DT experts on what's the standard practice for this.

Quoted 4 lines

Compatible is still needed, in my opinion -- otherwise there will be no way to tell if the node is there to describe emc timings or if it's some new node used to describe something else (such as SDRAM chips as mentioned above).

Can't you go by node name; enumerate all nodes with a particular name. Or define another intermediate node that will always contain tables and nothing else, then just enumerate all child nodes of that node:

emc at xxxxx {
? ?emc-tables {
? ? ? ?table-333 at 0 {};
? ? ? ?table-666 at 0 {};
? ?};
};

The Tegra pinmux bindings I proposed certainly used this technique; a main node with a well-known name, followed by enumeration of all child nodes of that, and nobody /said/ anything about that being a bad idea.

Quoted 1 line

Olof Johansson wrote at Tuesday, October 18, 2011 12:43 PM:

Quoted 16 lines

Quoted 4 lines

Compatible is still needed, in my opinion -- otherwise there will be no way to tell if the node is there to describe emc timings or if it's some new node used to describe something else (such as SDRAM chips as mentioned above).

Can't you go by node name; enumerate all nodes with a particular name. Or define another intermediate node that will always contain tables and nothing else, then just enumerate all child nodes of that node:

emc at xxxxx {
? ?emc-tables {
? ? ? ?table-333 at 0 {};
? ? ? ?table-666 at 0 {};
? ?};
};

Quoted 3 lines

The Tegra pinmux bindings I proposed certainly used this technique; a main node with a well-known name, followed by enumeration of all child nodes of that, and nobody /said/ anything about that being a bad idea.

I'm not really picky on this, but I think I would rather use a compatible field than rely on naming.

That being said, doing a two-level approach will probably make it easier than the flat structure I initially had. So:

emc at xxx {
    nvidia,use-ram-code;
    emc-table-ram-code-0 {
        nvidia,ram-code = < 0 >;
        table-166 { compatible = "tegra20-emc-table"; ... };
        table-333 { ... };
    };

    emc-table-ram-code-1 {
        nvidia,ram-code = < 1 >;
        ...
    };
};

... and for none-ram-code, just leave out the emc-table-ramcode-x level.

So, for nvidia,use-ram-code case, it'll be one intermediate step of finding the right subnode, the rest of the table setup code will be common. None of it will be bound to actual node names though -- first step is iterating child nodes looking for nvidia,ram-code properties to match, and second step iterates by matching compatible fields.

Olof Johansson wrote at Tuesday, October 18, 2011 2:54 PM:

Quoted 2 lines · Stephen Warren

Quoted 1 line

Olof Johansson wrote at Tuesday, October 18, 2011 12:43 PM:

...

Quoted 40 lines

Quoted 16 lines

Quoted 4 lines

Compatible is still needed, in my opinion -- otherwise there will be no way to tell if the node is there to describe emc timings or if it's some new node used to describe something else (such as SDRAM chips as mentioned above).

Can't you go by node name; enumerate all nodes with a particular name. Or define another intermediate node that will always contain tables and nothing else, then just enumerate all child nodes of that node:

emc at xxxxx {
   emc-tables {
       table-333 at 0 {};
       table-666 at 0 {};
   };
};

Quoted 3 lines

The Tegra pinmux bindings I proposed certainly used this technique; a main node with a well-known name, followed by enumeration of all child nodes of that, and nobody /said/ anything about that being a bad idea.

I'm not really picky on this, but I think I would rather use a compatible field than rely on naming.

That being said, doing a two-level approach will probably make it easier than the flat structure I initially had. So:

emc at xxx {
    nvidia,use-ram-code;
    emc-table-ram-code-0 {
        nvidia,ram-code = < 0 >;
        table-166 { compatible = "tegra20-emc-table"; ... };
        table-333 { ... };
    };

    emc-table-ram-code-1 {
        nvidia,ram-code = < 1 >;
        ...
    };
};

Quoted 7 lines

... and for none-ram-code, just leave out the emc-table-ramcode-x level.

So, for nvidia,use-ram-code case, it'll be one intermediate step of finding the right subnode, the rest of the table setup code will be common. None of it will be bound to actual node names though -- first step is iterating child nodes looking for nvidia,ram-code properties to match, and second step iterates by matching compatible fields.

I only suggested the well-known-named sub-nodes in order to eliminate the need for a compatible property.

My inclination is that if we use compatible to distinguish the tables from anything else, there's little point having the extra level of nodes; we may as well lay it out as in your original patch, just with an explicit nvidia,ram-code property in each table (or omitted/ignored when not using it) instead of reg?

Quoted 3 lines · Stephen Warren

I only suggested the well-known-named sub-nodes in order to eliminate the need for a compatible property.

Quoted 5 lines

My inclination is that if we use compatible to distinguish the tables from anything else, there's little point having the extra level of nodes; we may as well lay it out as in your original patch, just with an explicit nvidia,ram-code property in each table (or omitted/ignored when not using it) instead of reg?

Node names should be generic like serial or ethernet. Compatible is used to specify the specific model.

In cases where unit addresses can be used to separate out identical entries, yes. For something like this, there's no logical addressing of the tables so something else must be used to distinguish them.

Quoted 10 lines · Rob Herring

Quoted 8 lines · Stephen Warren

I only suggested the well-known-named sub-nodes in order to eliminate the need for a compatible property.

My inclination is that if we use compatible to distinguish the tables from anything else, there's little point having the extra level of nodes; we may as well lay it out as in your original patch, just with an explicit nvidia,ram-code property in each table (or omitted/ignored when not using it) instead of reg?

Quoted 2 lines

Node names should be generic like serial or ethernet. Compatible is used to specify the specific model.

In cases where unit addresses can be used to separate out identical entries, yes. For something like this, there's no logical addressing of the tables so something else must be used to distinguish them.

Quoted 14 lines · Olof Johansson

Quoted 12 lines · Rob Herring

Quoted 8 lines · Stephen Warren

I only suggested the well-known-named sub-nodes in order to eliminate the need for a compatible property.

My inclination is that if we use compatible to distinguish the tables from anything else, there's little point having the extra level of nodes; we may as well lay it out as in your original patch, just with an explicit nvidia,ram-code property in each table (or omitted/ignored when not using it) instead of reg?

Node names should be generic like serial or ethernet. Compatible is used to specify the specific model.

Quoted 3 lines

In cases where unit addresses can be used to separate out identical entries, yes. For something like this, there's no logical addressing of the tables so something else must be used to distinguish them.

Using the frequency as was previously proposed would work assuming that is unique.

Quoted 2 lines

Using the frequency as was previously proposed would work assuming that is unique.

Someone suggested (off-list) to just use dummy addressing like cpu nodes do. Sounds reasonable to me.

So:

emc {
compatible = "tegra20-emc";
nvidia,use-ram-code;

  emc-tables@<ram-code> {
     nvidia,ram-code = < <ram-code> >;
     emc-table@<dummy enumerator> {
        compatible = "tegra20-emc-table";
        clock-frequency = < >;
        nvidia,emc-regs = < >;
     }

}

This also avoids having to handle 2-dimensional dummy numbering (i.e. <ramcode,table-number>) by breaking it in two levels:

Where nvidia,use-ram-code is missing in the emc node, the immediate child nodes will be scanned for the compatible nodes
Where nvidia,use-ram-code is present, first scan will be of all child nodes containing an nvidia,ram-code property, then from there treat it the same as the first case.

In the above, none of the names have meaning, so they can be changed as needed (but these seem like a reasonably generic and descriptive name to me).

Olof Johansson wrote at Wednesday, October 19, 2011 9:07 AM:

Quoted 1 line · Rob Herring

...

Quoted 29 lines

Quoted 2 lines

Using the frequency as was previously proposed would work assuming that is unique.

Someone suggested (off-list) to just use dummy addressing like cpu nodes do. Sounds reasonable to me.

So:

emc {
? compatible = "tegra20-emc";
? nvidia,use-ram-code;
? emc-tables@<ram-code> {
? ? ?nvidia,ram-code = < <ram-code> >;
? ? ?emc-table@<dummy enumerator> {
? ? ? ? compatible = "tegra20-emc-table";
? ? ? ? clock-frequency = < >;
? ? ? ? nvidia,emc-regs = < >;
? ? ?}
? }

This also avoids having to handle 2-dimensional dummy numbering (i.e. <ramcode,table-number>) by breaking it in two levels:

Where nvidia,use-ram-code is missing in the emc node, the immediate child nodes will be scanned for the compatible nodes
Where nvidia,use-ram-code is present, first scan will be of all child nodes containing an nvidia,ram-code property, then from there treat it the same as the first case.

Quoted 3 lines

In the above, none of the names have meaning, so they can be changed as needed (but these seem like a reasonably generic and descriptive name to me).

OK, I'm good with that general structure.

But, as Rob suggests, you may as well use the frequency instead of the <dummy enumerator> for the unit address of the final tables, right? The search algorithm might not care, but it'll be easier for humans to read the resulting dts file.

emc {
compatible = "tegra20-emc";
nvidia,use-ram-code;

  emc-tables@<ram-code> {
     nvidia,ram-code = < <ram-code> >;
     emc-table@<dummy enumerator> {
        compatible = "tegra20-emc-table";
        clock-frequency = < >;
        nvidia,emc-regs = < >;
     }

}

Quoted 11 lines

emc {
compatible = "tegra20-emc";
nvidia,use-ram-code;

  emc-tables@<ram-code> {
     nvidia,ram-code = < <ram-code> >;
     emc-table@<dummy enumerator> {
        compatible = "tegra20-emc-table";
        clock-frequency = < >;
        nvidia,emc-regs = < >;
     }

}

You need to have a reg in the emc-table node, and #address-cells #size-cells
in its parent node.

Quoted 11 lines

emc {
?compatible = "tegra20-emc";
?nvidia,use-ram-code;
?emc-tables@<ram-code> {
? ? nvidia,ram-code = < <ram-code> >;
? ? emc-table@<dummy enumerator> {
? ? ? ?compatible = "tegra20-emc-table";
? ? ? ?clock-frequency = < >;
? ? ? ?nvidia,emc-regs = < >;
? ? }
?}

You need to have a reg in the emc-table node, and #address-cells #size-cells in its parent node.

Olof Johansson wrote at Tuesday, October 18, 2011 2:54 PM:

Quoted 2 lines · Stephen Warren

Quoted 1 line

Olof Johansson wrote at Tuesday, October 18, 2011 12:43 PM:

...

Quoted 42 lines

Quoted 16 lines

Quoted 4 lines

Compatible is still needed, in my opinion -- otherwise there will be no way to tell if the node is there to describe emc timings or if it's some new node used to describe something else (such as SDRAM chips as mentioned above).

Can't you go by node name; enumerate all nodes with a particular name. Or define another intermediate node that will always contain tables and nothing else, then just enumerate all child nodes of that node:

emc at xxxxx {
? ?emc-tables {
? ? ? ?table-333 at 0 {};
? ? ? ?table-666 at 0 {};
? ?};
};

Quoted 3 lines

The Tegra pinmux bindings I proposed certainly used this technique; a main node with a well-known name, followed by enumeration of all child nodes of that, and nobody /said/ anything about that being a bad idea.

I'm not really picky on this, but I think I would rather use a compatible field than rely on naming.

That being said, doing a two-level approach will probably make it easier than the flat structure I initially had. So:

emc at xxx {
? ? nvidia,use-ram-code;
? ? emc-table-ram-code-0 {
? ? ? ? nvidia,ram-code = < 0 >;
? ? ? ? table-166 { compatible = "tegra20-emc-table"; ... }; ? ? ? ? table-333 { ... };
? ? };

? ? emc-table-ram-code-1 {
? ? ? ? nvidia,ram-code = < 1 >;
? ? ? ? ...
? ? };
};

... and for none-ram-code, just leave out the emc-table-ramcode-x level.

Quoted 5 lines

So, for nvidia,use-ram-code case, it'll be one intermediate step of finding the right subnode, the rest of the table setup code will be common. None of it will be bound to actual node names though -- first step is iterating child nodes looking for nvidia,ram-code properties to match, and second step iterates by matching compatible fields.

I only suggested the well-known-named sub-nodes in order to eliminate the need for a compatible property.

My inclination is that if we use compatible to distinguish the tables from anything else, there's little point having the extra level of nodes; we may as well lay it out as in your original patch, just with an explicit nvidia,ram-code property in each table (or omitted/ignored when not using it) instead of reg?