[PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding

[PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding

[Rafał Miłecki]

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

U-Boot uses environment variables for storing device setup data. It
usually needs to be accessed by a bootloader, kernel and often
user-space.

This binding allows describing environment data located in a raw flash
partition. It's treated as NVMEM device and can be reused later for
other storage devices.

Using DT should be cleaner than hardcoding & duplicating such info in
multiple places. Bootloader & kernel can share DTS and user-space can
try reading it too or just have correct data exposed by a kernel.

A custom "compatible" string allows system to automatically load
relevant NVMEM driver but phandle can be also used for reading raw
location.

Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
V2: Update descriptions to don't make this binding MTD (flash partition)
    specific. Mention multiple possible storage ways.
V3: Drop
    allOf:
      - $ref: nvmem.yaml#
    as we don't use anything rom the nvmem.yaml. Thanks Rob.
---
 .../devicetree/bindings/nvmem/u-boot,env.yaml | 62 +++++++++++++++++++
 MAINTAINERS                                   |  5 ++
 2 files changed, 67 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/u-boot,env.yaml

diff --git a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
new file mode 100644
index 000000000000..e70b2a60cb9a
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
@@ -0,0 +1,62 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/nvmem/u-boot,env.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: U-Boot environment variables
+
+description: |
+  U-Boot uses environment variables to store device parameters and
+  configuration. They may be used for booting process, setup or keeping end user
+  info.
+
+  Data is stored using U-Boot specific formats (variant specific header and NUL
+  separated key-value pairs).
+
+  Environment data can be stored on various storage entities, e.g.:
+  1. Raw flash partition
+  2. UBI volume
+
+  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.
+
+maintainers:
+  - Rafał Miłecki <rafal@milecki.pl>
+
+properties:
+  compatible:
+    oneOf:
+      - description: A standalone env data block
+        const: u-boot,env
+      - description: Two redundant blocks with active one flagged
+        const: u-boot,env-redundant-bool
+      - description: Two redundant blocks with active having higher counter
+        const: u-boot,env-redundant-count
+
+  reg:
+    maxItems: 1
+
+additionalProperties: false
+
+examples:
+  - |
+    partitions {
+        compatible = "fixed-partitions";
+        #address-cells = <1>;
+        #size-cells = <1>;
+
+        partition@0 {
+            reg = <0x0 0x40000>;
+            label = "u-boot";
+            read-only;
+        };
+
+        env: partition@40000 {
+            compatible = "u-boot,env";
+            reg = <0x40000 0x10000>;
+        };
+    };
diff --git a/MAINTAINERS b/MAINTAINERS
index db8052bc1d26..24fc181a7e6c 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -19958,6 +19958,11 @@ W:	http://linuxtv.org
 T:	git git://linuxtv.org/media_tree.git
 F:	drivers/media/pci/tw686x/
 
+U-BOOT ENVIRONMENT VARIABLES
+M:	Rafał Miłecki <rafal@milecki.pl>
+S:	Maintained
+F:	Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
+
 UACCE ACCELERATOR FRAMEWORK
 M:	Zhangfei Gao <zhangfei.gao@linaro.org>
 M:	Zhou Wang <wangzhou1@hisilicon.com>
-- 
2.34.1

Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding

[Rob Herring]

On Mon, Feb 28, 2022 at 02:12:50PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> U-Boot uses environment variables for storing device setup data. It
> usually needs to be accessed by a bootloader, kernel and often
> user-space.
> 
> This binding allows describing environment data located in a raw flash
> partition. It's treated as NVMEM device and can be reused later for
> other storage devices.
> 
> Using DT should be cleaner than hardcoding & duplicating such info in
> multiple places. Bootloader & kernel can share DTS and user-space can
> try reading it too or just have correct data exposed by a kernel.
> 
> A custom "compatible" string allows system to automatically load
> relevant NVMEM driver but phandle can be also used for reading raw
> location.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Update descriptions to don't make this binding MTD (flash partition)
>     specific. Mention multiple possible storage ways.
> V3: Drop
>     allOf:
>       - $ref: nvmem.yaml#
>     as we don't use anything rom the nvmem.yaml. Thanks Rob.
> ---
>  .../devicetree/bindings/nvmem/u-boot,env.yaml | 62 +++++++++++++++++++
>  MAINTAINERS                                   |  5 ++
>  2 files changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/u-boot,env.yaml

A u-boot maintainer should ack this.

Rob

Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding

[Michal Simek]

On 2/28/22 14:12, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> U-Boot uses environment variables for storing device setup data. It
> usually needs to be accessed by a bootloader, kernel and often
> user-space.
> 
> This binding allows describing environment data located in a raw flash
> partition. It's treated as NVMEM device and can be reused later for
> other storage devices.
> 
> Using DT should be cleaner than hardcoding & duplicating such info in
> multiple places. Bootloader & kernel can share DTS and user-space can
> try reading it too or just have correct data exposed by a kernel.
> 
> A custom "compatible" string allows system to automatically load
> relevant NVMEM driver but phandle can be also used for reading raw
> location.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Update descriptions to don't make this binding MTD (flash partition)
>      specific. Mention multiple possible storage ways.
> V3: Drop
>      allOf:
>        - $ref: nvmem.yaml#
>      as we don't use anything rom the nvmem.yaml. Thanks Rob.
> ---
>   .../devicetree/bindings/nvmem/u-boot,env.yaml | 62 +++++++++++++++++++
>   MAINTAINERS                                   |  5 ++
>   2 files changed, 67 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> new file mode 100644
> index 000000000000..e70b2a60cb9a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> @@ -0,0 +1,62 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/u-boot,env.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: U-Boot environment variables
> +
> +description: |
> +  U-Boot uses environment variables to store device parameters and
> +  configuration. They may be used for booting process, setup or keeping end user
> +  info.
> +
> +  Data is stored using U-Boot specific formats (variant specific header and NUL
> +  separated key-value pairs).
> +
> +  Environment data can be stored on various storage entities, e.g.:
> +  1. Raw flash partition
> +  2. UBI volume
> +
> +  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.
> +
> +maintainers:
> +  - Rafał Miłecki <rafal@milecki.pl>
> +
> +properties:
> +  compatible:
> +    oneOf:
> +      - description: A standalone env data block
> +        const: u-boot,env
> +      - description: Two redundant blocks with active one flagged
> +        const: u-boot,env-redundant-bool
> +      - description: Two redundant blocks with active having higher counter
> +        const: u-boot,env-redundant-count
> +
> +  reg:
> +    maxItems: 1
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    partitions {
> +        compatible = "fixed-partitions";
> +        #address-cells = <1>;
> +        #size-cells = <1>;
> +
> +        partition@0 {
> +            reg = <0x0 0x40000>;
> +            label = "u-boot";
> +            read-only;
> +        };
> +
> +        env: partition@40000 {
> +            compatible = "u-boot,env";
> +            reg = <0x40000 0x10000>;
> +        };
> +    };
> diff --git a/MAINTAINERS b/MAINTAINERS
> index db8052bc1d26..24fc181a7e6c 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -19958,6 +19958,11 @@ W:	http://linuxtv.org
>   T:	git git://linuxtv.org/media_tree.git
>   F:	drivers/media/pci/tw686x/
>   
> +U-BOOT ENVIRONMENT VARIABLES
> +M:	Rafał Miłecki <rafal@milecki.pl>
> +S:	Maintained
> +F:	Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> +
>   UACCE ACCELERATOR FRAMEWORK
>   M:	Zhangfei Gao <zhangfei.gao@linaro.org>
>   M:	Zhou Wang <wangzhou1@hisilicon.com>

I think that parsing these partitions is quite sw intensive process and I can't 
still see the value to have compatible string here.
I would prefer to have just any link from u-boot node to partition instead.

But up to Simon or Tom to decide.

Thanks,
Michal

Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding

[Rafał Miłecki]

On 9.03.2022 14:42, Michal Simek wrote:
> On 2/28/22 14:12, Rafał Miłecki wrote:
>> From: Rafał Miłecki <rafal@milecki.pl>
>>
>> U-Boot uses environment variables for storing device setup data. It
>> usually needs to be accessed by a bootloader, kernel and often
>> user-space.
>>
>> This binding allows describing environment data located in a raw flash
>> partition. It's treated as NVMEM device and can be reused later for
>> other storage devices.
>>
>> Using DT should be cleaner than hardcoding & duplicating such info in
>> multiple places. Bootloader & kernel can share DTS and user-space can
>> try reading it too or just have correct data exposed by a kernel.
>>
>> A custom "compatible" string allows system to automatically load
>> relevant NVMEM driver but phandle can be also used for reading raw
>> location.
>>
>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>> ---
>> V2: Update descriptions to don't make this binding MTD (flash partition)
>>      specific. Mention multiple possible storage ways.
>> V3: Drop
>>      allOf:
>>        - $ref: nvmem.yaml#
>>      as we don't use anything rom the nvmem.yaml. Thanks Rob.
>> ---
>>   .../devicetree/bindings/nvmem/u-boot,env.yaml | 62 +++++++++++++++++++
>>   MAINTAINERS                                   |  5 ++
>>   2 files changed, 67 insertions(+)
>>   create mode 100644 Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
>>
>> diff --git a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
>> new file mode 100644
>> index 000000000000..e70b2a60cb9a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
>> @@ -0,0 +1,62 @@
>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>> +%YAML 1.2
>> +---
>> +$id: http://devicetree.org/schemas/nvmem/u-boot,env.yaml#
>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>> +
>> +title: U-Boot environment variables
>> +
>> +description: |
>> +  U-Boot uses environment variables to store device parameters and
>> +  configuration. They may be used for booting process, setup or keeping end user
>> +  info.
>> +
>> +  Data is stored using U-Boot specific formats (variant specific header and NUL
>> +  separated key-value pairs).
>> +
>> +  Environment data can be stored on various storage entities, e.g.:
>> +  1. Raw flash partition
>> +  2. UBI volume
>> +
>> +  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.
>> +
>> +maintainers:
>> +  - Rafał Miłecki <rafal@milecki.pl>
>> +
>> +properties:
>> +  compatible:
>> +    oneOf:
>> +      - description: A standalone env data block
>> +        const: u-boot,env
>> +      - description: Two redundant blocks with active one flagged
>> +        const: u-boot,env-redundant-bool
>> +      - description: Two redundant blocks with active having higher counter
>> +        const: u-boot,env-redundant-count
>> +
>> +  reg:
>> +    maxItems: 1
>> +
>> +additionalProperties: false
>> +
>> +examples:
>> +  - |
>> +    partitions {
>> +        compatible = "fixed-partitions";
>> +        #address-cells = <1>;
>> +        #size-cells = <1>;
>> +
>> +        partition@0 {
>> +            reg = <0x0 0x40000>;
>> +            label = "u-boot";
>> +            read-only;
>> +        };
>> +
>> +        env: partition@40000 {
>> +            compatible = "u-boot,env";
>> +            reg = <0x40000 0x10000>;
>> +        };
>> +    };
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index db8052bc1d26..24fc181a7e6c 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -19958,6 +19958,11 @@ W:    http://linuxtv.org
>>   T:    git git://linuxtv.org/media_tree.git
>>   F:    drivers/media/pci/tw686x/
>> +U-BOOT ENVIRONMENT VARIABLES
>> +M:    Rafał Miłecki <rafal@milecki.pl>
>> +S:    Maintained
>> +F:    Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
>> +
>>   UACCE ACCELERATOR FRAMEWORK
>>   M:    Zhangfei Gao <zhangfei.gao@linaro.org>
>>   M:    Zhou Wang <wangzhou1@hisilicon.com>
> 
> I think that parsing these partitions is quite sw intensive process and I can't still see the value to have compatible string here.
> I would prefer to have just any link from u-boot node to partition instead.
> 
> But up to Simon or Tom to decide.

In the first place DT should describe hardware / platform / device as it
is. Without taking shortcuts. If environment data can be stored in flash
device partition it should be described exactly as that.

Systems like Linux can benefit from that.

If some systems (e.g. a bootloader - U-Boot or any other one) can't
afford parsing / covering whole DT structure due to some limitations -
then we can come with helpers. I've no objections here.

In dt-schema [1] we have place for U-Boot specific options. Please see
0986f729eff0 ("dt-bindings: u-boot: Add an initial binding for config").

We can add support e.g. for
u-boot {
         (...)
         u-boot,env = <&env>;
};

[1] https://github.com/devicetree-org/dt-schema
[2] https://github.com/devicetree-org/dt-schema/commit/0986f729eff0f40a66e85ab9dfb37681bf025ac4

Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding

[Rob Herring]

On Wed, Mar 09, 2022 at 02:42:43PM +0100, Michal Simek wrote:
> 
> 
> On 2/28/22 14:12, Rafał Miłecki wrote:
> > From: Rafał Miłecki <rafal@milecki.pl>
> > 
> > U-Boot uses environment variables for storing device setup data. It
> > usually needs to be accessed by a bootloader, kernel and often
> > user-space.
> > 
> > This binding allows describing environment data located in a raw flash
> > partition. It's treated as NVMEM device and can be reused later for
> > other storage devices.
> > 
> > Using DT should be cleaner than hardcoding & duplicating such info in
> > multiple places. Bootloader & kernel can share DTS and user-space can
> > try reading it too or just have correct data exposed by a kernel.
> > 
> > A custom "compatible" string allows system to automatically load
> > relevant NVMEM driver but phandle can be also used for reading raw
> > location.
> > 
> > Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> > ---
> > V2: Update descriptions to don't make this binding MTD (flash partition)
> >      specific. Mention multiple possible storage ways.
> > V3: Drop
> >      allOf:
> >        - $ref: nvmem.yaml#
> >      as we don't use anything rom the nvmem.yaml. Thanks Rob.
> > ---
> >   .../devicetree/bindings/nvmem/u-boot,env.yaml | 62 +++++++++++++++++++
> >   MAINTAINERS                                   |  5 ++
> >   2 files changed, 67 insertions(+)
> >   create mode 100644 Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> > new file mode 100644
> > index 000000000000..e70b2a60cb9a
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> > @@ -0,0 +1,62 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/nvmem/u-boot,env.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: U-Boot environment variables
> > +
> > +description: |
> > +  U-Boot uses environment variables to store device parameters and
> > +  configuration. They may be used for booting process, setup or keeping end user
> > +  info.
> > +
> > +  Data is stored using U-Boot specific formats (variant specific header and NUL
> > +  separated key-value pairs).
> > +
> > +  Environment data can be stored on various storage entities, e.g.:
> > +  1. Raw flash partition
> > +  2. UBI volume
> > +
> > +  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.
> > +
> > +maintainers:
> > +  - Rafał Miłecki <rafal@milecki.pl>
> > +
> > +properties:
> > +  compatible:
> > +    oneOf:
> > +      - description: A standalone env data block
> > +        const: u-boot,env
> > +      - description: Two redundant blocks with active one flagged
> > +        const: u-boot,env-redundant-bool
> > +      - description: Two redundant blocks with active having higher counter
> > +        const: u-boot,env-redundant-count
> > +
> > +  reg:
> > +    maxItems: 1
> > +
> > +additionalProperties: false
> > +
> > +examples:
> > +  - |
> > +    partitions {
> > +        compatible = "fixed-partitions";
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +
> > +        partition@0 {
> > +            reg = <0x0 0x40000>;
> > +            label = "u-boot";
> > +            read-only;
> > +        };
> > +
> > +        env: partition@40000 {
> > +            compatible = "u-boot,env";
> > +            reg = <0x40000 0x10000>;
> > +        };
> > +    };
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index db8052bc1d26..24fc181a7e6c 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -19958,6 +19958,11 @@ W:	http://linuxtv.org
> >   T:	git git://linuxtv.org/media_tree.git
> >   F:	drivers/media/pci/tw686x/
> > +U-BOOT ENVIRONMENT VARIABLES
> > +M:	Rafał Miłecki <rafal@milecki.pl>
> > +S:	Maintained
> > +F:	Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> > +
> >   UACCE ACCELERATOR FRAMEWORK
> >   M:	Zhangfei Gao <zhangfei.gao@linaro.org>
> >   M:	Zhou Wang <wangzhou1@hisilicon.com>
> 
> I think that parsing these partitions is quite sw intensive process and I
> can't still see the value to have compatible string here.

It's always good to know what a node represents.

> I would prefer to have just any link from u-boot node to partition instead.

That's a separate issue and having 'compatible' in no way disallows 
that.

Rob

Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding

[Michal Simek]

On 3/9/22 16:40, Rob Herring wrote:
> On Wed, Mar 09, 2022 at 02:42:43PM +0100, Michal Simek wrote:
>>
>>
>> On 2/28/22 14:12, Rafał Miłecki wrote:
>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>
>>> U-Boot uses environment variables for storing device setup data. It
>>> usually needs to be accessed by a bootloader, kernel and often
>>> user-space.
>>>
>>> This binding allows describing environment data located in a raw flash
>>> partition. It's treated as NVMEM device and can be reused later for
>>> other storage devices.
>>>
>>> Using DT should be cleaner than hardcoding & duplicating such info in
>>> multiple places. Bootloader & kernel can share DTS and user-space can
>>> try reading it too or just have correct data exposed by a kernel.
>>>
>>> A custom "compatible" string allows system to automatically load
>>> relevant NVMEM driver but phandle can be also used for reading raw
>>> location.
>>>
>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>> ---
>>> V2: Update descriptions to don't make this binding MTD (flash partition)
>>>       specific. Mention multiple possible storage ways.
>>> V3: Drop
>>>       allOf:
>>>         - $ref: nvmem.yaml#
>>>       as we don't use anything rom the nvmem.yaml. Thanks Rob.
>>> ---
>>>    .../devicetree/bindings/nvmem/u-boot,env.yaml | 62 +++++++++++++++++++
>>>    MAINTAINERS                                   |  5 ++
>>>    2 files changed, 67 insertions(+)
>>>    create mode 100644 Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
>>>
>>> diff --git a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
>>> new file mode 100644
>>> index 000000000000..e70b2a60cb9a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
>>> @@ -0,0 +1,62 @@
>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>> +%YAML 1.2
>>> +---
>>> +$id: http://devicetree.org/schemas/nvmem/u-boot,env.yaml#
>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>> +
>>> +title: U-Boot environment variables
>>> +
>>> +description: |
>>> +  U-Boot uses environment variables to store device parameters and
>>> +  configuration. They may be used for booting process, setup or keeping end user
>>> +  info.
>>> +
>>> +  Data is stored using U-Boot specific formats (variant specific header and NUL
>>> +  separated key-value pairs).
>>> +
>>> +  Environment data can be stored on various storage entities, e.g.:
>>> +  1. Raw flash partition
>>> +  2. UBI volume
>>> +
>>> +  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.
>>> +
>>> +maintainers:
>>> +  - Rafał Miłecki <rafal@milecki.pl>
>>> +
>>> +properties:
>>> +  compatible:
>>> +    oneOf:
>>> +      - description: A standalone env data block
>>> +        const: u-boot,env
>>> +      - description: Two redundant blocks with active one flagged
>>> +        const: u-boot,env-redundant-bool
>>> +      - description: Two redundant blocks with active having higher counter
>>> +        const: u-boot,env-redundant-count
>>> +
>>> +  reg:
>>> +    maxItems: 1
>>> +
>>> +additionalProperties: false
>>> +
>>> +examples:
>>> +  - |
>>> +    partitions {
>>> +        compatible = "fixed-partitions";
>>> +        #address-cells = <1>;
>>> +        #size-cells = <1>;
>>> +
>>> +        partition@0 {
>>> +            reg = <0x0 0x40000>;
>>> +            label = "u-boot";
>>> +            read-only;
>>> +        };
>>> +
>>> +        env: partition@40000 {
>>> +            compatible = "u-boot,env";
>>> +            reg = <0x40000 0x10000>;
>>> +        };
>>> +    };
>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>> index db8052bc1d26..24fc181a7e6c 100644
>>> --- a/MAINTAINERS
>>> +++ b/MAINTAINERS
>>> @@ -19958,6 +19958,11 @@ W:	http://linuxtv.org
>>>    T:	git git://linuxtv.org/media_tree.git
>>>    F:	drivers/media/pci/tw686x/
>>> +U-BOOT ENVIRONMENT VARIABLES
>>> +M:	Rafał Miłecki <rafal@milecki.pl>
>>> +S:	Maintained
>>> +F:	Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
>>> +
>>>    UACCE ACCELERATOR FRAMEWORK
>>>    M:	Zhangfei Gao <zhangfei.gao@linaro.org>
>>>    M:	Zhou Wang <wangzhou1@hisilicon.com>
>>
>> I think that parsing these partitions is quite sw intensive process and I
>> can't still see the value to have compatible string here.
> 
> It's always good to know what a node represents.

Also agree but isn't it enough to use proper label for it?

> 
>> I would prefer to have just any link from u-boot node to partition instead.
> 
> That's a separate issue and having 'compatible' in no way disallows
> that.

Agree on this one.

Thanks,
Michal

Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding

[Rafał Miłecki]

On 10.03.2022 09:45, Michal Simek wrote:
> 
> 
> On 3/9/22 16:40, Rob Herring wrote:
>> On Wed, Mar 09, 2022 at 02:42:43PM +0100, Michal Simek wrote:
>>>
>>>
>>> On 2/28/22 14:12, Rafał Miłecki wrote:
>>>> From: Rafał Miłecki <rafal@milecki.pl>
>>>>
>>>> U-Boot uses environment variables for storing device setup data. It
>>>> usually needs to be accessed by a bootloader, kernel and often
>>>> user-space.
>>>>
>>>> This binding allows describing environment data located in a raw flash
>>>> partition. It's treated as NVMEM device and can be reused later for
>>>> other storage devices.
>>>>
>>>> Using DT should be cleaner than hardcoding & duplicating such info in
>>>> multiple places. Bootloader & kernel can share DTS and user-space can
>>>> try reading it too or just have correct data exposed by a kernel.
>>>>
>>>> A custom "compatible" string allows system to automatically load
>>>> relevant NVMEM driver but phandle can be also used for reading raw
>>>> location.
>>>>
>>>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
>>>> ---
>>>> V2: Update descriptions to don't make this binding MTD (flash partition)
>>>>       specific. Mention multiple possible storage ways.
>>>> V3: Drop
>>>>       allOf:
>>>>         - $ref: nvmem.yaml#
>>>>       as we don't use anything rom the nvmem.yaml. Thanks Rob.
>>>> ---
>>>>    .../devicetree/bindings/nvmem/u-boot,env.yaml | 62 +++++++++++++++++++
>>>>    MAINTAINERS                                   |  5 ++
>>>>    2 files changed, 67 insertions(+)
>>>>    create mode 100644 Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
>>>> new file mode 100644
>>>> index 000000000000..e70b2a60cb9a
>>>> --- /dev/null
>>>> +++ b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
>>>> @@ -0,0 +1,62 @@
>>>> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
>>>> +%YAML 1.2
>>>> +---
>>>> +$id: http://devicetree.org/schemas/nvmem/u-boot,env.yaml#
>>>> +$schema: http://devicetree.org/meta-schemas/core.yaml#
>>>> +
>>>> +title: U-Boot environment variables
>>>> +
>>>> +description: |
>>>> +  U-Boot uses environment variables to store device parameters and
>>>> +  configuration. They may be used for booting process, setup or keeping end user
>>>> +  info.
>>>> +
>>>> +  Data is stored using U-Boot specific formats (variant specific header and NUL
>>>> +  separated key-value pairs).
>>>> +
>>>> +  Environment data can be stored on various storage entities, e.g.:
>>>> +  1. Raw flash partition
>>>> +  2. UBI volume
>>>> +
>>>> +  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.
>>>> +
>>>> +maintainers:
>>>> +  - Rafał Miłecki <rafal@milecki.pl>
>>>> +
>>>> +properties:
>>>> +  compatible:
>>>> +    oneOf:
>>>> +      - description: A standalone env data block
>>>> +        const: u-boot,env
>>>> +      - description: Two redundant blocks with active one flagged
>>>> +        const: u-boot,env-redundant-bool
>>>> +      - description: Two redundant blocks with active having higher counter
>>>> +        const: u-boot,env-redundant-count
>>>> +
>>>> +  reg:
>>>> +    maxItems: 1
>>>> +
>>>> +additionalProperties: false
>>>> +
>>>> +examples:
>>>> +  - |
>>>> +    partitions {
>>>> +        compatible = "fixed-partitions";
>>>> +        #address-cells = <1>;
>>>> +        #size-cells = <1>;
>>>> +
>>>> +        partition@0 {
>>>> +            reg = <0x0 0x40000>;
>>>> +            label = "u-boot";
>>>> +            read-only;
>>>> +        };
>>>> +
>>>> +        env: partition@40000 {
>>>> +            compatible = "u-boot,env";
>>>> +            reg = <0x40000 0x10000>;
>>>> +        };
>>>> +    };
>>>> diff --git a/MAINTAINERS b/MAINTAINERS
>>>> index db8052bc1d26..24fc181a7e6c 100644
>>>> --- a/MAINTAINERS
>>>> +++ b/MAINTAINERS
>>>> @@ -19958,6 +19958,11 @@ W:    http://linuxtv.org
>>>>    T:    git git://linuxtv.org/media_tree.git
>>>>    F:    drivers/media/pci/tw686x/
>>>> +U-BOOT ENVIRONMENT VARIABLES
>>>> +M:    Rafał Miłecki <rafal@milecki.pl>
>>>> +S:    Maintained
>>>> +F:    Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
>>>> +
>>>>    UACCE ACCELERATOR FRAMEWORK
>>>>    M:    Zhangfei Gao <zhangfei.gao@linaro.org>
>>>>    M:    Zhou Wang <wangzhou1@hisilicon.com>
>>>
>>> I think that parsing these partitions is quite sw intensive process and I
>>> can't still see the value to have compatible string here.
>>
>> It's always good to know what a node represents.
> 
> Also agree but isn't it enough to use proper label for it?

Let me quote Rob here:

 > 'label' is supposed to correspond to a sticker on a port or something
 > human identifiable

^^ https://patchwork.ozlabs.org/comment/2812214/

"label" is already abused for naming MTD partitions, I don't think it's
a good idea to abuse it even more for different purposes. Also
"compatible" is a standard way for describing hardware blocks & various
entities (identifying them).


I'm also wondering if using "label" instead of "compatible" wouldn't
require breaking changes in some DT files. What if someone uses a random
"label" (e.g. "ub00tenv") and has user-space based on that MTD partition
name?
If we require changing "label" that will require people to also update
names in other places (user-space).
I'm not sure how valid is that argument, just wondering.

Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 932 bytes --]

On Mon, Feb 28, 2022 at 02:12:50PM +0100, Rafał Miłecki wrote:

> From: Rafał Miłecki <rafal@milecki.pl>
> 
> U-Boot uses environment variables for storing device setup data. It
> usually needs to be accessed by a bootloader, kernel and often
> user-space.
> 
> This binding allows describing environment data located in a raw flash
> partition. It's treated as NVMEM device and can be reused later for
> other storage devices.
> 
> Using DT should be cleaner than hardcoding & duplicating such info in
> multiple places. Bootloader & kernel can share DTS and user-space can
> try reading it too or just have correct data exposed by a kernel.
> 
> A custom "compatible" string allows system to automatically load
> relevant NVMEM driver but phandle can be also used for reading raw
> location.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>

Reviewed-by: Tom Rini <trini@konsulko.com>

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding

[Rob Herring]

On Mon, 28 Feb 2022 14:12:50 +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
> 
> U-Boot uses environment variables for storing device setup data. It
> usually needs to be accessed by a bootloader, kernel and often
> user-space.
> 
> This binding allows describing environment data located in a raw flash
> partition. It's treated as NVMEM device and can be reused later for
> other storage devices.
> 
> Using DT should be cleaner than hardcoding & duplicating such info in
> multiple places. Bootloader & kernel can share DTS and user-space can
> try reading it too or just have correct data exposed by a kernel.
> 
> A custom "compatible" string allows system to automatically load
> relevant NVMEM driver but phandle can be also used for reading raw
> location.
> 
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
> ---
> V2: Update descriptions to don't make this binding MTD (flash partition)
>     specific. Mention multiple possible storage ways.
> V3: Drop
>     allOf:
>       - $ref: nvmem.yaml#
>     as we don't use anything rom the nvmem.yaml. Thanks Rob.
> ---
>  .../devicetree/bindings/nvmem/u-boot,env.yaml | 62 +++++++++++++++++++
>  MAINTAINERS                                   |  5 ++
>  2 files changed, 67 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> 

Applied, thanks!