tegra20-emc: Identify memory chip by LPDDR configuration
v3 cover tegra20-emc: Identify memory chip by LPDDR configuration
Support memory chip identification by LPDDR2 configuration, which is needed by ASUS Transformer TF101 tablet device that doesn't store RAMCODE in Tegra's NVMEM.
Changelog:
v3: - Corrected sub-node name in tegra20-emc.yaml.
v2: - Added separate binding for standard LPDDR2 properties, like it
was suggested by Krzysztof Kozlowski.
- Switched Tegra binding to use new lpddr2-configuration sub-node that contains the standard properties.
- Extended commit message of the "emc: Document new LPDDR2 sub-node" patch, telling how the properties are supposed to be used, which was requested by Krzysztof Kozlowski.
- Added new common helpers for parsing LPDDR2 properties and made tegra20-emc driver to use these helpers.
Dmitry Osipenko (4):
dt-bindings: memory: Add LPDDR2 binding
dt-bindings: memory: tegra20: emc: Document new LPDDR2 sub-node
memory: Add LPDDR2 configuration helpers
memory: tegra20-emc: Support matching timings by LPDDR2 configuration
.../memory-controllers/jedec,lpddr2.yaml | 80 ++++++++ .../nvidia,tegra20-emc.yaml | 17 +- drivers/memory/jedec_ddr.h | 21 ++ drivers/memory/jedec_ddr_data.c | 42 ++++ drivers/memory/of_memory.c | 34 ++++ drivers/memory/of_memory.h | 9 + drivers/memory/tegra/Kconfig | 1 + drivers/memory/tegra/tegra20-emc.c | 191 ++++++++++++++++-- include/dt-bindings/memory/lpddr2.h | 25 +++ 9 files changed, 404 insertions(+), 16 deletions(-) create mode 100644 Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml create mode 100644 include/dt-bindings/memory/lpddr2.h
Signature · 1 line
2.32.0
PATCH v3 1/4 dt-bindings: memory: Add LPDDR2 binding
Add binding for standard LPDDR2 memory chip properties.
--- .../memory-controllers/jedec,lpddr2.yaml | 80 +++++++++++++++++++ include/dt-bindings/memory/lpddr2.h | 25 ++++++ 2 files changed, 105 insertions(+) create mode 100644 Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml create mode 100644 include/dt-bindings/memory/lpddr2.h
Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml new file +80 -0
@@ -0,0 +1,80 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/memory-controllers/jedec,lpddr2.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: JEDEC LPDDR2 SDRAM + +maintainers: + - Krzysztof Kozlowski <[email protected]> + +properties: + jedec,lpddr2-manufacturer-id: + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 255 + description: | + Unique manufacturer ID of SDRAM chip. See MR5 description in JESD209-2. + + jedec,lpddr2-revision-id1: + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 255 + description: | + Revision 1 value of SDRAM chip. + See MR6 description in chip vendor specification. + + jedec,lpddr2-revision-id2: + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 255 + description: | + Revision 2 value of SDRAM chip. + See MR7 description in chip vendor specification. + + jedec,lpddr2-density-mbits: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + Density in megabits of SDRAM chip. See MR8 description in JESD209-2. + enum: + - 64 + - 128 + - 256 + - 512 + - 1024 + - 2048 + - 4096 + - 8192 + - 16384 + - 32768 + + jedec,lpddr2-io-width-bits: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + IO bus width in bits of SDRAM chip. See MR8 description in JESD209-2. + enum: + - 32 + - 16 + - 8 + + jedec,lpddr2-type: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + LPDDR type which corresponds to a number of words SDRAM pre-fetches + per column request. See MR8 description in JESD209-2. + enum: + - 0 # S4 (4 words prefetch architecture) + - 1 # S2 (2 words prefetch architecture) + - 2 # NVM (Non-volatile memory) + +additionalProperties: false + +examples: + - | + #include <dt-bindings/memory/lpddr2.h> + + lpddr2 { + jedec,lpddr2-manufacturer-id = <LPDDR2_MANID_ELPIDA>; + jedec,lpddr2-revision-id1 = <1>; + jedec,lpddr2-density-mbits = <2048>; + jedec,lpddr2-io-width-bits = <16>; + jedec,lpddr2-type = <LPDDR2_TYPE_S4>; + };
include/dt-bindings/memory/lpddr2.h new file +25 -0
@@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */ +#ifndef _DT_BINDINGS_LPDDR2_H +#define _DT_BINDINGS_LPDDR2_H + +#define LPDDR2_MANID_SAMSUNG 1 +#define LPDDR2_MANID_QIMONDA 2 +#define LPDDR2_MANID_ELPIDA 3 +#define LPDDR2_MANID_ETRON 4 +#define LPDDR2_MANID_NANYA 5 +#define LPDDR2_MANID_HYNIX 6 +#define LPDDR2_MANID_MOSEL 7 +#define LPDDR2_MANID_WINBOND 8 +#define LPDDR2_MANID_ESMT 9 +#define LPDDR2_MANID_SPANSION 11 +#define LPDDR2_MANID_SST 12 +#define LPDDR2_MANID_ZMOS 13 +#define LPDDR2_MANID_INTEL 14 +#define LPDDR2_MANID_NUMONYX 254 +#define LPDDR2_MANID_MICRON 255 + +#define LPDDR2_TYPE_S4 0 +#define LPDDR2_TYPE_S2 1 +#define LPDDR2_TYPE_NVM 2 + +#endif /*_DT_BINDINGS_LPDDR2_H */
Signature · 1 line
2.32.0
Re: [PATCH v3 1/4] dt-bindings: memory: Add LPDDR2 binding
Quoted 6 lines · Dmitry Osipenko
Add binding for standard LPDDR2 memory chip properties.
Signed-off-by Dmitry Osipenko <[email protected]>--- .../memory-controllers/jedec,lpddr2.yaml | 80 +++++++++++++++++++ include/dt-bindings/memory/lpddr2.h | 25 ++++++
Hi Dmitry,
Thanks for doing this. I think I should be slightly more descriptive in my previous comment. What I meant, is to use existing DDR bindings (which might include or require converting them to YAML):Documentation/devicetree/bindings/ddr/
The bindings are already used:arch/arm/boot/dts/elpida_ecb240abacn.dtsiarch/arm/boot/dts/exynos5422-odroid-core.dtsidrivers/memory/samsung/exynos5422-dmc.c
You can remove the Documentation/devicetree/bindings/ddr/lpddr2.txt after full conversion, so also including AC timings and AC timing parameters. The timing parameters could be a separate YAML, if you want to convert everything. You can also skip it, because it is not necessary for your work.
Rob,
Any advice from your side where to put LPDDR2 dtschema bindings? The existing location was bindings/ddr/ but maybe this should be part of memory-controllers (although it is not actually a controller but rather used by the controller)?
Quoted 12 lines
2 files changed, 105 insertions(+) create mode 100644 Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml create mode 100644 include/dt-bindings/memory/lpddr2.hDocumentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml new file +2
-0@@ -0,0 +1,80 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2
Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml line 12
Quoted 10 lines
Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml new file +10
-0+--- +$id: http://devicetree.org/schemas/memory-controllers/jedec,lpddr2.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: JEDEC LPDDR2 SDRAM + +maintainers: + - Krzysztof Kozlowski <[email protected]> + +properties:
You need compatible (see lpddr2.txt)
Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml line 17
Quoted 5 lines
Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml new file +5
-0+ jedec,lpddr2-manufacturer-id: + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 255 + description: | + Unique manufacturer ID of SDRAM chip. See MR5 description in JESD209-2.
Plus:
"See include/dt-bindings/memory/lpddr2.h for known manufactured IDs."
However I wonder whether we need it. It should be taken from the vendor part of compatible.
Quoted 39 lines
Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml new file +39
-0+ + jedec,lpddr2-revision-id1: + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 255 + description: | + Revision 1 value of SDRAM chip. + See MR6 description in chip vendor specification. + + jedec,lpddr2-revision-id2: + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 255 + description: | + Revision 2 value of SDRAM chip. + See MR7 description in chip vendor specification. + + jedec,lpddr2-density-mbits: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + Density in megabits of SDRAM chip. See MR8 description in JESD209-2. + enum: + - 64 + - 128 + - 256 + - 512 + - 1024 + - 2048 + - 4096 + - 8192 + - 16384 + - 32768 + + jedec,lpddr2-io-width-bits: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + IO bus width in bits of SDRAM chip. See MR8 description in JESD209-2. + enum: + - 32 + - 16 + - 8
Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml line 66
Quoted 10 lines
Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml new file +10
-0+ + jedec,lpddr2-type: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + LPDDR type which corresponds to a number of words SDRAM pre-fetches + per column request. See MR8 description in JESD209-2. + enum: + - 0 # S4 (4 words prefetch architecture) + - 1 # S2 (2 words prefetch architecture) + - 2 # NVM (Non-volatile memory)
Type should not be needed but instead taken from compatible. Unless Rob has here preference and maybe change the DDR bindings?
requiredProperties for compatible, density, io-width.
Quoted 36 lines
Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml new file +14
-0+ +additionalProperties: false + +examples: + - | + #include <dt-bindings/memory/lpddr2.h> + + lpddr2 { + jedec,lpddr2-manufacturer-id = <LPDDR2_MANID_ELPIDA>; + jedec,lpddr2-revision-id1 = <1>; + jedec,lpddr2-density-mbits = <2048>; + jedec,lpddr2-io-width-bits = <16>; + jedec,lpddr2-type = <LPDDR2_TYPE_S4>; + };include/dt-bindings/memory/lpddr2.h new file +15
-0@@ -0,0 +1,25 @@ +/* SPDX-License-Identifier: GPL-2.0 OR BSD-2-Clause */ +#ifndef _DT_BINDINGS_LPDDR2_H +#define _DT_BINDINGS_LPDDR2_H + +#define LPDDR2_MANID_SAMSUNG 1 +#define LPDDR2_MANID_QIMONDA 2 +#define LPDDR2_MANID_ELPIDA 3 +#define LPDDR2_MANID_ETRON 4 +#define LPDDR2_MANID_NANYA 5 +#define LPDDR2_MANID_HYNIX 6 +#define LPDDR2_MANID_MOSEL 7 +#define LPDDR2_MANID_WINBOND 8 +#define LPDDR2_MANID_ESMT 9 +#define LPDDR2_MANID_SPANSION 11 +#define LPDDR2_MANID_SST 12
include/dt-bindings/memory/lpddr2.h line 25
Quoted 10 lines
include/dt-bindings/memory/lpddr2.h new file +10
-0+#define LPDDR2_MANID_ZMOS 13 +#define LPDDR2_MANID_INTEL 14 +#define LPDDR2_MANID_NUMONYX 254 +#define LPDDR2_MANID_MICRON 255 + +#define LPDDR2_TYPE_S4 0 +#define LPDDR2_TYPE_S2 1 +#define LPDDR2_TYPE_NVM 2 + +#endif /*_DT_BINDINGS_LPDDR2_H */
Best regards,
Krzysztof
Re: [PATCH v3 1/4] dt-bindings: memory: Add LPDDR2 binding
04.10.2021 10:42, Krzysztof Kozlowski пишет:
Quoted 15 lines · Dmitry Osipenko
Quoted 6 lines
Add binding for standard LPDDR2 memory chip properties.
Signed-off-by Dmitry Osipenko <[email protected]>--- .../memory-controllers/jedec,lpddr2.yaml | 80 +++++++++++++++++++ include/dt-bindings/memory/lpddr2.h | 25 ++++++Hi Dmitry,
Thanks for doing this. I think I should be slightly more descriptive in my previous comment. What I meant, is to use existing DDR bindings (which might include or require converting them to YAML):
Documentation/devicetree/bindings/ddr/
Quoted 4 lines
The bindings are already used:
arch/arm/boot/dts/elpida_ecb240abacn.dtsiarch/arm/boot/dts/exynos5422-odroid-core.dtsidrivers/memory/samsung/exynos5422-dmc.c
Thanks! I missed that there is ddr/ subdir.
Quoted 7 lines
You can remove the
Documentation/devicetree/bindings/ddr/lpddr2.txtafter full conversion, so also including AC timings and AC timing parameters. The timing parameters could be a separate YAML, if you want to convert everything. You can also skip it, because it is not necessary for your work.
Quoted 5 lines
Rob,
Any advice from your side where to put LPDDR2 dtschema bindings? The existing location was bindings/ddr/ but maybe this should be part of memory-controllers (although it is not actually a controller but rather used by the controller)?
+1 for having it inside of memory-controllers/
Quoted 31 lines
Quoted 12 lines
2 files changed, 105 insertions(+) create mode 100644 Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml create mode 100644 include/dt-bindings/memory/lpddr2.hDocumentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml new file +2
-0@@ -0,0 +1,80 @@ +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) +%YAML 1.2Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml line 12
Quoted 10 lines
Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml new file +10
-0+--- +$id: http://devicetree.org/schemas/memory-controllers/jedec,lpddr2.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: JEDEC LPDDR2 SDRAM + +maintainers: + - Krzysztof Kozlowski <[email protected]> + +properties:You need compatible (see lpddr2.txt)
Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml line 17
Quoted 5 lines
Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml new file +5
-0+ jedec,lpddr2-manufacturer-id: + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 255 + description: | + Unique manufacturer ID of SDRAM chip. See MR5 description in JESD209-2.
Quoted 5 lines
Plus:
"Seeinclude/dt-bindings/memory/lpddr2.hfor known manufactured IDs."However I wonder whether we need it. It should be taken from the vendor part of compatible.
It shouldn't be needed if compatible is used.
Quoted 53 lines
Quoted 39 lines
Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml new file +39
-0+ + jedec,lpddr2-revision-id1: + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 255 + description: | + Revision 1 value of SDRAM chip. + See MR6 description in chip vendor specification. + + jedec,lpddr2-revision-id2: + $ref: /schemas/types.yaml#/definitions/uint32 + maximum: 255 + description: | + Revision 2 value of SDRAM chip. + See MR7 description in chip vendor specification. + + jedec,lpddr2-density-mbits: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + Density in megabits of SDRAM chip. See MR8 description in JESD209-2. + enum: + - 64 + - 128 + - 256 + - 512 + - 1024 + - 2048 + - 4096 + - 8192 + - 16384 + - 32768 + + jedec,lpddr2-io-width-bits: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + IO bus width in bits of SDRAM chip. See MR8 description in JESD209-2. + enum: + - 32 + - 16 + - 8Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml line 66
Quoted 10 lines
Documentation/devicetree/bindings/memory-controllers/jedec,lpddr2.yaml new file +10
-0+ + jedec,lpddr2-type: + $ref: /schemas/types.yaml#/definitions/uint32 + description: | + LPDDR type which corresponds to a number of words SDRAM pre-fetches + per column request. See MR8 description in JESD209-2. + enum: + - 0 # S4 (4 words prefetch architecture) + - 1 # S2 (2 words prefetch architecture) + - 2 # NVM (Non-volatile memory)Type should not be needed but instead taken from compatible. Unless Rob has here preference and maybe change the DDR bindings?
Quoted 1 line
requiredProperties for compatible, density, io-width.
Alright
PATCH v3 2/4 dt-bindings: memory: tegra20: emc: Document new LPDDR2 sub-node
In reply to Dmitry OsipenkoSome Tegra20 boards don't have RAM code stored in NVMEM, which is used for the memory chip identification and the identity information should be read out from LPDDR2 chip in this case. Document new sub-node containing generic LPDDR2 properties that will be used for the memory chip identification if RAM code isn't available. The identification is done by reading out memory configuration values from generic LPDDR2 mode registers of SDRAM chip and comparing them with the values of device-tree sub-node's.
--- .../memory-controllers/nvidia,tegra20-emc.yaml | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml +14 -3
@@ -164,13 +164,14 @@ patternProperties: "#size-cells": const: 0 + lpddr2-configuration: + $ref: "jedec,lpddr2.yaml#" + type: object + patternProperties: "^emc-table@[0-9]+$": $ref: "#/$defs/emc-table" - required: - - nvidia,ram-code - additionalProperties: false required:
@@ -186,6 +187,8 @@ additionalProperties: false examples: - | + #include <dt-bindings/memory/lpddr2.h> + external-memory-controller@7000f400 { compatible = "nvidia,tegra20-emc"; reg = <0x7000f400 0x400>;
@@ -226,5 +229,13 @@ examples: 0x007fe010 0x00001414 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000>; }; + + lpddr2-configuration { + jedec,lpddr2-manufacturer-id = <LPDDR2_MANID_ELPIDA>; + jedec,lpddr2-revision-id1 = <1>; + jedec,lpddr2-density-mbits = <2048>; + jedec,lpddr2-io-width-bits = <16>; + jedec,lpddr2-type = <LPDDR2_TYPE_S4>; + }; }; };
Signature · 1 line
2.32.0
Re: [PATCH v3 2/4] dt-bindings: memory: tegra20: emc: Document new LPDDR2 sub-node
Quoted 18 lines · Dmitry Osipenko
Some Tegra20 boards don't have RAM code stored in NVMEM, which is used for the memory chip identification and the identity information should be read out from LPDDR2 chip in this case. Document new sub-node containing generic LPDDR2 properties that will be used for the memory chip identification if RAM code isn't available. The identification is done by reading out memory configuration values from generic LPDDR2 mode registers of SDRAM chip and comparing them with the values of device-tree sub-node's.
Signed-off-by Dmitry Osipenko <[email protected]>--- .../memory-controllers/nvidia,tegra20-emc.yaml | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml +0
-0
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml patternProperties: · line 167
Quoted 4 lines
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml +1
-0@@ -164,13 +164,14 @@ patternProperties: "#size-cells": const: 0 + lpddr2-configuration:
Nodes should be named generic, so just lpddr2?
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml patternProperties: · line 172
Quoted 9 lines
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml +3
-2+ $ref: "jedec,lpddr2.yaml#" + type: object + patternProperties: "^emc-table@[0-9]+$": $ref: "#/$defs/emc-table" - required: - - nvidia,ram-code
Isn't lpddr2-configuration required in such case? If not, probably you want either this or that (oneOf like in reserved-memory.yaml).
Quoted 18 lines
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml +2
-1- additionalProperties: false required:@@ -186,6 +187,8 @@ additionalProperties: false examples: - | + #include <dt-bindings/memory/lpddr2.h> + external-memory-controller@7000f400 { compatible = "nvidia,tegra20-emc"; reg = <0x7000f400 0x400>;@@ -226,5 +229,13 @@ examples: 0x007fe010 0x00001414 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000 0x00000000>; };
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml examples: · line 241
Quoted 10 lines
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml +8
-0+ + lpddr2-configuration { + jedec,lpddr2-manufacturer-id = <LPDDR2_MANID_ELPIDA>; + jedec,lpddr2-revision-id1 = <1>; + jedec,lpddr2-density-mbits = <2048>; + jedec,lpddr2-io-width-bits = <16>; + jedec,lpddr2-type = <LPDDR2_TYPE_S4>; + }; }; };
Best regards,
Krzysztof
Re: [PATCH v3 2/4] dt-bindings: memory: tegra20: emc: Document new LPDDR2 sub-node
04.10.2021 11:37, Krzysztof Kozlowski пишет:
Quoted 22 lines · Dmitry Osipenko
Quoted 21 lines
Some Tegra20 boards don't have RAM code stored in NVMEM, which is used for the memory chip identification and the identity information should be read out from LPDDR2 chip in this case. Document new sub-node containing generic LPDDR2 properties that will be used for the memory chip identification if RAM code isn't available. The identification is done by reading out memory configuration values from generic LPDDR2 mode registers of SDRAM chip and comparing them with the values of device-tree sub-node's.
Signed-off-by Dmitry Osipenko <[email protected]>--- .../memory-controllers/nvidia,tegra20-emc.yaml | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml +0
-0@@ -164,13 +164,14 @@ patternProperties: "#size-cells": const: 0
Quoted 3 lines
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml patternProperties: · line 167
Quoted 1 line
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml +1
-0+ lpddr2-configuration:Nodes should be named generic, so just lpddr2?
Yes
Quoted 7 lines
Quoted 7 lines
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml +3
-0+ $ref: "jedec,lpddr2.yaml#" + type: object + patternProperties: "^emc-table@[0-9]+$": $ref: "#/$defs/emc-table"
Quoted 5 lines
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml patternProperties: · line 172
Quoted 2 lines
Documentation/devicetree/bindings/memory-controllers/nvidia,tegra20-emc.yaml +0
-2- required: - - nvidia,ram-codeIsn't lpddr2-configuration required in such case? If not, probably you want either this or that (oneOf like in
reserved-memory.yaml).
Thanks, oneOf will work.
PATCH v3 3/4 memory: Add LPDDR2 configuration helpers
In reply to Dmitry OsipenkoAdd helpers for reading and parsing standard LPDDR2 properties.
--- drivers/memory/jedec_ddr.h | 21 +++++++++++++++++ drivers/memory/jedec_ddr_data.c | 42 +++++++++++++++++++++++++++++++++ drivers/memory/of_memory.c | 34 ++++++++++++++++++++++++++ drivers/memory/of_memory.h | 9 +++++++ 4 files changed, 106 insertions(+)
drivers/memory/jedec_ddr.h +21 -0
@@ -230,4 +230,25 @@ struct lpddr3_min_tck { u32 tMRD; }; +union lpddr2_basic_config4 { +u32 value; + +struct { +unsigned int arch_type :2 ; +unsigned int density :4 ; +unsigned int io_width :2 ; + } __packed; +}; + +struct lpddr2_configuration { +int arch_type; +int density; +int io_width; +int manufacturer_id; +int revision_id1; +int revision_id2; +}; + +const char *lpddr2_jedec_manufacturer (unsigned int manufacturer_id); +#endif /* __JEDEC_DDR_H */
drivers/memory/jedec_ddr_data.c +42 -0
51 lines not on this page · raw message
drivers/memory/of_memory.c +34 -0
@@ -298,3 +298,37 @@ const struct lpddr3_timingsreturn NULL ; }EXPORT_SYMBOL (of_lpddr3_get_ddr_timings); + +/** +* of_lpddr2_get_config() - extracts the lpddr2 chip configuration. +* @np: Pointer to device tree node containing configuration +* @conf: Configuration updated by this function +* +* Populates lpddr2_configuration structure by extracting data from device +* tree node. Returns 0 on success or error code on failure. If property +* is missing in device-tree, then the corresponding @conf value is set to +* -ENOENT. +*/ +int of_lpddr2_get_config (struct device_node *np, +struct lpddr2_configuration *conf) +{ +int err, ret = -ENOENT ; + +#define OF_LPDDR2_READ_U32 (prop, dtprop) \ + err = of_property_read_u32(np, dtprop, &conf->prop); \ + if (err) \ + conf->prop = -ENOENT; \ + else \ + ret = 0 + +/* at least one property should be parsed */ +OF_LPDDR2_READ_U32 (manufacturer_id,"jedec,lpddr2-manufacturer-id" ); +OF_LPDDR2_READ_U32 (revision_id1,"jedec,lpddr2-revision-id1" ); +OF_LPDDR2_READ_U32 (revision_id2,"jedec,lpddr2-revision-id2" ); +OF_LPDDR2_READ_U32 (io_width,"jedec,lpddr2-io-width-bits" ); +OF_LPDDR2_READ_U32 (density,"jedec,lpddr2-density-mbits" ); +OF_LPDDR2_READ_U32 (arch_type,"jedec,lpddr2-type" ); + +return ret; +} +EXPORT_SYMBOL (of_lpddr2_get_config);
drivers/memory/of_memory.h +9 -0
@@ -20,6 +20,9 @@ const struct lpddr3_min_tck *of_lpddr3_get_min_tck(struct device_node *np,const struct lpddr3_timings *of_lpddr3_get_ddr_timings (struct device_node *np_ddr,struct device *dev,u32 device_type,u32 *nr_frequencies); + +int of_lpddr2_get_config (struct device_node *np, +struct lpddr2_configuration *conf);#else static inline const struct lpddr2_min_tck *of_get_min_tck (struct device_node *np,struct device *dev)
@@ -46,6 +49,12 @@ static inline const struct lpddr3_timings {return NULL ; } + +static int of_lpddr2_get_config (struct device_node *np, +struct lpddr2_configuration *conf) +{ +return -ENOENT ; +}#endif /* CONFIG_OF && CONFIG_DDR */ #endif /* __LINUX_MEMORY_OF_REG_ */
Signature · 1 line
2.32.0
Re: [PATCH v3 3/4] memory: Add LPDDR2 configuration helpers
Quoted 109 lines · Dmitry Osipenko
Add helpers for reading and parsing standard LPDDR2 properties.
Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/memory/jedec_ddr.h | 21 +++++++++++++++++ drivers/memory/jedec_ddr_data.c | 42 +++++++++++++++++++++++++++++++++ drivers/memory/of_memory.c | 34 ++++++++++++++++++++++++++ drivers/memory/of_memory.h | 9 +++++++ 4 files changed, 106 insertions(+)drivers/memory/jedec_ddr.h +21
-0@@ -230,4 +230,25 @@ struct lpddr3_min_tck { u32 tMRD; }; +union lpddr2_basic_config4 { +u32 value; + +struct { +unsigned int arch_type :2 ; +unsigned int density :4 ; +unsigned int io_width :2 ; + } __packed; +}; + +struct lpddr2_configuration { +int arch_type; +int density; +int io_width; +int manufacturer_id; +int revision_id1; +int revision_id2; +}; + +const char *lpddr2_jedec_manufacturer (unsigned int manufacturer_id); +#endif /* __JEDEC_DDR_H */ drivers/memory/jedec_ddr_data.c +42
-0@@ -7,6 +7,7 @@* Aneesh V <[email protected]> */ +#include <dt-bindings/memory/lpddr2.h> #include <linux/export.h> #include "jedec_ddr.h" @@ -131,3 +132,44 @@ const struct lpddr2_min_tck lpddr2_jedec_min_tck = { .tFAW = 8 };EXPORT_SYMBOL_GPL (lpddr2_jedec_min_tck); + +const char *lpddr2_jedec_manufacturer (unsigned int manufacturer_id) +{ +switch (manufacturer_id) { +case LPDDR2_MANID_SAMSUNG : +return "Samsung" ; +case LPDDR2_MANID_QIMONDA : +return "Qimonda" ; +case LPDDR2_MANID_ELPIDA : +return "Elpida" ; +case LPDDR2_MANID_ETRON : +return "Etron" ; +case LPDDR2_MANID_NANYA : +return "Nanya" ; +case LPDDR2_MANID_HYNIX : +return "Hynix" ; +case LPDDR2_MANID_MOSEL : +return "Mosel" ; +case LPDDR2_MANID_WINBOND : +return "Winbond" ; +case LPDDR2_MANID_ESMT : +return "ESMT" ; +case LPDDR2_MANID_SPANSION : +return "Spansion" ; +case LPDDR2_MANID_SST : +return "SST" ; +case LPDDR2_MANID_ZMOS : +return "ZMOS" ; +case LPDDR2_MANID_INTEL : +return "Intel" ; +case LPDDR2_MANID_NUMONYX : +return "Numonyx" ; +case LPDDR2_MANID_MICRON : +return "Micron" ; +default : +break ; + } + +return "invalid" ; +} +EXPORT_SYMBOL_GPL (lpddr2_jedec_manufacturer);drivers/memory/of_memory.c +4
-0@@ -298,3 +298,37 @@ const struct lpddr3_timingsreturn NULL ; }EXPORT_SYMBOL (of_lpddr3_get_ddr_timings); + +/** +* of_lpddr2_get_config() - extracts the lpddr2 chip configuration. +* @np: Pointer to device tree node containing configuration
drivers/memory/of_memory.c const struct lpddr3_timings · line 314
Quoted 10 lines
drivers/memory/of_memory.c +10
-0+* @conf: Configuration updated by this function +* +* Populates lpddr2_configuration structure by extracting data from device +* tree node. Returns 0 on success or error code on failure. If property +* is missing in device-tree, then the corresponding @conf value is set to +* -ENOENT. +*/ +int of_lpddr2_get_config (struct device_node *np, +struct lpddr2_configuration *conf) +{
Interface should be rather like of_get_ddr_timings() - allocate memory for structure and return it. It's less error-prone.
Quoted 6 lines
drivers/memory/of_memory.c +6
-0+int err, ret = -ENOENT ; + +#define OF_LPDDR2_READ_U32 (prop, dtprop) \ + err = of_property_read_u32(np, dtprop, &conf->prop); \ + if (err) \ + conf->prop = -ENOENT; \
drivers/memory/of_memory.c const struct lpddr3_timings · line 330
Quoted 10 lines
drivers/memory/of_memory.c +10
-0+else \ + ret = 0 + +/* at least one property should be parsed */ +OF_LPDDR2_READ_U32 (manufacturer_id,"jedec,lpddr2-manufacturer-id" ); +OF_LPDDR2_READ_U32 (revision_id1,"jedec,lpddr2-revision-id1" ); +OF_LPDDR2_READ_U32 (revision_id2,"jedec,lpddr2-revision-id2" ); +OF_LPDDR2_READ_U32 (io_width,"jedec,lpddr2-io-width-bits" ); +OF_LPDDR2_READ_U32 (density,"jedec,lpddr2-density-mbits" ); +OF_LPDDR2_READ_U32 (arch_type,"jedec,lpddr2-type" );
density and io-width are required properties in existing bindings, so return code should not be overridden.
Quoted 22 lines
drivers/memory/of_memory.c +4
-0+ +return ret; +} +EXPORT_SYMBOL (of_lpddr2_get_config);drivers/memory/of_memory.h +3
-0@@ -20,6 +20,9 @@ const struct lpddr3_min_tck *of_lpddr3_get_min_tck(struct device_node *np,const struct lpddr3_timings *of_lpddr3_get_ddr_timings (struct device_node *np_ddr,struct device *dev,u32 device_type,u32 *nr_frequencies); + +int of_lpddr2_get_config (struct device_node *np, +struct lpddr2_configuration *conf);#else static inline const struct lpddr2_min_tck *of_get_min_tck (struct device_node *np,struct device *dev)@@ -46,6 +49,12 @@ static inline const struct lpddr3_timings {return NULL ;
drivers/memory/of_memory.h static inline const struct lpddr3_timings · line 60
Quoted 10 lines
drivers/memory/of_memory.h +6
-0} + +static int of_lpddr2_get_config (struct device_node *np, +struct lpddr2_configuration *conf) +{ +return -ENOENT ; +}#endif /* CONFIG_OF && CONFIG_DDR */ #endif /* __LINUX_MEMORY_OF_REG_ */
Best regards,
Krzysztof
PATCH v3 4/4 memory: tegra20-emc: Support matching timings by LPDDR2 configuration
In reply to Dmitry OsipenkoAsus Transformer TF101 doesn't provide RAM code and in this case memory timings should be selected based on identity information read out from SDRAM chip. Support matching timings by LPDDR2 configuration.
--- drivers/memory/tegra/Kconfig | 1 + drivers/memory/tegra/tegra20-emc.c | 191 +++++++++++++++++++++++++++-- 2 files changed, 179 insertions(+), 13 deletions(-)
drivers/memory/tegra/Kconfig +1 -0
@@ -16,6 +16,7 @@ config TEGRA20_EMC depends on ARCH_TEGRA_2x_SOC || COMPILE_TEST select DEVFREQ_GOV_SIMPLE_ONDEMAND select PM_DEVFREQ + select DDR help This driver is for the External Memory Controller (EMC) found on Tegra20 chips. The EMC controls the external DRAM on the board.
drivers/memory/tegra/tegra20-emc.c +178 -13
283 lines not on this page · raw message
Signature · 1 line
2.32.0
Re: [PATCH v3 4/4] memory: tegra20-emc: Support matching timings by LPDDR2 configuration
Quoted 183 lines · Dmitry Osipenko
Asus Transformer TF101 doesn't provide RAM code and in this case memory timings should be selected based on identity information read out from SDRAM chip. Support matching timings by LPDDR2 configuration.
Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/memory/tegra/Kconfig | 1 + drivers/memory/tegra/tegra20-emc.c | 191 +++++++++++++++++++++++++++-- 2 files changed, 179 insertions(+), 13 deletions(-)drivers/memory/tegra/Kconfig +1
-0@@ -16,6 +16,7 @@ config TEGRA20_EMC depends on ARCH_TEGRA_2x_SOC || COMPILE_TEST select DEVFREQ_GOV_SIMPLE_ONDEMAND select PM_DEVFREQ + select DDR help This driver is for the External Memory Controller (EMC) found on Tegra20 chips. The EMC controls the external DRAM on the board.drivers/memory/tegra/tegra20-emc.c +79
-3@@ -5,6 +5,7 @@* Author: Dmitry Osipenko <[email protected]> */ +#include <linux/bitfield.h> #include <linux/clk.h> #include <linux/clk/tegra.h> #include <linux/debugfs.h> @@ -27,11 +28,15 @@#include <soc/tegra/common.h> #include <soc/tegra/fuse.h> +#include "../jedec_ddr.h" +#include "../of_memory.h" +#include "mc.h" #define EMC_INTSTATUS 0x000#define EMC_INTMASK 0x004#define EMC_DBG 0x008 +#define EMC_ADR_CFG_0 0x010#define EMC_TIMING_CONTROL 0x028#define EMC_RC 0x02c#define EMC_RFC 0x030@@ -68,6 +73,7 @@#define EMC_QUSE_EXTRA 0x0ac#define EMC_ODT_WRITE 0x0b0#define EMC_ODT_READ 0x0b4 +#define EMC_MRR 0x0ec#define EMC_FBIO_CFG5 0x104#define EMC_FBIO_CFG6 0x114#define EMC_STAT_CONTROL 0x160@@ -94,6 +100,7 @@#define EMC_REFRESH_OVERFLOW_INT BIT(3)#define EMC_CLKCHANGE_COMPLETE_INT BIT(4) +#define EMC_MRR_DIVLD_INT BIT(5)#define EMC_DBG_READ_MUX_ASSEMBLY BIT(0)#define EMC_DBG_WRITE_MUX_ACTIVE BIT(1)@@ -102,11 +109,25 @@#define EMC_DBG_CFG_PRIORITY BIT(24)#define EMC_FBIO_CFG5_DRAM_WIDTH_X16 BIT(4) +#define EMC_FBIO_CFG5_DRAM_TYPE GENMASK(1, 0) + +#define EMC_MRR_DEV_SELECTN GENMASK(31, 30) +#define EMC_MRR_MRR_MA GENMASK(23, 16) +#define EMC_MRR_MRR_DATA GENMASK(15, 0) + +#define EMC_ADR_CFG_0_EMEM_NUMDEV GENMASK(25, 24)#define EMC_PWR_GATHER_CLEAR (1 << 8)#define EMC_PWR_GATHER_DISABLE (2 << 8)#define EMC_PWR_GATHER_ENABLE (3 << 8) +enum emc_dram_type { +DRAM_TYPE_RESERVED , +DRAM_TYPE_DDR1 , +DRAM_TYPE_LPDDR2 , +DRAM_TYPE_DDR2 , +}; +static const u16 emc_timing_registers[] = {EMC_RC ,EMC_RFC ,@@ -201,6 +222,12 @@ struct tegra_emc {struct mutex rate_lock;struct devfreq_simple_ondemand_data ondemand_data; + +/* memory chip identity information */ +union lpddr2_basic_config4 basic_conf4; +unsigned int manufacturer_id; +unsigned int revision_id1; +unsigned int revision_id2; };static irqreturn_t tegra_emc_isr (int irq,void *data)@@ -401,6 +428,9 @@ static int tegra_emc_load_timings_from_dt(struct tegra_emc *emc, timing = emc->timings;for_each_child_of_node (node, child) { +if (of_node_name_eq (child,"lpddr2-configuration" )) +continue ; + err =load_one_timing_from_dt (emc, timing++, child);if (err) {of_node_put (child);@@ -422,8 +452,9 @@ static int tegra_emc_load_timings_from_dt(struct tegra_emc *emc, }static struct device_node * -tegra_emc_find_node_by_ram_code (struct device *dev) +tegra_emc_find_node_by_ram_code (struct tegra_emc *emc) { +struct device *dev = emc->dev;struct device_node *np;u32 value, ram_code;int err;@@ -442,8 +473,53 @@ tegra_emc_find_node_by_ram_code(struct device *dev) np = of_find_node_by_name(np, "emc-tables")) { err =of_property_read_u32 (np,"nvidia,ram-code" , &value);if (err || value != ram_code) { -of_node_put (np); -continue ; +struct device_node *lpddr2_np; +bool conf_mismatches = false; + + lpddr2_np =of_find_node_by_name (np,"lpddr2-configuration" ); +if (lpddr2_np) { +struct lpddr2_configuration cfg; + + err =of_lpddr2_get_config (lpddr2_np, &cfg); +if (!err) { +if (cfg.manufacturer_id >=0 && + cfg.manufacturer_id != emc->manufacturer_id) + conf_mismatches = true; + +if (cfg.revision_id1 >=0 && + cfg.revision_id1 != emc->revision_id1) + conf_mismatches = true; + +if (cfg.revision_id2 >=0 && + cfg.revision_id2 != emc->revision_id2) + conf_mismatches = true; + +if (cfg.density >=0 && + cfg.density !=64 << emc->basic_conf4.density) + conf_mismatches = true; + +if (cfg.io_width >=0 && + cfg.io_width !=32 >> emc->basic_conf4.io_width) + conf_mismatches = true; + +if (cfg.arch_type >=0 && + cfg.arch_type != emc->basic_conf4.arch_type) + conf_mismatches = true; + }else { +dev_err (emc->dev,"failed to parse %pOF: %d\n" , + lpddr2_np, err); + conf_mismatches = true; + } + +of_node_put (lpddr2_np); + }else { + conf_mismatches = true; + } + +if (conf_mismatches) { +of_node_put (np); +continue ; + } }return np;
drivers/memory/tegra/tegra20-emc.c tegra_emc_find_node_by_ram_code(struct device *dev) · line 539
Quoted 9 lines
drivers/memory/tegra/tegra20-emc.c +6
-0@@ -455,10 +531,70 @@ tegra_emc_find_node_by_ram_code(struct device *dev)return NULL ; } +static int emc_read_lpddr_mode_register (struct tegra_emc *emc, +unsigned int emem_dev, +unsigned int register_addr, +unsigned int *register_data) +{ +u32 val, memory_dev = emem_dev +1 , mr_mask =0xff ;
One initialization per line, in reversed christmas tree (which matches rest of the file, I thnk).
Quoted 27 lines
drivers/memory/tegra/tegra20-emc.c +27
-0+int err; + +/* clear data-valid interrupt status */ +writel_relaxed (EMC_MRR_DIVLD_INT , emc->regs +EMC_INTSTATUS ); + +/* issue mode register read request */ + val =FIELD_PREP (EMC_MRR_DEV_SELECTN , memory_dev); + val |=FIELD_PREP (EMC_MRR_MRR_MA , register_addr); + +writel_relaxed (val, emc->regs +EMC_MRR ); + +/* wait for the LPDDR2 data-valid interrupt */ + err =readl_relaxed_poll_timeout_atomic (emc->regs +EMC_INTSTATUS , val, + val &EMC_MRR_DIVLD_INT , +1 ,100 ); +if (err) { +dev_err (emc->dev,"mode-register %u read failed: %d\n" , + register_addr, err); +return err; + } + +/* read out register data */ + val =readl_relaxed (emc->regs +EMC_MRR ); + *register_data =FIELD_GET (EMC_MRR_MRR_DATA , val) & mr_mask; + +return 0 ; +}
drivers/memory/tegra/tegra20-emc.c tegra_emc_find_node_by_ram_code(struct device *dev) · line 576
Quoted 10 lines
drivers/memory/tegra/tegra20-emc.c +10
-0+ +static void emc_read_lpddr_sdram_info (struct tegra_emc *emc, +unsigned int emem_dev, +bool print_out) +{ +/* these registers are standard for all LPDDR JEDEC memory chips */ +emc_read_lpddr_mode_register (emc, emem_dev,5 , &emc->manufacturer_id); +emc_read_lpddr_mode_register (emc, emem_dev,6 , &emc->revision_id1); +emc_read_lpddr_mode_register (emc, emem_dev,7 , &emc->revision_id2); +emc_read_lpddr_mode_register (emc, emem_dev,8 , &emc->basic_conf4.value);
You ignore the return codes but you should not try to load timings in such case. The DT could (by mistake or on purpose) have values '0' for the fields you compare.
Quoted 9 lines
drivers/memory/tegra/tegra20-emc.c +9
-0+ +if (!print_out) +return ; + + dev_info(emc->dev, "SDRAM[dev%u]: manufacturer: 0x%x (%s) rev1: 0x%x rev2: 0x%x prefetch: S%u density: %uMbit iowidth: %ubit\n", + emem_dev, emc->manufacturer_id, + lpddr2_jedec_manufacturer(emc->manufacturer_id), + emc->revision_id1, emc->revision_id2, + 4 >> emc->basic_conf4.arch_type,
drivers/memory/tegra/tegra20-emc.c tegra_emc_find_node_by_ram_code(struct device *dev) · line 594
Quoted 10 lines
drivers/memory/tegra/tegra20-emc.c +6
-1+64 << emc->basic_conf4.density, + 32 >> emc->basic_conf4.io_width); +} +static int emc_setup_hw (struct tegra_emc *emc) { +u32 emc_cfg, emc_dbg, emc_fbio, emc_adr_cfg;u32 intmask =EMC_REFRESH_OVERFLOW_INT ; -u32 emc_cfg, emc_dbg, emc_fbio; +static bool print_sdram_info_once;
How about moving print_sdram_info_once to emc_read_lpddr_sdram_info()? Less code here.
drivers/memory/tegra/tegra20-emc.c static int emc_setup_hw(struct tegra_emc *emc) · line 634
Quoted 10 lines
drivers/memory/tegra/tegra20-emc.c +3
-0+enum emc_dram_type dram_type; +const char *dram_type_str; +unsigned int emem_numdev; emc_cfg =readl_relaxed (emc->regs +EMC_CFG_2 );@@ -496,7 +632,36 @@ static int emc_setup_hw(struct tegra_emc *emc)else emc->dram_bus_width =32 ;
Best regards,
Krzysztof
Re: [PATCH v3 4/4] memory: tegra20-emc: Support matching timings by LPDDR2 configuration
04.10.2021 12:09, Krzysztof Kozlowski пишет:
Quoted 9 lines
drivers/memory/tegra/tegra20-emc.c tegra_emc_find_node_by_ram_code(struct device *dev) · line 594
Quoted 6 lines
static int
emc_setup_hw(struct tegra_emc *emc)
{drivers/memory/tegra/tegra20-emc.c +2
-1+u32 emc_cfg, emc_dbg, emc_fbio, emc_adr_cfg;u32 intmask =EMC_REFRESH_OVERFLOW_INT ; -u32 emc_cfg, emc_dbg, emc_fbio; +static bool print_sdram_info_once;How about moving
print_sdram_info_oncetoemc_read_lpddr_sdram_info()? Less code here.
The SDRAM info is printed out for each attached SDRAM chip. There are two prints if two chips are attached to memory controller. Hence this print_once flag should cover both prints.
Re: [PATCH v3 4/4] memory: tegra20-emc: Support matching timings by LPDDR2 configuration
In reply to Krzysztof Kozlowski04.10.2021 12:09, Krzysztof Kozlowski пишет:
Quoted 4 lines
Quoted 4 lines
drivers/memory/tegra/tegra20-emc.c +4
-0+static void emc_read_lpddr_sdram_info (struct tegra_emc *emc, +unsigned int emem_dev, +bool print_out) +{
Quoted 9 lines
drivers/memory/tegra/tegra20-emc.c tegra_emc_find_node_by_ram_code(struct device *dev) · line 576
Quoted 5 lines
drivers/memory/tegra/tegra20-emc.c +5
-0+/* these registers are standard for all LPDDR JEDEC memory chips */ +emc_read_lpddr_mode_register (emc, emem_dev,5 , &emc->manufacturer_id); +emc_read_lpddr_mode_register (emc, emem_dev,6 , &emc->revision_id1); +emc_read_lpddr_mode_register (emc, emem_dev,7 , &emc->revision_id2); +emc_read_lpddr_mode_register (emc, emem_dev,8 , &emc->basic_conf4.value);You ignore the return codes but you should not try to load timings in such case. The DT could (by mistake or on purpose) have values '0' for the fields you compare.
Good suggestion. I'll add a flag that will prevent loading timings if there was MRR error, thanks.