Re: [PATCH V3 1/8] dt-bindings: memory: tegra: Add external memory controller binding for Tegra210

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/15/19 12:28 AM, Dmitry Osipenko wrote:
10.05.2019 11:47, Joseph Lo пишет:
Add the binding document for the external memory controller (EMC) which
communicates with external LPDDR4 devices. It includes the bindings of
the EMC node and a sub-node of EMC table which under the reserved memory
node. The EMC table contains the data of the rates that EMC supported.

Signed-off-by: Joseph Lo <josephl@xxxxxxxxxx>
---
v3:
- drop the bindings of EMC table
- add memory-region and reserved-memory node for EMC table
---
  .../nvidia,tegra210-emc.txt                   | 55 +++++++++++++++++++
  1 file changed, 55 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 000000000000..d65aeef2329c
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.txt
@@ -0,0 +1,55 @@
+NVIDIA Tegra210 SoC EMC (external memory controller)
+====================================================
+
+Device node
+===========
+Required properties :
+- compatible : should be "nvidia,tegra210-emc".
+- reg : physical base address and length of the controller's registers.
+- clocks : phandles of the possible source clocks.
+- clock-names : names of the possible source clocks.
+- interrupts : Should contain the EMC general interrupt.
+- memory-region : phandle to the reserved memory (see
+  Documentation/devicetree/bindings/reserved-memory/reserved-memory.txt) which
+  contains a sub-node of EMC table.
+- nvidia,memory-controller : phandle of the memory controller.
+
+Reserved memory node
+====================
+Should contain a sub-node of EMC table with required properties:
+- compatible : should be "nvidia,tegra210-emc-table".
+- reg : physical address and length of the location of EMC table.
+
+Example:
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+
+		emc_table: emc-table@8be00000 {
+			compatible = "nvidia,tegra210-emc-table";
+			reg = <0x0 0x8be00000 0x0 0x10000>;
+			status = "okay";
+		};

You essentially moved the v1 binding into obscure and undocumented blob,
ignoring previous review comments. This is a very odd move... please
explain what is going on.


Discussed with Thierry offline which way we prefer to pass the EMC table to the kernel. Some reasons below we decide to chose this one (via binary blob).

- The EMC table is much bigger than the previous Tegra generations (LPDDR4 v.s. LPDDR2/3). It's harder to settle in the review process. And if there is a new fix of the table in the future, we'll need to go through that again. - Because it's LPDDR4 we want to support here, to support higher rates, the devices have must be gone through the training process, which is done in the firmware. Which means We already have the table somewhere in the memory and kernel can just re-use that. No need to convert them back to DT and pass to the kernel. This is much easier to maintain in the future if there is something needs to fix. - With the mechanism above, we don't need to maintain the huge EMC table in the DT file like below.
http://patchwork.ozlabs.org/patch/1063886/
http://patchwork.ozlabs.org/patch/1063889/

And sorry, maybe it's not clear at that moment, but I did mention that we want to go with the new method (via binary blob) in the previous review.
Please see http://patchwork.ozlabs.org/patch/1084467/

Thanks,
Joseph




[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux