* Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding
2022-02-28 13:12 [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding Rafał Miłecki
@ 2022-03-07 23:18 ` Rob Herring
2022-03-09 13:42 ` Michal Simek
` (2 subsequent siblings)
3 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2022-03-07 23:18 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Tom Rini, Srinivas Kandagatla, Krzysztof Kozlowski,
Ricardo Salveti, Michal Simek, Jorge Ramirez-Ortiz,
Sean Anderson, devicetree, u-boot, linux-kernel,
Rafał Miłecki
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding
2022-02-28 13:12 [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding Rafał Miłecki
2022-03-07 23:18 ` Rob Herring
@ 2022-03-09 13:42 ` Michal Simek
2022-03-09 14:06 ` Rafał Miłecki
2022-03-09 15:40 ` Rob Herring
2022-03-23 13:32 ` Tom Rini
2022-03-23 16:56 ` Rob Herring
3 siblings, 2 replies; 9+ messages in thread
From: Michal Simek @ 2022-03-09 13:42 UTC (permalink / raw)
To: Rafał Miłecki, Rob Herring, Tom Rini, Simon Glass
Cc: Srinivas Kandagatla, Krzysztof Kozlowski, Ricardo Salveti,
Michal Simek, Jorge Ramirez-Ortiz, Sean Anderson, devicetree,
u-boot, linux-kernel, Rafał Miłecki
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding
2022-03-09 13:42 ` Michal Simek
@ 2022-03-09 14:06 ` Rafał Miłecki
2022-03-09 15:40 ` Rob Herring
1 sibling, 0 replies; 9+ messages in thread
From: Rafał Miłecki @ 2022-03-09 14:06 UTC (permalink / raw)
To: Michal Simek, Rob Herring, Tom Rini, Simon Glass
Cc: Srinivas Kandagatla, Krzysztof Kozlowski, Ricardo Salveti,
Jorge Ramirez-Ortiz, Sean Anderson, devicetree, u-boot,
linux-kernel, 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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding
2022-03-09 13:42 ` Michal Simek
2022-03-09 14:06 ` Rafał Miłecki
@ 2022-03-09 15:40 ` Rob Herring
2022-03-10 8:45 ` Michal Simek
1 sibling, 1 reply; 9+ messages in thread
From: Rob Herring @ 2022-03-09 15:40 UTC (permalink / raw)
To: Michal Simek
Cc: Rafał Miłecki, Tom Rini, Simon Glass,
Srinivas Kandagatla, Krzysztof Kozlowski, Ricardo Salveti,
Jorge Ramirez-Ortiz, Sean Anderson, devicetree, u-boot,
linux-kernel, Rafał Miłecki
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding
2022-03-09 15:40 ` Rob Herring
@ 2022-03-10 8:45 ` Michal Simek
2022-03-10 9:34 ` Rafał Miłecki
0 siblings, 1 reply; 9+ messages in thread
From: Michal Simek @ 2022-03-10 8:45 UTC (permalink / raw)
To: Rob Herring, Michal Simek
Cc: Rafał Miłecki, Tom Rini, Simon Glass,
Srinivas Kandagatla, Krzysztof Kozlowski, Ricardo Salveti,
Jorge Ramirez-Ortiz, Sean Anderson, devicetree, u-boot,
linux-kernel, Rafał Miłecki
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
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding
2022-03-10 8:45 ` Michal Simek
@ 2022-03-10 9:34 ` Rafał Miłecki
0 siblings, 0 replies; 9+ messages in thread
From: Rafał Miłecki @ 2022-03-10 9:34 UTC (permalink / raw)
To: Michal Simek, Rob Herring
Cc: Tom Rini, Simon Glass, Srinivas Kandagatla, Krzysztof Kozlowski,
Ricardo Salveti, Jorge Ramirez-Ortiz, Sean Anderson, devicetree,
u-boot, linux-kernel, 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.
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding
2022-02-28 13:12 [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding Rafał Miłecki
2022-03-07 23:18 ` Rob Herring
2022-03-09 13:42 ` Michal Simek
@ 2022-03-23 13:32 ` Tom Rini
2022-03-23 16:56 ` Rob Herring
3 siblings, 0 replies; 9+ messages in thread
From: Tom Rini @ 2022-03-23 13:32 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Rob Herring, Srinivas Kandagatla, Krzysztof Kozlowski,
Ricardo Salveti, Michal Simek, Jorge Ramirez-Ortiz,
Sean Anderson, devicetree, u-boot, linux-kernel,
Rafał Miłecki
[-- 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 --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding
2022-02-28 13:12 [PATCH V3] dt-bindings: nvmem: add U-Boot environment variables binding Rafał Miłecki
` (2 preceding siblings ...)
2022-03-23 13:32 ` Tom Rini
@ 2022-03-23 16:56 ` Rob Herring
3 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2022-03-23 16:56 UTC (permalink / raw)
To: Rafał Miłecki
Cc: Srinivas Kandagatla, Jorge Ramirez-Ortiz, Tom Rini, devicetree,
linux-kernel, Sean Anderson, Rafał Miłecki,
Michal Simek, Ricardo Salveti, Rob Herring, Krzysztof Kozlowski,
u-boot
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!
^ permalink raw reply [flat|nested] 9+ messages in thread