Skip to contentSkip to conversation

dt-bindings: nvmem: layouts: add fixed-layout

PATCH Superseded Rafał Miłecki 3 participants 7 messages Mar 10, 2023

Subsystems (2)

v3 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 dt-bindings: nvmem: layouts: add fixed-layout

From: Rafał Miłecki <[email protected]>

With the introduction of NVMEM layouts we should prefer and support describing all NVMEM devices content in the "nvmem-layout" node. That inludes using it for fixed NVMEM cells (those with hardcoded offset & size).

This seems to be cleaner design and more explicit.

Introduce a binding allowing fixed NVMEM cells as a type of layout. To avoid code duplication put shared part in the fixed-cell.yaml.

My plan is to deprecate old binding (NVMEM cells put directly in the device node) in next 2 months or so.

---
V2: Use fixed-cell.yaml
V3: Update commit description
Allow additionalProperties (used by st,stm32-romem.yaml) Allow #address-cells and #size-cells

---
 .../bindings/nvmem/layouts/fixed-cell.yaml    | 30 ++++++++++++
 .../bindings/nvmem/layouts/fixed-layout.yaml  | 49 +++++++++++++++++++
 .../bindings/nvmem/layouts/nvmem-layout.yaml  |  5 +-
 .../devicetree/bindings/nvmem/nvmem.yaml      | 18 +------
 4 files changed, 81 insertions(+), 21 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
 create mode 100644 Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml
Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml new file +30 -0
new file mode 100644
index 000000000000..a27a32424d69
@@ -0,0 +1,30 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/nvmem/layouts/fixed-cell.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Fixed offset & size NVMEM cell
+
+maintainers:
+  - Rafał Miłecki <[email protected]>
+
+properties:
+  reg:
+    maxItems: 1
+
+  bits:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    items:
+      - minimum: 0
+        maximum: 7
+        description:
+          Offset in bit within the address range specified by reg.
+      - minimum: 1
+        description:
+          Size in bit within the address range specified by reg.
+
+required:
+  - reg
+
+additionalProperties: true
Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml new file +49 -0

49 lines not on this page · raw message

Documentation/devicetree/bindings/nvmem/layouts/nvmem-layout.yaml +1 -4
index 8512ee538c4c..3b40f7880774 100644
@@ -18,16 +18,13 @@ description: |
   perform their parsing. The nvmem-layout container is here to describe these.
 
 oneOf:
+  - $ref: fixed-layout.yaml
   - $ref: kontron,sl28-vpd.yaml
   - $ref: onie,tlv-layout.yaml
 
 properties:
   compatible: true
 
-  '#address-cells': false
-
-  '#size-cells': false
-
 required:
   - compatible
 
Documentation/devicetree/bindings/nvmem/nvmem.yaml +1 -17
index 75bb93dda9df..732162e9d13e 100644
@@ -49,23 +49,7 @@ properties:
 patternProperties:
   "@[0-9a-f]+(,[0-7])?$":
     type: object
-
-    properties:
-      reg:
-        maxItems: 1
-        description:
-          Offset and size in bytes within the storage device.
-
-      bits:
-        $ref: /schemas/types.yaml#/definitions/uint32-array
-        items:
-          - minimum: 0
-            maximum: 7
-            description:
-              Offset in bit within the address range specified by reg.
-          - minimum: 1
-            description:
-              Size in bit within the address range specified by reg.
+    $ref: layouts/fixed-cell.yaml
 
 additionalProperties: true
 
Signature · 1 line
2.34.1
#1 lore raw

PATCH v3 1/1 dt-bindings: nvmem: convert base example to use "nvmem-layout" node

From: Rafał Miłecki <[email protected]>

With support for "fixed-layout" binding we can use now "nvmem-layout" even for fixed NVMEM cells. Use that in the base example as it should be preferred over placing cells directly in the device node.

New and other bindings should follow as old binding will get deprecated at some point.

---
 .../devicetree/bindings/nvmem/nvmem.yaml      | 42 +++++++++++--------
 1 file changed, 24 insertions(+), 18 deletions(-)
Documentation/devicetree/bindings/nvmem/nvmem.yaml +24 -18
index 732162e9d13e..c77be1c20e47 100644
@@ -67,24 +67,30 @@ examples:
 
           /* ... */
 
-          /* Data cells */
-          tsens_calibration: calib@404 {
-              reg = <0x404 0x10>;
-          };
-
-          tsens_calibration_bckp: calib_bckp@504 {
-              reg = <0x504 0x11>;
-              bits = <6 128>;
-          };
-
-          pvs_version: pvs-version@6 {
-              reg = <0x6 0x2>;
-              bits = <7 2>;
-          };
-
-          speed_bin: speed-bin@c{
-              reg = <0xc 0x1>;
-              bits = <2 3>;
+          nvmem-layout {
+              compatible = "fixed-layout";
+              #address-cells = <1>;
+              #size-cells = <1>;
+
+              /* Data cells */
+              tsens_calibration: calib@404 {
+                  reg = <0x404 0x10>;
+              };
+
+              tsens_calibration_bckp: calib_bckp@504 {
+                  reg = <0x504 0x11>;
+                  bits = <6 128>;
+              };
+
+              pvs_version: pvs-version@6 {
+                  reg = <0x6 0x2>;
+                  bits = <7 2>;
+              };
+
+              speed_bin: speed-bin@c{
+                  reg = <0xc 0x1>;
+                  bits = <2 3>;
+              };
           };
       };
 
Signature · 1 line
2.34.1
#2 lore raw

Re: [PATCH V3] dt-bindings: nvmem: convert base example to use "nvmem-layout" node

Hi Rafał,

[email protected] wrote on Fri, 10 Mar 2023 08:51:45 +0100:

Quoted 31 lines

From: Rafał Miłecki <[email protected]>

With support for "fixed-layout" binding we can use now "nvmem-layout" even for fixed NVMEM cells. Use that in the base example as it should be preferred over placing cells directly in the device node.

New and other bindings should follow as old binding will get deprecated at some point.

---
 .../devicetree/bindings/nvmem/nvmem.yaml      | 42 +++++++++++--------
 1 file changed, 24 insertions(+), 18 deletions(-)
Documentation/devicetree/bindings/nvmem/nvmem.yaml +0 -9
index 732162e9d13e..c77be1c20e47 100644
@@ -67,24 +67,30 @@ examples:
 
           /* ... */
 
-          /* Data cells */
-          tsens_calibration: calib@404 {
-              reg = <0x404 0x10>;
-          };
-
-          tsens_calibration_bckp: calib_bckp@504 {
-              reg = <0x504 0x11>;
-              bits = <6 128>;
-          };

Documentation/devicetree/bindings/nvmem/nvmem.yaml examples: · line 70

Quoted 10 lines
Documentation/devicetree/bindings/nvmem/nvmem.yaml +1 -9
-
-          pvs_version: pvs-version@6 {
-              reg = <0x6 0x2>;
-              bits = <7 2>;
-          };
-
-          speed_bin: speed-bin@c{
-              reg = <0xc 0x1>;
-              bits = <2 3>;
+          nvmem-layout {

I believe we should not introduce "intermediate state" bindings when this is not strictly required, in order to avoid confusion with what is backward and what is transitory. So I would expect this to only apply after the switch to:

	nvmem-layout@xxx {
		reg = <xxx>;

I don't care who will take care of it, but I think it would be better to have everything in one series.

Other than the "order" problematic which I think is important here, I agree with this series.

Quoted 16 lines
Documentation/devicetree/bindings/nvmem/nvmem.yaml +16 -0
+              compatible = "fixed-layout";
+              #address-cells = <1>;
+              #size-cells = <1>;
+
+              /* Data cells */
+              tsens_calibration: calib@404 {
+                  reg = <0x404 0x10>;
+              };
+
+              tsens_calibration_bckp: calib_bckp@504 {
+                  reg = <0x504 0x11>;
+                  bits = <6 128>;
+              };
+
+              pvs_version: pvs-version@6 {
+                  reg = <0x6 0x2>;

Documentation/devicetree/bindings/nvmem/nvmem.yaml examples: · line 96

Quoted 10 lines
Documentation/devicetree/bindings/nvmem/nvmem.yaml +7 -0
+                  bits = <7 2>;
+              };
+
+              speed_bin: speed-bin@c{
+                  reg = <0xc 0x1>;
+                  bits = <2 3>;
+              };
           };
       };
 

Thanks,
Miquèl

#3 lore raw

Re: [PATCH V3] dt-bindings: nvmem: convert base example to use "nvmem-layout" node

Quoted 51 lines · Miquel Raynal

Hi Rafał,

[email protected] wrote on Fri, 10 Mar 2023 08:51:45 +0100:

Quoted 31 lines

From: Rafał Miłecki <[email protected]>

With support for "fixed-layout" binding we can use now "nvmem-layout" even for fixed NVMEM cells. Use that in the base example as it should be preferred over placing cells directly in the device node.

New and other bindings should follow as old binding will get deprecated at some point.

---
  .../devicetree/bindings/nvmem/nvmem.yaml      | 42 +++++++++++--------
  1 file changed, 24 insertions(+), 18 deletions(-)
Documentation/devicetree/bindings/nvmem/nvmem.yaml +0 -9
index 732162e9d13e..c77be1c20e47 100644
@@ -67,24 +67,30 @@ examples:
 
            /* ... */
 
-          /* Data cells */
-          tsens_calibration: calib@404 {
-              reg = <0x404 0x10>;
-          };
-
-          tsens_calibration_bckp: calib_bckp@504 {
-              reg = <0x504 0x11>;
-              bits = <6 128>;
-          };

Documentation/devicetree/bindings/nvmem/nvmem.yaml examples: · line 70

Quoted 10 lines
Documentation/devicetree/bindings/nvmem/nvmem.yaml +1 -9
-
-          pvs_version: pvs-version@6 {
-              reg = <0x6 0x2>;
-              bits = <7 2>;
-          };
-
-          speed_bin: speed-bin@c{
-              reg = <0xc 0x1>;
-              bits = <2 3>;
+          nvmem-layout {

I believe we should not introduce "intermediate state" bindings when this is not strictly required, in order to avoid confusion with what is backward and what is transitory. So I would expect this to only apply after the switch to:

Quoted 8 lines
	nvmem-layout@xxx {
		reg = <xxx>;

I don't care who will take care of it, but I think it would be better to have everything in one series.

Other than the "order" problematic which I think is important here, I agree with this series.

I fail to see how / why:
1. Adding new NVMEM layout
2. Supporting mutliple NVMEM layouts
would depend on each other.

We already have 2 NVMEM layouts bindings. I'm just adding a new (third) one.

If having NVMEM layouts bindings puts us in any kind of intermediate state then we're already there. I don't think my new NVMEM layout changes this situation.

Quoted 25 lines
Quoted 25 lines
Documentation/devicetree/bindings/nvmem/nvmem.yaml +23 -0
+              compatible = "fixed-layout";
+              #address-cells = <1>;
+              #size-cells = <1>;
+
+              /* Data cells */
+              tsens_calibration: calib@404 {
+                  reg = <0x404 0x10>;
+              };
+
+              tsens_calibration_bckp: calib_bckp@504 {
+                  reg = <0x504 0x11>;
+                  bits = <6 128>;
+              };
+
+              pvs_version: pvs-version@6 {
+                  reg = <0x6 0x2>;
+                  bits = <7 2>;
+              };
+
+              speed_bin: speed-bin@c{
+                  reg = <0xc 0x1>;
+                  bits = <2 3>;
+              };
            };
        };
#4 lore raw

Re: [PATCH V3] dt-bindings: nvmem: convert base example to use "nvmem-layout" node

In reply to Rafał Miłecki
Quoted 6 lines · Rafał Miłecki

From: Rafał Miłecki <[email protected]>

With support for "fixed-layout" binding we can use now "nvmem-layout" even for fixed NVMEM cells. Use that in the base example as it should be preferred over placing cells directly in the device node.

Fixed layouts are the core part of nvmem, am not sure why you want to deprecate this. Either we derive the cell information dt or via layouts or some post processing they should still endup as fixed layouts. this way the core part is always same irrespective of where the cell info comes from.

--srini

Quoted 61 lines

New and other bindings should follow as old binding will get deprecated at some point.

---
  .../devicetree/bindings/nvmem/nvmem.yaml      | 42 +++++++++++--------
  1 file changed, 24 insertions(+), 18 deletions(-)
Documentation/devicetree/bindings/nvmem/nvmem.yaml +24 -18
index 732162e9d13e..c77be1c20e47 100644
@@ -67,24 +67,30 @@ examples:
 
            /* ... */
 
-          /* Data cells */
-          tsens_calibration: calib@404 {
-              reg = <0x404 0x10>;
-          };
-
-          tsens_calibration_bckp: calib_bckp@504 {
-              reg = <0x504 0x11>;
-              bits = <6 128>;
-          };
-
-          pvs_version: pvs-version@6 {
-              reg = <0x6 0x2>;
-              bits = <7 2>;
-          };
-
-          speed_bin: speed-bin@c{
-              reg = <0xc 0x1>;
-              bits = <2 3>;
+          nvmem-layout {
+              compatible = "fixed-layout";
+              #address-cells = <1>;
+              #size-cells = <1>;
+
+              /* Data cells */
+              tsens_calibration: calib@404 {
+                  reg = <0x404 0x10>;
+              };
+
+              tsens_calibration_bckp: calib_bckp@504 {
+                  reg = <0x504 0x11>;
+                  bits = <6 128>;
+              };
+
+              pvs_version: pvs-version@6 {
+                  reg = <0x6 0x2>;
+                  bits = <7 2>;
+              };
+
+              speed_bin: speed-bin@c{
+                  reg = <0xc 0x1>;
+                  bits = <2 3>;
+              };
            };
        };
 
#5 lore raw

Re: [PATCH V3] dt-bindings: nvmem: convert base example to use "nvmem-layout" node

Quoted 11 lines · Srinivas Kandagatla
Quoted 6 lines · Rafał Miłecki

From: Rafał Miłecki <[email protected]>

With support for "fixed-layout" binding we can use now "nvmem-layout" even for fixed NVMEM cells. Use that in the base example as it should be preferred over placing cells directly in the device node.

Fixed layouts are the core part of nvmem, am not sure why you want to deprecate this. Either we derive the cell information dt or via layouts or some post processing they should still endup as fixed layouts. this way the core part is always same irrespective of where the cell info comes from.

I really don't understand why my changes get misunderstood. It's just happened another time. Yesterday Michael wrote: "your motivation to drop handling the fixed cells".

Again:
I DON'T deprecate or drop support for fixed layouts (fixed NVMEM cells) I DON'T deprecate or drop support for fixed layouts (fixed NVMEM cells)

I just want NVMEM bindings to move location of DT fixed NVMEM cells from *device* node to *nvmem-layout* node.

Also:
I WON'T drop support for old binding. We stay backward compatible. I WON'T drop support for old binding. We stay backward compatible.

Quoted 58 lines
Quoted 58 lines

New and other bindings should follow as old binding will get deprecated at some point.

---
  .../devicetree/bindings/nvmem/nvmem.yaml      | 42 +++++++++++--------

  1 file changed, 24 insertions(+), 18 deletions(-)
Documentation/devicetree/bindings/nvmem/nvmem.yaml +24 -18
index 732162e9d13e..c77be1c20e47 100644
@@ -67,24 +67,30 @@ examples:
             /* ... */
-          /* Data cells */
-          tsens_calibration: calib@404 {
-              reg = <0x404 0x10>;
-          };
-
-          tsens_calibration_bckp: calib_bckp@504 {
-              reg = <0x504 0x11>;
-              bits = <6 128>;
-          };
-
-          pvs_version: pvs-version@6 {
-              reg = <0x6 0x2>;
-              bits = <7 2>;
-          };
-
-          speed_bin: speed-bin@c{
-              reg = <0xc 0x1>;
-              bits = <2 3>;
+          nvmem-layout {
+              compatible = "fixed-layout";
+              #address-cells = <1>;
+              #size-cells = <1>;
+
+              /* Data cells */
+              tsens_calibration: calib@404 {
+                  reg = <0x404 0x10>;
+              };
+
+              tsens_calibration_bckp: calib_bckp@504 {
+                  reg = <0x504 0x11>;
+                  bits = <6 128>;
+              };
+
+              pvs_version: pvs-version@6 {
+                  reg = <0x6 0x2>;
+                  bits = <7 2>;
+              };
+
+              speed_bin: speed-bin@c{
+                  reg = <0xc 0x1>;
+                  bits = <2 3>;
+              };
             };
         };
#6 lore raw

Re: [PATCH V3] dt-bindings: nvmem: convert base example to use "nvmem-layout" node

In reply to Srinivas Kandagatla

Hi Srinivas,

[email protected] wrote on Fri, 10 Mar 2023 09:29:18 +0000:

Quoted 9 lines · Rafał Miłecki
Quoted 6 lines

From: Rafał Miłecki <[email protected]>

With support for "fixed-layout" binding we can use now "nvmem-layout" even for fixed NVMEM cells. Use that in the base example as it should be preferred over placing cells directly in the device node.

Fixed layouts are the core part of nvmem, am not sure why you want to deprecate this. Either we derive the cell information dt or via layouts or some post processing they should still endup as fixed layouts. this way the core part is always same irrespective of where the cell info comes from.

Actually I was suspicious as first but we had a very similar case in mtd:
- People need partitioning so we add partitions in the mtd node
- People need more advanced partitioning and partitioning becomes a mess so we containerize everything in a "partitions" subnode. It definitely improves the readability and makes the code more
resilient: outside of the container, it's not a partition, period.

I believe that's what Rafał is trying to anticipate. Just moving the fixed cells declaration into a container (and we have one already:
nvmem-layout).

Thanks,
Miquèl