| Message ID | 20200310152003.2945170-5-thierry.reding@gmail.com |
|---|---|
| State | Deferred |
| Headers | show |
| Series | Add EMC scaling support for Tegra210 | expand |
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?
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
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.
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 --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>; + };