[PATCH v4 05/20] dt-bindings: memory: factorise LPDDR props into SDRAM props

Clément Le Goffic posted 20 patches 2 months, 2 weeks ago
There is a newer version of this series
[PATCH v4 05/20] dt-bindings: memory: factorise LPDDR props into SDRAM props
Posted by Clément Le Goffic 2 months, 2 weeks ago
LPDDR and DDR bindings are SDRAM types and are likely to share the same
properties (at least for density, io-width and reg).
To avoid bindings duplication, factorise the properties.

The compatible description has been updated because the MR (Mode
registers) used to get manufacturer ID and revision ID are not present
in case of DDR.
Those information should be in a SPD (Serial Presence Detect) EEPROM in
case of DIMM module or are known in case of soldered memory chips as
they are in the datasheet of the memory chips.

Signed-off-by: Clément Le Goffic <clement.legoffic@foss.st.com>
---
 .../memory-controllers/ddr/jedec,lpddr-props.yaml  | 74 -----------------
 .../memory-controllers/ddr/jedec,lpddr2.yaml       |  2 +-
 .../memory-controllers/ddr/jedec,lpddr3.yaml       |  2 +-
 .../memory-controllers/ddr/jedec,lpddr4.yaml       |  2 +-
 .../memory-controllers/ddr/jedec,lpddr5.yaml       |  2 +-
 .../memory-controllers/ddr/jedec,sdram-props.yaml  | 92 ++++++++++++++++++++++
 6 files changed, 96 insertions(+), 78 deletions(-)

diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml
deleted file mode 100644
index 30267ce70124..000000000000
--- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml
+++ /dev/null
@@ -1,74 +0,0 @@
-# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
-%YAML 1.2
----
-$id: http://devicetree.org/schemas/memory-controllers/ddr/jedec,lpddr-props.yaml#
-$schema: http://devicetree.org/meta-schemas/core.yaml#
-
-title: Common properties for LPDDR types
-
-description:
-  Different LPDDR types generally use the same properties and only differ in the
-  range of legal values for each. This file defines the common parts that can be
-  reused for each type. Nodes using this schema should generally be nested under
-  an LPDDR channel node.
-
-maintainers:
-  - Krzysztof Kozlowski <krzk@kernel.org>
-
-properties:
-  compatible:
-    description:
-      Compatible strings can be either explicit vendor names and part numbers
-      (e.g. elpida,ECB240ABACN), or generated strings of the form
-      lpddrX-YY,ZZZZ where X is the LPDDR version, YY is the manufacturer ID
-      (from MR5) and ZZZZ is the revision ID (from MR6 and MR7). Both IDs are
-      formatted in lower case hexadecimal representation with leading zeroes.
-      The latter form can be useful when LPDDR nodes are created at runtime by
-      boot firmware that doesn't have access to static part number information.
-
-  reg:
-    description:
-      The rank number of this LPDDR rank when used as a subnode to an LPDDR
-      channel.
-    minimum: 0
-    maximum: 3
-
-  revision-id:
-    $ref: /schemas/types.yaml#/definitions/uint32-array
-    description:
-      Revision IDs read from Mode Register 6 and 7. One byte per uint32 cell (i.e. <MR6 MR7>).
-    maxItems: 2
-    items:
-      minimum: 0
-      maximum: 255
-
-  density:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description:
-      Density in megabits of SDRAM chip. Decoded from Mode Register 8.
-    enum:
-      - 64
-      - 128
-      - 256
-      - 512
-      - 1024
-      - 2048
-      - 3072
-      - 4096
-      - 6144
-      - 8192
-      - 12288
-      - 16384
-      - 24576
-      - 32768
-
-  io-width:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description:
-      IO bus width in bits of SDRAM chip. Decoded from Mode Register 8.
-    enum:
-      - 8
-      - 16
-      - 32
-
-additionalProperties: true
diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml
index a237bc259273..704bbc562528 100644
--- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml
@@ -10,7 +10,7 @@ maintainers:
   - Krzysztof Kozlowski <krzk@kernel.org>
 
 allOf:
-  - $ref: jedec,lpddr-props.yaml#
+  - $ref: jedec,sdram-props.yaml#
 
 properties:
   compatible:
diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml
index e328a1195ba6..0d28df3d2bfa 100644
--- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml
@@ -10,7 +10,7 @@ maintainers:
   - Krzysztof Kozlowski <krzk@kernel.org>
 
 allOf:
-  - $ref: jedec,lpddr-props.yaml#
+  - $ref: jedec,sdram-props.yaml#
 
 properties:
   compatible:
diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr4.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr4.yaml
index a078892fecee..65aa07861453 100644
--- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr4.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr4.yaml
@@ -10,7 +10,7 @@ maintainers:
   - Krzysztof Kozlowski <krzk@kernel.org>
 
 allOf:
-  - $ref: jedec,lpddr-props.yaml#
+  - $ref: jedec,sdram-props.yaml#
 
 properties:
   compatible:
diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr5.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr5.yaml
index e441dac5f154..cf5d5a8e94b3 100644
--- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr5.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr5.yaml
@@ -10,7 +10,7 @@ maintainers:
   - Krzysztof Kozlowski <krzk@kernel.org>
 
 allOf:
-  - $ref: jedec,lpddr-props.yaml#
+  - $ref: jedec,sdram-props.yaml#
 
 properties:
   compatible:
diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,sdram-props.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,sdram-props.yaml
new file mode 100644
index 000000000000..0838ee092255
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,sdram-props.yaml
@@ -0,0 +1,92 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/memory-controllers/ddr/jedec,sdram-props.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Common properties for SDRAM types
+
+description:
+  Different SDRAM types generally use the same properties and only differ in the
+  range of legal values for each. This file defines the common parts that can be
+  reused for each type. Nodes using this schema should generally be nested under
+  a SDRAM channel node.
+
+maintainers:
+  - Krzysztof Kozlowski <krzk@kernel.org>
+
+properties:
+  compatible:
+    description: |
+      Compatible strings can be either explicit vendor names and part numbers
+      (e.g. elpida,ECB240ABACN), or generated strings of the form
+      (lp)?ddrX-Y,Z where X, Y and Z are in lower case hexadecimal with leading
+      zeroes and :
+        - X is the SDRAM version (2, 3, 4, etc.)
+        - for LPDDR :
+          - Y is the manufacturer ID (from MR5), 2 bytes
+          - Z is the revision ID (from MR6 and MR7), 4 bytes
+        - for DDR4 with SPD, according to JEDEC SPD4.1.2.L-6 :
+          - Y is the manufacturer ID, 2 bytes, from bytes 320 and 321
+          - Z is the revision ID, 1 byte, from byte 349
+      The latter form can be useful when SDRAM nodes are created at runtime by
+      boot firmware that doesn't have access to static part number information.
+      The former form is useful when the SDRAM vendor and part number are
+      known, such as when the SDRAM is soldered on the board.
+
+  reg:
+    description:
+      The rank number of this memory rank when used as a subnode to an memory
+      channel.
+    minimum: 0
+    maximum: 3
+
+  revision-id:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description: |
+      SDRAM revision ID:
+        - LPDDR SDRAM, decoded from Mode Register 6 and 7.
+        - DDR4 SDRAM, decoded from the SPD from bytes 349 according to
+          JEDEC SPD4.1.2.L-6.
+      One byte per uint32 cell (i.e. <MR6 MR7>).
+    maxItems: 2
+    items:
+      minimum: 0
+      maximum: 255
+
+  density:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Density of SDRAM chip in megabits:
+        - LPDDR SDRAM, decoded from Mode Register 8.
+        - DDR4 SDRAM, decoded from the SPD from bytes 322 to 325 according to
+          JEDEC SPD4.1.2.L-6.
+    enum:
+      - 64
+      - 128
+      - 256
+      - 512
+      - 1024
+      - 2048
+      - 3072
+      - 4096
+      - 6144
+      - 8192
+      - 12288
+      - 16384
+      - 24576
+      - 32768
+
+  io-width:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      IO bus width in bits of SDRAM chip:
+        - LPDDR SDRAM, decoded from Mode Register 8.
+        - DDR4 SDRAM with, decoded from the SPD from bits 2~0 of byte 12
+          according to JEDEC SPD4.1.2.L-6.
+    enum:
+      - 8
+      - 16
+      - 32
+
+additionalProperties: true

-- 
2.43.0

Re: [PATCH v4 05/20] dt-bindings: memory: factorise LPDDR props into SDRAM props
Posted by Julius Werner 2 months, 2 weeks ago
> +      Compatible strings can be either explicit vendor names and part numbers
> +      (e.g. elpida,ECB240ABACN), or generated strings of the form
> +      (lp)?ddrX-Y,Z where X, Y and Z are in lower case hexadecimal with leading
> +      zeroes and :
> +        - X is the SDRAM version (2, 3, 4, etc.)
> +        - for LPDDR :
> +          - Y is the manufacturer ID (from MR5), 2 bytes
> +          - Z is the revision ID (from MR6 and MR7), 4 bytes

It's actually one byte manufacturer, two bytes revision. The YY,ZZZZ
is supposed to represent the amount of hex digits needed.

> +        - for DDR4 with SPD, according to JEDEC SPD4.1.2.L-6 :
> +          - Y is the manufacturer ID, 2 bytes, from bytes 320 and 321
> +          - Z is the revision ID, 1 byte, from byte 349

I don't think this will identify a part unambiguously, I would expect
the DDR revision ID to be specific to the part number. (In fact, we're
also not sure whether manufacturer+revision identifies LPDDR parts
unambiguously for every vendor, we just didn't have anything more to
work with there.) I would suggest to use either `ddrX-YYYY,AAA...,ZZ`
or `ddrX-YYYY,ZZ,AAA...` (where AAA... is the part number string from
SPD 329-348 without the trailing spaces). The first version looks a
bit more natural but it might get confusing on the off chance that
someone uses a comma in a part number string.

> +      The latter form can be useful when SDRAM nodes are created at runtime by
> +      boot firmware that doesn't have access to static part number information.

nit: This text slightly doesn't make sense anymore when in the DDR
case we do actually have the part number. I guess the real thing the
bootloader wouldn't have access to is the JEDEC manufacturer ID to
name mapping.

> +      SDRAM revision ID:
> +        - LPDDR SDRAM, decoded from Mode Register 6 and 7.
> +        - DDR4 SDRAM, decoded from the SPD from bytes 349 according to
> +          JEDEC SPD4.1.2.L-6.

nit: Clarify that this is always two bytes for LPDDR and always one
byte for DDR.

> +      Density of SDRAM chip in megabits:
> +        - LPDDR SDRAM, decoded from Mode Register 8.
> +        - DDR4 SDRAM, decoded from the SPD from bytes 322 to 325 according to
> +          JEDEC SPD4.1.2.L-6.

Are these numbers correct? I downloaded SPD4.1.2.L-6 now and it looks
like 322 is manufacturing location and 323-324 are manufacturing date.
(Also, I think all of these are specific to DDR4 (and possibly 5?),
but not to earlier versions. I don't think we need to list it for
every version, but we should at least be specific what it applies to.)
Re: [PATCH v4 05/20] dt-bindings: memory: factorise LPDDR props into SDRAM props
Posted by Clement LE GOFFIC 2 months, 2 weeks ago
Hi Julius,

On 7/23/25 23:48, Julius Werner wrote:
>> +      Compatible strings can be either explicit vendor names and part numbers
>> +      (e.g. elpida,ECB240ABACN), or generated strings of the form
>> +      (lp)?ddrX-Y,Z where X, Y and Z are in lower case hexadecimal with leading
>> +      zeroes and :
>> +        - X is the SDRAM version (2, 3, 4, etc.)
>> +        - for LPDDR :
>> +          - Y is the manufacturer ID (from MR5), 2 bytes
>> +          - Z is the revision ID (from MR6 and MR7), 4 bytes
> 
> It's actually one byte manufacturer, two bytes revision. The YY,ZZZZ
> is supposed to represent the amount of hex digits needed.

Oh yes I see my error..

> 
>> +        - for DDR4 with SPD, according to JEDEC SPD4.1.2.L-6 :
>> +          - Y is the manufacturer ID, 2 bytes, from bytes 320 and 321
>> +          - Z is the revision ID, 1 byte, from byte 349
> 
> I don't think this will identify a part unambiguously, I would expect
> the DDR revision ID to be specific to the part number. (In fact, we're
> also not sure whether manufacturer+revision identifies LPDDR parts
> unambiguously for every vendor, we just didn't have anything more to
> work with there.) I would suggest to use either `ddrX-YYYY,AAA...,ZZ`
> or `ddrX-YYYY,ZZ,AAA...` (where AAA... is the part number string from
> SPD 329-348 without the trailing spaces). The first version looks a
> bit more natural but it might get confusing on the off chance that
> someone uses a comma in a part number string.

The first one seems better indeed.
If the manufacturer put a comma in the part number we should handle it 
at a software level to me and if it is a devicetree error it is up to 
the devicetree writer to fix it.
What do you think ?

> 
>> +      The latter form can be useful when SDRAM nodes are created at runtime by
>> +      boot firmware that doesn't have access to static part number information.
> 
> nit: This text slightly doesn't make sense anymore when in the DDR
> case we do actually have the part number. I guess the real thing the
> bootloader wouldn't have access to is the JEDEC manufacturer ID to
> name mapping.

Yes I will update it.

> 
>> +      SDRAM revision ID:
>> +        - LPDDR SDRAM, decoded from Mode Register 6 and 7.
>> +        - DDR4 SDRAM, decoded from the SPD from bytes 349 according to
>> +          JEDEC SPD4.1.2.L-6.
> 
> nit: Clarify that this is always two bytes for LPDDR and always one
> byte for DDR.

Ok

> 
>> +      Density of SDRAM chip in megabits:
>> +        - LPDDR SDRAM, decoded from Mode Register 8.
>> +        - DDR4 SDRAM, decoded from the SPD from bytes 322 to 325 according to
>> +          JEDEC SPD4.1.2.L-6.
> 
> Are these numbers correct? I downloaded SPD4.1.2.L-6 now and it looks
> like 322 is manufacturing location and 323-324 are manufacturing date.
> (Also, I think all of these are specific to DDR4 (and possibly 5?),
> but not to earlier versions. I don't think we need to list it for
> every version, but we should at least be specific what it applies to.)

I just reopen the doc and you are right, didn't have my glasses on I guess.
Accordingly to SPD4.1.2.L-6 it the info seems in the byte 4 on bits 3~0.

Best regards,
Clément
Re: [PATCH v4 05/20] dt-bindings: memory: factorise LPDDR props into SDRAM props
Posted by Julius Werner 2 months, 1 week ago
> > I don't think this will identify a part unambiguously, I would expect
> > the DDR revision ID to be specific to the part number. (In fact, we're
> > also not sure whether manufacturer+revision identifies LPDDR parts
> > unambiguously for every vendor, we just didn't have anything more to
> > work with there.) I would suggest to use either `ddrX-YYYY,AAA...,ZZ`
> > or `ddrX-YYYY,ZZ,AAA...` (where AAA... is the part number string from
> > SPD 329-348 without the trailing spaces). The first version looks a
> > bit more natural but it might get confusing on the off chance that
> > someone uses a comma in a part number string.
>
> The first one seems better indeed.
> If the manufacturer put a comma in the part number we should handle it
> at a software level to me and if it is a devicetree error it is up to
> the devicetree writer to fix it.
> What do you think ?

Not sure what you mean by "handle it at a software level"? Using comma
characters in the part number is not illegal according to the SPD
spec, as far as I can tell.

That said, it is still possible to disambiguate this as long as the
revision number is always there, you just have to look for the last
comma from the end (so e.g. the string `ddr4-1234,some,part,567,89`
could be unambiguously parsed as manufacturer ID 0x1234, part number
`some,part,567` and revision ID 0x89, the parsing code just needs to
be a bit careful). So maybe this is not actually a problem.
Re: [PATCH v4 05/20] dt-bindings: memory: factorise LPDDR props into SDRAM props
Posted by Clement LE GOFFIC 2 months, 1 week ago
Hi Julius,

On 7/25/25 00:33, Julius Werner wrote:
>>> I don't think this will identify a part unambiguously, I would expect
>>> the DDR revision ID to be specific to the part number. (In fact, we're
>>> also not sure whether manufacturer+revision identifies LPDDR parts
>>> unambiguously for every vendor, we just didn't have anything more to
>>> work with there.) I would suggest to use either `ddrX-YYYY,AAA...,ZZ`
>>> or `ddrX-YYYY,ZZ,AAA...` (where AAA... is the part number string from
>>> SPD 329-348 without the trailing spaces). The first version looks a
>>> bit more natural but it might get confusing on the off chance that
>>> someone uses a comma in a part number string.
>>
>> The first one seems better indeed.
>> If the manufacturer put a comma in the part number we should handle it
>> at a software level to me and if it is a devicetree error it is up to
>> the devicetree writer to fix it.
>> What do you think ?

I meant exactly what you are stating below :-)

> 
> Not sure what you mean by "handle it at a software level"? Using comma
> characters in the part number is not illegal according to the SPD
> spec, as far as I can tell.
> 
> That said, it is still possible to disambiguate this as long as the
> revision number is always there, you just have to look for the last
> comma from the end (so e.g. the string `ddr4-1234,some,part,567,89`
> could be unambiguously parsed as manufacturer ID 0x1234, part number
> `some,part,567` and revision ID 0x89, the parsing code just needs to
> be a bit careful). So maybe this is not actually a problem.

Best regards,
Clément