diff mbox series

[1/4,v2] dt-bindings: memory: Factor out common properties of LPDDR bindings

Message ID 20220907232914.243502-1-jwerner@chromium.org
State Superseded
Headers show
Series [1/4,v2] dt-bindings: memory: Factor out common properties of LPDDR bindings | expand

Commit Message

Julius Werner Sept. 7, 2022, 11:29 p.m. UTC
The bindings for different LPDDR versions mostly use the same kinds of
properties, so in order to reduce duplication when we're adding support
for more versions, this patch creates a new lpddr-props subschema that
can be referenced by the others to define these common parts. (This will
consider a few smaller I/O width and density numbers "legal" for LPDDR3
that are usually not used there, but this should be harmless.)

Signed-off-by: Julius Werner <jwerner@chromium.org>
---
 .../ddr/jedec,lpddr-props.yaml                | 52 +++++++++++++++++++
 .../memory-controllers/ddr/jedec,lpddr2.yaml  | 40 ++------------
 .../memory-controllers/ddr/jedec,lpddr3.yaml  | 32 ++----------
 3 files changed, 60 insertions(+), 64 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml

Changelog:

- v2:
  - removed minItems
  - reordered io-width enum from lowest to highest
  - moved `$ref` below `mainainers`
  - removed part about undeprecating manufacturer-id

Comments

Krzysztof Kozlowski Sept. 8, 2022, 2:04 p.m. UTC | #1
On 08/09/2022 01:29, Julius Werner wrote:
> This patch adds bindings for LPDDR4 and LPDDR5 memory analogous to the
> existing bindings for LPDDR2 and LPDDR3. For now, the new types are only
> needed for topology description, so other properties like timing
> parameters are omitted. They can be added later if needed.
> 
> Signed-off-by: Julius Werner <jwerner@chromium.org>
> ---
>  .../ddr/jedec,lpddr-props.yaml                |  4 ++
>  .../memory-controllers/ddr/jedec,lpddr4.yaml  | 36 ++++++++++++++
>  .../memory-controllers/ddr/jedec,lpddr5.yaml  | 47 +++++++++++++++++++
>  3 files changed, 87 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr4.yaml
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr5.yaml
> 
> Changelog:
> 
> - v2
>   - removed minItems
>   - moved `$ref` below `maintainers`
>   - renamed example node from `lpddr4` to `lpddr`
> 
> diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml
> index 4114cfa8de67f1..92ef660888f318 100644
> --- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml
> +++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml
> @@ -45,9 +45,13 @@ properties:
>        - 512
>        - 1024
>        - 2048
> +      - 3072
>        - 4096
> +      - 6144
>        - 8192
> +      - 12288
>        - 16384
> +      - 24576
>        - 32768
>  
>    io-width:
> diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr4.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr4.yaml
> new file mode 100644
> index 00000000000000..0b2394c032797f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr4.yaml
> @@ -0,0 +1,36 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/memory-controllers/ddr/jedec,lpddr4.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LPDDR4 SDRAM compliant to JEDEC JESD209-4
> +
> +maintainers:
> +  - Krzysztof Kozlowski <krzk@kernel.org>
> +
> +allOf:
> +  - $ref: "jedec,lpddr-props.yaml#"

No need for quotes.

> +
> +properties:
> +  compatible:
> +    items:
> +      - pattern: "^lpddr4-[0-9a-f]{2},[0-9a-f]{4}$"
> +      - const: jedec,lpddr4
> +
> +required:
> +  - compatible
> +  - density
> +  - io-width
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    lpddr {
> +        compatible = "lpddr4-ff,0100", "jedec,lpddr4";
> +        density = <8192>;
> +        io-width = <16>;
> +        manufacturer-id = <255>;
> +        revision-id = <1 0>;
> +    };
> diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr5.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr5.yaml
> new file mode 100644
> index 00000000000000..d86e1e30380e47
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr5.yaml
> @@ -0,0 +1,47 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/memory-controllers/ddr/jedec,lpddr5.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: LPDDR5 SDRAM compliant to JEDEC JESD209-5
> +
> +maintainers:
> +  - Krzysztof Kozlowski <krzk@kernel.org>
> +
> +allOf:
> +  - $ref: "jedec,lpddr-props.yaml#"

No need for quotes.

> +
> +properties:
> +  compatible:
> +    items:
> +      - pattern: "^lpddr5-[0-9a-f]{2},[0-9a-f]{4}$"
> +      - const: jedec,lpddr5
> +
> +  serial-id:
> +    $ref: /schemas/types.yaml#/definitions/uint32-array
> +    description:
> +      Serial IDs read from Mode Registers 47 through 54. One byte per uint32
> +      cell (i.e. <MR47 MR48 MR49 MR50 MR51 MR52 MR53 MR54>).
> +    maxItems: 8
> +    items:
> +      minimum: 0
> +      maximum: 255
> +
> +required:
> +  - compatible
> +  - density
> +  - io-width
> +
> +unevaluatedProperties: false
> +
> +examples:
> +  - |
> +    lpddr {
> +        compatible = "lpddr5-01,0200", "jedec,lpddr5";
> +        density = <8192>;
> +        io-width = <8>;
> +        manufacturer-id = <1>;

Does not look like you tested the bindings. Please run `make
dt_binding_check` (see
Documentation/devicetree/bindings/writing-schema.rst for instructions).

> +        revision-id = <2 0>;
> +        serial-id = <3 1 0 0 0 0 0 0>;
> +    };


Best regards,
Krzysztof
Rob Herring Sept. 12, 2022, 9:32 p.m. UTC | #2
On Wed, 07 Sep 2022 16:29:11 -0700, Julius Werner wrote:
> The bindings for different LPDDR versions mostly use the same kinds of
> properties, so in order to reduce duplication when we're adding support
> for more versions, this patch creates a new lpddr-props subschema that
> can be referenced by the others to define these common parts. (This will
> consider a few smaller I/O width and density numbers "legal" for LPDDR3
> that are usually not used there, but this should be harmless.)
> 
> Signed-off-by: Julius Werner <jwerner@chromium.org>
> ---
>  .../ddr/jedec,lpddr-props.yaml                | 52 +++++++++++++++++++
>  .../memory-controllers/ddr/jedec,lpddr2.yaml  | 40 ++------------
>  .../memory-controllers/ddr/jedec,lpddr3.yaml  | 32 ++----------
>  3 files changed, 60 insertions(+), 64 deletions(-)
>  create mode 100644 Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml
> 
> Changelog:
> 
> - v2:
>   - removed minItems
>   - reordered io-width enum from lowest to highest
>   - moved `$ref` below `mainainers`
>   - removed part about undeprecating manufacturer-id
> 

Acked-by: Rob Herring <robh@kernel.org>
Rob Herring Sept. 12, 2022, 9:37 p.m. UTC | #3
On Wed, 07 Sep 2022 16:29:12 -0700, Julius Werner wrote:
> This patch allows a new kind of compatible string for LPDDR parts in the
> device tree bindings, in addition to the existing hardcoded
> <vendor>,<part-number> strings. The new format contains manufacturer and
> part (revision) information in numerical form, such as lpddr3-ff,0201
> for an LPDDR3 part with manufacturer ID ff and revision ID 0201. This
> helps cases where LPDDR parts are probed at runtime by boot firmware and
> cannot be matched to hardcoded part numbers, such as the firmware on the
> qcom/sc7280-herobrine boards does (which supports 4 different memory
> configurations at the moment, and more are expected to be added later at
> a point where the boot firmware can no longer be updated to specifically
> accomodate them).
> 
> Signed-off-by: Julius Werner <jwerner@chromium.org>
> ---
>  .../memory-controllers/ddr/jedec,lpddr-props.yaml    | 10 ++++++++++
>  .../memory-controllers/ddr/jedec,lpddr2.yaml         |  8 +++++---
>  .../memory-controllers/ddr/jedec,lpddr3.yaml         | 12 ++++++++----
>  3 files changed, 23 insertions(+), 7 deletions(-)
> 
> Changelog:
> 
> - v2
>   - Updated commit message to describe intended use case as an example
> 

Acked-by: Rob Herring <robh@kernel.org>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml
new file mode 100644
index 00000000000000..02700ac3c387ec
--- /dev/null
+++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr-props.yaml
@@ -0,0 +1,52 @@ 
+# 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.
+
+maintainers:
+  - Krzysztof Kozlowski <krzk@kernel.org>
+
+properties:
+  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
+      - 4096
+      - 8192
+      - 16384
+      - 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 9d78f140609b6c..65f4926d39c254 100644
--- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr2.yaml
@@ -9,6 +9,9 @@  title: LPDDR2 SDRAM compliant to JEDEC JESD209-2
 maintainers:
   - Krzysztof Kozlowski <krzk@kernel.org>
 
+allOf:
+  - $ref: "jedec,lpddr-props.yaml#"
+
 properties:
   compatible:
     oneOf:
@@ -41,41 +44,6 @@  properties:
       Property is deprecated, use revision-id instead.
     deprecated: true
 
-  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>).
-    minItems: 2
-    maxItems: 2
-    items:
-      minimum: 0
-      maximum: 255
-
-  density:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description: |
-      Density in megabits of SDRAM chip. Obtained from device datasheet.
-    enum:
-      - 64
-      - 128
-      - 256
-      - 512
-      - 1024
-      - 2048
-      - 4096
-      - 8192
-      - 16384
-      - 32768
-
-  io-width:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description: |
-      IO bus width in bits of SDRAM chip. Obtained from device datasheet.
-    enum:
-      - 32
-      - 16
-      - 8
-
   tRRD-min-tck:
     $ref: /schemas/types.yaml#/definitions/uint32
     maximum: 16
@@ -168,7 +136,7 @@  required:
   - density
   - io-width
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |
diff --git a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml
index 48908a19473c3f..91b64943062472 100644
--- a/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml
+++ b/Documentation/devicetree/bindings/memory-controllers/ddr/jedec,lpddr3.yaml
@@ -9,6 +9,9 @@  title: LPDDR3 SDRAM compliant to JEDEC JESD209-3
 maintainers:
   - Krzysztof Kozlowski <krzk@kernel.org>
 
+allOf:
+  - $ref: "jedec,lpddr-props.yaml#"
+
 properties:
   compatible:
     items:
@@ -20,24 +23,6 @@  properties:
     const: 1
     deprecated: true
 
-  density:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description: |
-      Density in megabits of SDRAM chip.
-    enum:
-      - 4096
-      - 8192
-      - 16384
-      - 32768
-
-  io-width:
-    $ref: /schemas/types.yaml#/definitions/uint32
-    description: |
-      IO bus width in bits of SDRAM chip.
-    enum:
-      - 32
-      - 16
-
   manufacturer-id:
     $ref: /schemas/types.yaml#/definitions/uint32
     description: |
@@ -45,15 +30,6 @@  properties:
       deprecated, manufacturer should be derived from the compatible.
     deprecated: true
 
-  revision-id:
-    $ref: /schemas/types.yaml#/definitions/uint32-array
-    minItems: 2
-    maxItems: 2
-    items:
-      maximum: 255
-    description: |
-      Revision value of SDRAM chip read from Mode Registers 6 and 7.
-
   '#size-cells':
     const: 0
     deprecated: true
@@ -206,7 +182,7 @@  required:
   - density
   - io-width
 
-additionalProperties: false
+unevaluatedProperties: false
 
 examples:
   - |