[PATCH V6 0/3] nvmem: add fixed cells layout

[PATCH V6 0/3] nvmem: add fixed cells layout

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

NVMEM bindings received a new feature recently: layouts. They allow 
defining NVMEM device content in a separated DT node named 
"nvmem-layout". That improvement results in cleaner and more generic
bindings.

New bindings & drivers should use layouts design. If possible we should
also try converting existing bindings & drivers to layouts.

This patchset allows defining NVMEM cells in a new way (using NVMEM
layout feature).

This deprecates support for defining NVMEM cells as direct DT device
node subnodes but IT DOESN'T drop support for that.

PLEASE understand what this patchset handles. It adds new & preferred
way of defining NVMEM cells. This patchset does NOT meant to:
1. Convert ALL existing bindings or drivers (it's a task for later)
2. Add support for new extra features (like multiple layouts)
3. Break any backward compatibility (it WON'T happen)

V5:
Support "fixed-layout" internally in NVMEM core (see PATCH 3/3)
V6:
Rename function & add unevaluatedProperties

Rafał Miłecki (3):
  dt-bindings: nvmem: layouts: add fixed-layout
  dt-bindings: nvmem: convert base example to use NVMEM fixed cells
    layout
  nvmem: core: add support for fixed cells *layout*

 .../bindings/nvmem/layouts/fixed-cell.yaml    | 31 ++++++++++
 .../bindings/nvmem/layouts/fixed-layout.yaml  | 50 +++++++++++++++
 .../bindings/nvmem/layouts/nvmem-layout.yaml  |  5 +-
 .../devicetree/bindings/nvmem/nvmem.yaml      | 61 ++++++++-----------
 drivers/nvmem/core.c                          | 32 +++++++++-
 5 files changed, 137 insertions(+), 42 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
 create mode 100644 Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml

-- 
2.34.1

[PATCH V6 1/3] dt-bindings: nvmem: layouts: add fixed-layout

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

With the introduction of NVMEM layouts, new NVMEM content structures
should be defined as such. We should also try to convert / migrate
existing NVMEM content bindings to layouts.

This commit handles fixed NVMEM cells. So far they had to be defined
directly - as device subnodes. With this change it's allowed to put them
in the DT node named "nvmem-layout".

Having NVMEM cells in separated node is preferred as it draws a nice
line between NVMEM device and its content. It results in cleaner
bindings.

FWIW a very similar situation has happened to MTD devices and their
partitions: see commit 5d96ea42eb63 ("dt-bindings: mtd: Clarify all
partition subnodes").

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V6: Add unevaluatedProperties: false
---
 .../bindings/nvmem/layouts/fixed-cell.yaml    | 31 ++++++++++++
 .../bindings/nvmem/layouts/fixed-layout.yaml  | 50 +++++++++++++++++++
 .../bindings/nvmem/layouts/nvmem-layout.yaml  |  5 +-
 .../devicetree/bindings/nvmem/nvmem.yaml      | 19 +------
 4 files changed, 84 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

diff --git a/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml b/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
new file mode 100644
index 000000000000..e698098450e1
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
@@ -0,0 +1,31 @@
+# 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 <rafal@milecki.pl>
+  - Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
+
+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
diff --git a/Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml b/Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml
new file mode 100644
index 000000000000..c271537d0714
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/nvmem/layouts/fixed-layout.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: NVMEM layout for fixed NVMEM cells
+
+description:
+  Many NVMEM devices have hardcoded cells layout (offset and size of defined
+  NVMEM content doesn't change).
+
+  This binding allows defining such NVMEM layout with its cells. It can be used
+  on top of any NVMEM device.
+
+maintainers:
+  - Rafał Miłecki <rafal@milecki.pl>
+
+properties:
+  compatible:
+    const: fixed-layout
+
+  "#address-cells":
+    const: 1
+
+  "#size-cells":
+    const: 1
+
+patternProperties:
+  "@[a-f0-9]+$":
+    type: object
+    $ref: fixed-cell.yaml
+    unevaluatedProperties: false
+
+required:
+  - compatible
+
+additionalProperties: false
+
+examples:
+  - |
+    nvmem-layout {
+        compatible = "fixed-layout";
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        calibration@4000 {
+            reg = <0x4000 0x100>;
+        };
+    };
diff --git a/Documentation/devicetree/bindings/nvmem/layouts/nvmem-layout.yaml b/Documentation/devicetree/bindings/nvmem/layouts/nvmem-layout.yaml
index 8512ee538c4c..3b40f7880774 100644
--- a/Documentation/devicetree/bindings/nvmem/layouts/nvmem-layout.yaml
+++ b/Documentation/devicetree/bindings/nvmem/layouts/nvmem-layout.yaml
@@ -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
 
diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
index 75bb93dda9df..b79f1bb795fb 100644
--- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
@@ -49,23 +49,8 @@ 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
+    deprecated: true
 
 additionalProperties: true
 
-- 
2.34.1

[PATCH V6 2/3] dt-bindings: nvmem: convert base example to use NVMEM fixed cells layout

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

With support for the "fixed-layout" binding it's possible and preferred
now to define fixed NVMEM cells in the layout node. Do that for the
example binding.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../devicetree/bindings/nvmem/nvmem.yaml      | 42 +++++++++++--------
 1 file changed, 24 insertions(+), 18 deletions(-)

diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.yaml b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
index b79f1bb795fb..980244100690 100644
--- a/Documentation/devicetree/bindings/nvmem/nvmem.yaml
+++ b/Documentation/devicetree/bindings/nvmem/nvmem.yaml
@@ -68,24 +68,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>;
+              };
           };
       };
 
-- 
2.34.1

[PATCH V6 3/3] nvmem: core: add support for fixed cells *layout*

[Rafał Miłecki]

From: Rafał Miłecki <rafal@milecki.pl>

This adds support for the "fixed-layout" NVMEM layout binding. It allows
defining NVMEM cells in a layout DT node named "nvmem-layout".

While NVMEM subsystem supports layout drivers it has been discussed that
"fixed-layout" may actually be supperted internally. It's because:
1. It's a very basic layout
2. It allows sharing code with legacy syntax parsing
3. It's safer for soc_device_match() due to -EPROBE_DEFER
4. This will make the syntax transition easier

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Reviewed-by: Michael Walle <michael@walle.cc>
---
V6: s/nvmem_add_cells_fixed/nvmem_add_cells_from_fixed_layout/
---
 drivers/nvmem/core.c | 32 +++++++++++++++++++++++++++++---
 1 file changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index a62973d010ff..b3d5a29477f9 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -696,7 +696,7 @@ static int nvmem_validate_keepouts(struct nvmem_device *nvmem)
 	return 0;
 }
 
-static int nvmem_add_cells_from_of(struct nvmem_device *nvmem)
+static int nvmem_add_cells_from_dt(struct nvmem_device *nvmem, struct device_node *np)
 {
 	struct nvmem_layout *layout = nvmem->layout;
 	struct device *dev = &nvmem->dev;
@@ -704,7 +704,7 @@ static int nvmem_add_cells_from_of(struct nvmem_device *nvmem)
 	const __be32 *addr;
 	int len, ret;
 
-	for_each_child_of_node(dev->of_node, child) {
+	for_each_child_of_node(np, child) {
 		struct nvmem_cell_info info = {0};
 
 		addr = of_get_property(child, "reg", &len);
@@ -742,6 +742,28 @@ static int nvmem_add_cells_from_of(struct nvmem_device *nvmem)
 	return 0;
 }
 
+static int nvmem_add_cells_from_legacy_of(struct nvmem_device *nvmem)
+{
+	return nvmem_add_cells_from_dt(nvmem, nvmem->dev.of_node);
+}
+
+static int nvmem_add_cells_from_fixed_layout(struct nvmem_device *nvmem)
+{
+	struct device_node *layout_np;
+	int err = 0;
+
+	layout_np = of_nvmem_layout_get_container(nvmem);
+	if (!layout_np)
+		return 0;
+
+	if (of_device_is_compatible(layout_np, "fixed-layout"))
+		err = nvmem_add_cells_from_dt(nvmem, layout_np);
+
+	of_node_put(layout_np);
+
+	return err;
+}
+
 int __nvmem_layout_register(struct nvmem_layout *layout, struct module *owner)
 {
 	layout->owner = owner;
@@ -972,7 +994,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	if (rval)
 		goto err_remove_cells;
 
-	rval = nvmem_add_cells_from_of(nvmem);
+	rval = nvmem_add_cells_from_legacy_of(nvmem);
 	if (rval)
 		goto err_remove_cells;
 
@@ -982,6 +1004,10 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	if (rval)
 		goto err_remove_cells;
 
+	rval = nvmem_add_cells_from_fixed_layout(nvmem);
+	if (rval)
+		goto err_remove_cells;
+
 	rval = nvmem_add_cells_from_layout(nvmem);
 	if (rval)
 		goto err_remove_cells;
-- 
2.34.1

Re: [PATCH V6 1/3] dt-bindings: nvmem: layouts: add fixed-layout

[Rob Herring]

On Mon, 27 Mar 2023 16:16:09 +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> With the introduction of NVMEM layouts, new NVMEM content structures
> should be defined as such. We should also try to convert / migrate
> existing NVMEM content bindings to layouts.
> 
> This commit handles fixed NVMEM cells. So far they had to be defined
> directly - as device subnodes. With this change it's allowed to put them
> in the DT node named "nvmem-layout".
> 
> Having NVMEM cells in separated node is preferred as it draws a nice
> line between NVMEM device and its content. It results in cleaner
> bindings.
> 
> FWIW a very similar situation has happened to MTD devices and their
> partitions: see commit 5d96ea42eb63 ("dt-bindings: mtd: Clarify all
> partition subnodes").
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V6: Add unevaluatedProperties: false
> ---
>  .../bindings/nvmem/layouts/fixed-cell.yaml    | 31 ++++++++++++
>  .../bindings/nvmem/layouts/fixed-layout.yaml  | 50 +++++++++++++++++++
>  .../bindings/nvmem/layouts/nvmem-layout.yaml  |  5 +-
>  .../devicetree/bindings/nvmem/nvmem.yaml      | 19 +------
>  4 files changed, 84 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
> 

Reviewed-by: Rob Herring <robh@kernel.org>

Re: [PATCH V6 0/3] nvmem: add fixed cells layout

[Rafał Miłecki]

Hi Srini,

On 27.03.2023 16:16, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> NVMEM bindings received a new feature recently: layouts. They allow
> defining NVMEM device content in a separated DT node named
> "nvmem-layout". That improvement results in cleaner and more generic
> bindings.
> 
> New bindings & drivers should use layouts design. If possible we should
> also try converting existing bindings & drivers to layouts.
> 
> This patchset allows defining NVMEM cells in a new way (using NVMEM
> layout feature).
> 
> This deprecates support for defining NVMEM cells as direct DT device
> node subnodes but IT DOESN'T drop support for that.
> 
> PLEASE understand what this patchset handles. It adds new & preferred
> way of defining NVMEM cells. This patchset does NOT meant to:
> 1. Convert ALL existing bindings or drivers (it's a task for later)
> 2. Add support for new extra features (like multiple layouts)
> 3. Break any backward compatibility (it WON'T happen)
> 
> V5:
> Support "fixed-layout" internally in NVMEM core (see PATCH 3/3)
> V6:
> Rename function & add unevaluatedProperties

This patchset seemed ready for 6.4 but I guess it has missed your window

I see you already picked some stuff into your for-next:
https://git.kernel.org/pub/scm/linux/kernel/git/srini/nvmem.git/log/?h=for-next
but I don't see those changes.

Can you pick this work too, please? I would like to avoid waiting
another month before getting this upstreamed.


> Rafał Miłecki (3):
>    dt-bindings: nvmem: layouts: add fixed-layout
>    dt-bindings: nvmem: convert base example to use NVMEM fixed cells
>      layout
>    nvmem: core: add support for fixed cells *layout*
> 
>   .../bindings/nvmem/layouts/fixed-cell.yaml    | 31 ++++++++++
>   .../bindings/nvmem/layouts/fixed-layout.yaml  | 50 +++++++++++++++
>   .../bindings/nvmem/layouts/nvmem-layout.yaml  |  5 +-
>   .../devicetree/bindings/nvmem/nvmem.yaml      | 61 ++++++++-----------
>   drivers/nvmem/core.c                          | 32 +++++++++-
>   5 files changed, 137 insertions(+), 42 deletions(-)
>   create mode 100644 Documentation/devicetree/bindings/nvmem/layouts/fixed-cell.yaml
>   create mode 100644 Documentation/devicetree/bindings/nvmem/layouts/fixed-layout.yaml
> 

Re: [PATCH V6 0/3] nvmem: add fixed cells layout

[Srinivas Kandagatla]

On Mon, 27 Mar 2023 16:16:08 +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> NVMEM bindings received a new feature recently: layouts. They allow
> defining NVMEM device content in a separated DT node named
> "nvmem-layout". That improvement results in cleaner and more generic
> bindings.
> 
> [...]

Applied, thanks!

[1/3] dt-bindings: nvmem: layouts: add fixed-layout
      commit: 4d0c9ce1a7a26947269a15fad622f1ffb6698627
[2/3] dt-bindings: nvmem: convert base example to use NVMEM fixed cells layout
      commit: 75dd9c7a421a239be6e7bd48e6b3a1474500fe98
[3/3] nvmem: core: add support for fixed cells *layout*
      commit: ac1c0d15aedc0863031bc8d678d5f96ab11d3a41

Best regards,
-- 
Srinivas Kandagatla <srinivas.kandagatla@linaro.org>