diff mbox series

[v5,4/8] dt-bindings: memory: tegra: Add external memory controller binding for Tegra210

Message ID 20200310152003.2945170-5-thierry.reding@gmail.com (mailing list archive)
State Superseded, archived
Headers show
Series Add EMC scaling support for Tegra210 | expand

Commit Message

Thierry Reding March 10, 2020, 3:19 p.m. UTC
From: Joseph Lo <josephl@nvidia.com>

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@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v5:
- convert to dt-schema

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

Comments

Dmitry Osipenko March 10, 2020, 4:35 p.m. UTC | #1
10.03.2020 18:19, Thierry Reding пишет:
> From: Joseph Lo <josephl@nvidia.com>
> 
> 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@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v5:
> - convert to dt-schema

...

> +  memory-region:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      phandle to a reserved memory region describing the table of EMC
> +      frequencies trained by the firmware

Shouldn't the table's format be documented?
Thierry Reding March 10, 2020, 5:12 p.m. UTC | #2
On Tue, Mar 10, 2020 at 07:35:01PM +0300, Dmitry Osipenko wrote:
> 10.03.2020 18:19, Thierry Reding пишет:
> > From: Joseph Lo <josephl@nvidia.com>
> > 
> > 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@nvidia.com>
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> > Changes in v5:
> > - convert to dt-schema
> 
> ...
> 
> > +  memory-region:
> > +    $ref: /schemas/types.yaml#/definitions/phandle
> > +    description:
> > +      phandle to a reserved memory region describing the table of EMC
> > +      frequencies trained by the firmware
> 
> Shouldn't the table's format be documented?

I'm not sure that's needed here. A proprietary bootloader creates this
table and the kernel has a structure for it. Describing the exact layout
in the device tree binding seems a bit excessive.

Thierry
Rob Herring (Arm) March 10, 2020, 6:38 p.m. UTC | #3
On Tue, 10 Mar 2020 16:19:59 +0100, Thierry Reding wrote:
> From: Joseph Lo <josephl@nvidia.com>
> 
> 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@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v5:
> - convert to dt-schema
> 
>  .../nvidia,tegra210-emc.yaml                  | 83 +++++++++++++++++++
>  1 file changed, 83 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.example.dts:23.13-20: Warning (ranges_format): /example-0/reserved-memory:ranges: empty "ranges" property but its #address-cells (2) differs from /example-0 (1)
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.example.dts:23.13-20: Warning (ranges_format): /example-0/reserved-memory:ranges: empty "ranges" property but its #size-cells (2) differs from /example-0 (1)

See https://patchwork.ozlabs.org/patch/1252240
Please check and re-submit.
Thierry Reding March 23, 2020, 10:35 a.m. UTC | #4
On Tue, Mar 10, 2020 at 04:19:59PM +0100, Thierry Reding wrote:
> From: Joseph Lo <josephl@nvidia.com>
> 
> 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@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v5:
> - convert to dt-schema
> 
>  .../nvidia,tegra210-emc.yaml                  | 83 +++++++++++++++++++
>  1 file changed, 83 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.yaml
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.yaml
> new file mode 100644
> index 000000000000..caf21c08f9cc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.yaml
> @@ -0,0 +1,83 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/memory-controllers/nvidia,tegra210-emc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: NVIDIA Tegra210 SoC External Memory Controller
> +
> +maintainers:
> +  - Thierry Reding <thierry.reding@gmail.com>
> +  - Jon Hunter <jonathanh@nvidia.com>
> +
> +description: |
> +  The EMC interfaces with the off-chip SDRAM to service the request stream
> +  sent from the memory controller.
> +
> +properties:
> +  compatible:
> +    const: nvidia,tegra210-emc
> +
> +  reg:
> +    maxItems: 3
> +
> +  clocks:
> +    items:
> +      - description: external memory clock
> +
> +  clock-names:
> +    items:
> +      - const: emc
> +
> +  interrupts:
> +    items:
> +      - description: EMC general interrupt
> +
> +  memory-region:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      phandle to a reserved memory region describing the table of EMC
> +      frequencies trained by the firmware

Hi Rob,

the dt_binding_check error aside, do you have any feedback on this
particular property? This is a replacement for what we used to do on
earlier chips where each frequency had its own device tree node, and
each such node had a bunch of properties, which made it not very
readable and cumbersome to parse.

The reason I ask about this specifically is because there are two
levels of bootloaders involved here to pass the information to the
kernel and I'd like to get those patches merged into the bootloaders
while I'm finishing up the Linux kernel support.

Dmitry asked whether the format of this table would need to be
documented in the bindings. I'm on the fence about this. On one hand
we don't have this documented anywhere, but on the other hand, the table
has things like revision fields and so on, so it could technically
change, even though it's very unlikely that it will.

If you do want it formatted, do you have any suggestions on what that
should look like? Should I simply dump the C struct definition into the
bindings document?

Thierry

> +
> +  nvidia,memory-controller:
> +    $ref: /schemas/types.yaml#/definitions/phandle
> +    description:
> +      phandle of the memory controller node
> +
> +required:
> +  - compatible
> +  - reg
> +  - clocks
> +  - clock-names
> +  - nvidia,memory-controller
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/clock/tegra210-car.h>
> +    #include <dt-bindings/interrupt-controller/arm-gic.h>
> +    #
> +    reserved-memory {
> +        #address-cells = <2>;
> +        #size-cells = <2>;
> +        ranges;
> +
> +        emc_table: emc-table@83400000 {
> +            compatible = "nvidia,tegra210-emc-table";
> +            reg = <0x0 0x83400000 0x0 0x10000>;
> +            status = "okay";
> +        };
> +    };
> +
> +    external-memory-controller@7001b000 {
> +        compatible = "nvidia,tegra210-emc";
> +        reg = <0x0 0x7001b000 0x0 0x1000>,
> +              <0x0 0x7001e000 0x0 0x1000>,
> +              <0x0 0x7001f000 0x0 0x1000>;
> +        clocks = <&tegra_car TEGRA210_CLK_EMC>;
> +        clock-names = "emc";
> +        interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
> +        memory-region = <&emc_table>;
> +        nvidia,memory-controller = <&mc>;
> +    };
> -- 
> 2.24.1
>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.yaml b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.yaml
new file mode 100644
index 000000000000..caf21c08f9cc
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/nvidia,tegra210-emc.yaml
@@ -0,0 +1,83 @@ 
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-controllers/nvidia,tegra210-emc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVIDIA Tegra210 SoC External Memory Controller
+
+maintainers:
+  - Thierry Reding <thierry.reding@gmail.com>
+  - Jon Hunter <jonathanh@nvidia.com>
+
+description: |
+  The EMC interfaces with the off-chip SDRAM to service the request stream
+  sent from the memory controller.
+
+properties:
+  compatible:
+    const: nvidia,tegra210-emc
+
+  reg:
+    maxItems: 3
+
+  clocks:
+    items:
+      - description: external memory clock
+
+  clock-names:
+    items:
+      - const: emc
+
+  interrupts:
+    items:
+      - description: EMC general interrupt
+
+  memory-region:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle to a reserved memory region describing the table of EMC
+      frequencies trained by the firmware
+
+  nvidia,memory-controller:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      phandle of the memory controller node
+
+required:
+  - compatible
+  - reg
+  - clocks
+  - clock-names
+  - nvidia,memory-controller
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/tegra210-car.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #
+    reserved-memory {
+        #address-cells = <2>;
+        #size-cells = <2>;
+        ranges;
+
+        emc_table: emc-table@83400000 {
+            compatible = "nvidia,tegra210-emc-table";
+            reg = <0x0 0x83400000 0x0 0x10000>;
+            status = "okay";
+        };
+    };
+
+    external-memory-controller@7001b000 {
+        compatible = "nvidia,tegra210-emc";
+        reg = <0x0 0x7001b000 0x0 0x1000>,
+              <0x0 0x7001e000 0x0 0x1000>,
+              <0x0 0x7001f000 0x0 0x1000>;
+        clocks = <&tegra_car TEGRA210_CLK_EMC>;
+        clock-names = "emc";
+        interrupts = <GIC_SPI 78 IRQ_TYPE_LEVEL_HIGH>;
+        memory-region = <&emc_table>;
+        nvidia,memory-controller = <&mc>;
+    };