On Tue, Sep 25, 2018 at 7:11 AM Peter De Schrijver
[off-list ref] wrote:

On Mon, Sep 24, 2018 at 02:04:24PM -0700, Rob Herring wrote:

quoted

On Fri, Sep 14, 2018 at 11:03:09PM +0300, Peter De Schrijver wrote:

quoted

Signed-off-by: Peter De Schrijver <redacted>

Needs a commit msg.

quoted

---
 .../memory-controllers/nvidia,tegra210-emc.txt     | 448 +++++++++++++++++++++
 1 file changed, 448 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.txt

diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.txt b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.txt
new file mode 100644
index 0000000..1c52f47
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.txt

@@ -0,0 +1,448 @@
+NVIDIA Tegra210 SoC EMC (external memory controller)
+====================================================
+
+Required properties :
+- compatible : Should be "nvidia,tegra21-emc", "nvidia,tegra124-emc".
+- reg : physical base address and length of the controller's registers.
+- nvidia,memory-controller : phandle of the MC driver.

Huh? What is this block then?

This is the EMC. The EMC handles the interface with the external DRAM chips.
The MC aggregates and schedules requests to the EMC. It also handles address
translation. See figure 33 of the TRM.

Sorry, but I hardly have time to go read TRM's for every binding.

Just explain what the MC is here and remove 'driver'.

quoted

quoted

+- clocks : phandles of the possible source clocks
+- clock-names : names of the possible source clocks
+
+The node should contain a "emc-table" subnode for each supported RAM type
+(see field RAM_CODE in register PMC_STRAPPING_OPT_A), with its unit address
+being its RAM_CODE.

Unit address is based on reg property.

quoted

+
+Required properties for "emc-table" nodes :
+- nvidia,ram-code : Should contain the value of RAM_CODE this timing set is
+used for.
+
+Each "emc-table" node should contain a "emc-table" subnode for every supported
+EMC clock rate. The "emc-table" subnodes should have the clock rate in kHz as
+their unit address.
+
+Required properties for "emc-table" nodes :

Which emc-table nodes, the child or grand-child nodes?

The child "emc-table" node.

quoted

quoted

+- compatible  "nvidia,tegra21-emc-table", "nvidia,tegra210-emc-table"

quoted

+- nvidia,revision : revision of the parameter set used for this node. All
+                    nodes in the same "emc-table" should have the same revision
+- clock-frequency : frequency in kHz
+- nvidia,emc-min-mv : minimum voltage for this OPP
+- nvidia,gk20a-min-mv : minimum GPU voltage for this OPP
+- nvidia,source : clock source to be used for this OPP

Is this memory timings/settings or OPPs? We have a binding for OPPs
already.

This is memory timings.

quoted

quoted

+- nvidia,src-sel-reg : value of EMC CAR register to be used for this OPP
+- nvidia,needs-training : 1 if the OPP needs training at boot, 0 otherwise
+- nvidia,trained : 1 if initial training has been done by firmware, 0 otherwise
+- nvidia,periodic_training : 1 if the OPP needs periodic training, 0 otherwise
+- nvidia,trained_dram_clktree_c0d0u0 : training data word
+- nvidia,trained_dram_clktree_c0d0u1 : training data word

[...]

This is a huge list of properties. For all the things that are memory
timings, is there really value to defining a property for each setting?
Perhaps you should just define your own format and either make it a
separate firmware file or include that file in the dtb.

If it's a binary structure, how would this be passed given the bootloader has
to fill in the training information?

Perhaps training data makes sense to keep as DT properties, but it is
not all training data, right? Things like a list of register
initialization values do not sound like training data.

I assume the the training data is read back from the h/w, can't the
kernel just do that itself? Or the data is lost after init is
complete?

Rob