diff mbox series

dt-bindings: nvmem: convert U-Boot env to a layout

Message ID 20240705225821.13196-1-zajec5@gmail.com
State New
Headers show
Series dt-bindings: nvmem: convert U-Boot env to a layout | expand

Commit Message

Rafał Miłecki July 5, 2024, 10:58 p.m. UTC
From: Rafał Miłecki <rafal@milecki.pl>

U-Boot environment variables can be stored in various data sources. MTD
is just one of available options. Refactor DT binding into a layout so
it can be used with UBI volumes and other NVMEM devices.

Link: https://lore.kernel.org/all/20231221173421.13737-1-zajec5@gmail.com/
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
I'm sending this PATCH without Linux changes to see if this is the
right approach - for developers and (DT) maintainers to review it first.

My previous attempt (see above Link) turned out in refusal so I'm just
trying to save some time in case this one goes wrong as well.

Hopefully the included example (which I really think we should add)
explains well how I think this binding should be used with layouts.

If I get some positive feedback I'll work on V2 with actual Linux
changes.

 .../bindings/nvmem/layouts/nvmem-layout.yaml  |  1 +
 .../nvmem/{ => layouts}/u-boot,env.yaml       | 39 ++++++++++++++++---
 2 files changed, 35 insertions(+), 5 deletions(-)
 rename Documentation/devicetree/bindings/nvmem/{ => layouts}/u-boot,env.yaml (75%)

Comments

Miquel Raynal July 8, 2024, 6:32 a.m. UTC | #1
Hi Rafał,

zajec5@gmail.com wrote on Sat,  6 Jul 2024 00:58:21 +0200:

> From: Rafał Miłecki <rafal@milecki.pl>
> 
> U-Boot environment variables can be stored in various data sources. MTD
> is just one of available options. Refactor DT binding into a layout so
> it can be used with UBI volumes and other NVMEM devices.
> 
> Link: https://lore.kernel.org/all/20231221173421.13737-1-zajec5@gmail.com/
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> I'm sending this PATCH without Linux changes to see if this is the
> right approach - for developers and (DT) maintainers to review it first.
> 
> My previous attempt (see above Link) turned out in refusal so I'm just
> trying to save some time in case this one goes wrong as well.
> 
> Hopefully the included example (which I really think we should add)
> explains well how I think this binding should be used with layouts.
> 
> If I get some positive feedback I'll work on V2 with actual Linux
> changes.
> 

I am still in favor of this change. I know from a binding perspective
it might be questionable; but from an OS perspective it makes total
sense to deprecated the old U-Boot env "device" driver in favor of this.

Thanks,
Miquèl
Rafał Miłecki July 8, 2024, 9:42 a.m. UTC | #2
On 8.07.2024 08:32, Miquel Raynal wrote:
> zajec5@gmail.com wrote on Sat,  6 Jul 2024 00:58:21 +0200:
> 
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> U-Boot environment variables can be stored in various data sources. MTD
>> is just one of available options. Refactor DT binding into a layout so
>> it can be used with UBI volumes and other NVMEM devices.
>>
>> Link: https://lore.kernel.org/all/20231221173421.13737-1-zajec5@gmail.com/
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>> I'm sending this PATCH without Linux changes to see if this is the
>> right approach - for developers and (DT) maintainers to review it first.
>>
>> My previous attempt (see above Link) turned out in refusal so I'm just
>> trying to save some time in case this one goes wrong as well.
>>
>> Hopefully the included example (which I really think we should add)
>> explains well how I think this binding should be used with layouts.
>>
>> If I get some positive feedback I'll work on V2 with actual Linux
>> changes.
>>
> 
> I am still in favor of this change. I know from a binding perspective
> it might be questionable; but from an OS perspective it makes total
> sense to deprecated the old U-Boot env "device" driver in favor of this.

Please note that at this point I didn't decide to deprecate the old
binding (using "u-boot,env" for flash device partition). That would
require mixing "nvmem-layout" with "partitions" which I believe Rob
objected to.

My PATCH in its current form just allows reusing this binding with
"nvmem-layout"s which is needed for cases where we agreed to allow
"nvmem-layout" (UBI volumes and pure - non-MTD - NVMEM devices).

If we want to refactor this binding in MTD partitions context I hope we
can handle that at different time if we agree on some solution. I hope
and I don't think this should be a blocked for this PATCH.
Rob Herring (Arm) July 10, 2024, 9:26 p.m. UTC | #3
On Sat, 06 Jul 2024 00:58:21 +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> U-Boot environment variables can be stored in various data sources. MTD
> is just one of available options. Refactor DT binding into a layout so
> it can be used with UBI volumes and other NVMEM devices.
> 
> Link: https://lore.kernel.org/all/20231221173421.13737-1-zajec5@gmail.com/
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> I'm sending this PATCH without Linux changes to see if this is the
> right approach - for developers and (DT) maintainers to review it first.
> 
> My previous attempt (see above Link) turned out in refusal so I'm just
> trying to save some time in case this one goes wrong as well.
> 
> Hopefully the included example (which I really think we should add)
> explains well how I think this binding should be used with layouts.
> 
> If I get some positive feedback I'll work on V2 with actual Linux
> changes.
> 
>  .../bindings/nvmem/layouts/nvmem-layout.yaml  |  1 +
>  .../nvmem/{ => layouts}/u-boot,env.yaml       | 39 ++++++++++++++++---
>  2 files changed, 35 insertions(+), 5 deletions(-)
>  rename Documentation/devicetree/bindings/nvmem/{ => layouts}/u-boot,env.yaml (75%)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/nvmem/layouts/nvmem-layout.yaml b/Documentation/devicetree/bindings/nvmem/layouts/nvmem-layout.yaml
index 3b40f7880774..382507060651 100644
--- a/Documentation/devicetree/bindings/nvmem/layouts/nvmem-layout.yaml
+++ b/Documentation/devicetree/bindings/nvmem/layouts/nvmem-layout.yaml
@@ -21,6 +21,7 @@  oneOf:
   - $ref: fixed-layout.yaml
   - $ref: kontron,sl28-vpd.yaml
   - $ref: onie,tlv-layout.yaml
+  - $ref: u-boot,env.yaml
 
 properties:
   compatible: true
diff --git a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml b/Documentation/devicetree/bindings/nvmem/layouts/u-boot,env.yaml
similarity index 75%
rename from Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
rename to Documentation/devicetree/bindings/nvmem/layouts/u-boot,env.yaml
index 9c36afc7084b..56a8f55d4a09 100644
--- a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
+++ b/Documentation/devicetree/bindings/nvmem/layouts/u-boot,env.yaml
@@ -1,10 +1,10 @@ 
 # SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
 %YAML 1.2
 ---
-$id: http://devicetree.org/schemas/nvmem/u-boot,env.yaml#
+$id: http://devicetree.org/schemas/nvmem/layouts/u-boot,env.yaml#
 $schema: http://devicetree.org/meta-schemas/core.yaml#
 
-title: U-Boot environment variables
+title: U-Boot environment variables layout
 
 description: |
   U-Boot uses environment variables to store device parameters and
@@ -21,9 +21,6 @@  description: |
   This binding allows marking storage device (as containing env data) and
   specifying used format.
 
-  Right now only flash partition case is covered but it may be extended to e.g.
-  UBI volumes in the future.
-
   Variables can be defined as NVMEM device subnodes.
 
 maintainers:
@@ -42,6 +39,7 @@  properties:
         const: brcm,env
 
   reg:
+    description: Partition offset and size for env on top of MTD
     maxItems: 1
 
   bootcmd:
@@ -58,6 +56,17 @@  properties:
         description: The first argument is a MAC address offset.
         const: 1
 
+allOf:
+  - if:
+      properties:
+        $nodename:
+          not:
+            contains:
+              pattern: "^partition@[0-9a-f]+$"
+    then:
+      properties:
+        reg: false
+
 additionalProperties: false
 
 examples:
@@ -101,3 +110,23 @@  examples:
             };
         };
     };
+  - |
+    partition@0 {
+        reg = <0x0 0x100000>;
+        label = "ubi";
+        compatible = "linux,ubi";
+
+        volumes {
+            ubi-volume-u-boot-env {
+                volname = "env";
+
+                nvmem-layout {
+                    compatible = "u-boot,env";
+
+                    ethaddr {
+                        #nvmem-cell-cells = <1>;
+                    };
+                };
+            };
+        };
+    };