All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot environment NVMEM provider
@ 2021-10-13 23:20 Marek Behún
  2021-10-14  8:26 ` Srinivas Kandagatla
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Marek Behún @ 2021-10-13 23:20 UTC (permalink / raw)
  To: robh+dt, devicetree, Srinivas Kandagatla, U-Boot Mailing List
  Cc: linux-kernel, netdev, Luka Kovacic, Marek Behún

Add device tree bindings for U-Boot environment NVMEM provider.

U-Boot environment can be stored at a specific offset of a MTD device,
EEPROM, MMC, NAND or SATA device, on an UBI volume, or in a file on a
filesystem.

The environment can contain information such as device's MAC address,
which should be used by the ethernet controller node.

Signed-off-by: Marek Behún <kabel@kernel.org>
---
 .../bindings/nvmem/denx,u-boot-env.yaml       | 88 +++++++++++++++++++
 include/dt-bindings/nvmem/u-boot-env.h        | 18 ++++
 2 files changed, 106 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml
 create mode 100644 include/dt-bindings/nvmem/u-boot-env.h

diff --git a/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml b/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml
new file mode 100644
index 000000000000..56505c08e622
--- /dev/null
+++ b/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml
@@ -0,0 +1,88 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/nvmem/denx,u-boot-env.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: U-Boot environment NVMEM Device Tree Bindings
+
+maintainers:
+  - Marek Behún <kabel@kernel.org>
+
+description:
+  This binding represents U-Boot's environment NVMEM settings which can be
+  stored on a specific offset of an EEPROM, MMC, NAND or SATA device, or
+  an UBI volume, or in a file on a filesystem.
+
+properties:
+  compatible:
+    const: denx,u-boot-env
+
+  path:
+    description:
+      The path to the file containing the environment if on a filesystem.
+    $ref: /schemas/types.yaml#/definitions/string
+
+patternProperties:
+  "^[^=]+$":
+    type: object
+
+    description:
+      This node represents one U-Boot environment variable, which is also one
+      NVMEM data cell.
+
+    properties:
+      name:
+        description:
+          If the variable name contains characters not allowed in device tree node
+          name, use this property to specify the name, otherwise the variable name
+          is equal to node name.
+        $ref: /schemas/types.yaml#/definitions/string
+
+      type:
+        description:
+          Type of the variable. Since variables, even integers and MAC addresses,
+          are stored as strings in U-Boot environment, for proper conversion the
+          type needs to be specified. Use one of the U_BOOT_ENV_TYPE_* prefixed
+          definitions from include/dt-bindings/nvmem/u-boot-env.h.
+        $ref: /schemas/types.yaml#/definitions/uint32
+        minimum: 0
+        maximum: 5
+
+    required:
+      - type
+
+required:
+  - compatible
+
+additionalProperties: true
+
+examples:
+  - |
+
+    #include <dt-bindings/nvmem/u-boot-env.h>
+
+    spi-flash {
+        partitions {
+            compatible = "fixed-partitions";
+            #address-cells = <1>;
+            #size-cells = <1>;
+
+            partition@180000 {
+                compatible = "denx,u-boot-env";
+                label = "u-boot-env";
+                reg = <0x180000 0x10000>;
+
+                eth0_addr: ethaddr {
+                    type = <U_BOOT_ENV_TYPE_MAC_ADDRESS>;
+                };
+            };
+        };
+    };
+
+    ethernet-controller {
+        nvmem-cells = <&eth0_addr>;
+        nvmem-cell-names = "mac-address";
+    };
+
+...
diff --git a/include/dt-bindings/nvmem/u-boot-env.h b/include/dt-bindings/nvmem/u-boot-env.h
new file mode 100644
index 000000000000..760e5b240619
--- /dev/null
+++ b/include/dt-bindings/nvmem/u-boot-env.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Macros for U-Boot environment NVMEM device tree bindings.
+ *
+ * Copyright (C) 2021 Marek Behún <kabel@kernel.org>
+ */
+
+#ifndef __DT_BINDINGS_NVMEM_U_BOOT_ENV_H
+#define __DT_BINDINGS_NVMEM_U_BOOT_ENV_H
+
+#define U_BOOT_ENV_TYPE_STRING		0
+#define U_BOOT_ENV_TYPE_ULONG		1
+#define U_BOOT_ENV_TYPE_BOOL		2
+#define U_BOOT_ENV_TYPE_MAC_ADDRESS	3
+#define U_BOOT_ENV_TYPE_ULONG_HEX	4
+#define U_BOOT_ENV_TYPE_ULONG_DEC	5
+
+#endif /* __DT_BINDINGS_NVMEM_U_BOOT_ENV_H */
-- 
2.32.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot environment NVMEM provider
  2021-10-13 23:20 [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot environment NVMEM provider Marek Behún
@ 2021-10-14  8:26 ` Srinivas Kandagatla
  2021-10-14 10:06   ` Marek Behún
  2021-10-14 13:33 ` Tom Rini
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Srinivas Kandagatla @ 2021-10-14  8:26 UTC (permalink / raw)
  To: Marek Behún, robh+dt, devicetree, U-Boot Mailing List
  Cc: linux-kernel, netdev, Luka Kovacic



On 14/10/2021 00:20, Marek Behún wrote:
> Add device tree bindings for U-Boot environment NVMEM provider.
> 
> U-Boot environment can be stored at a specific offset of a MTD device,
> EEPROM, MMC, NAND or SATA device, on an UBI volume, or in a file on a
> filesystem.
> 
> The environment can contain information such as device's MAC address,
> which should be used by the ethernet controller node.
> 

Have you looked at 
./Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml ?


--srini

> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>   .../bindings/nvmem/denx,u-boot-env.yaml       | 88 +++++++++++++++++++
>   include/dt-bindings/nvmem/u-boot-env.h        | 18 ++++
>   2 files changed, 106 insertions(+)
>   create mode 100644 Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml
>   create mode 100644 include/dt-bindings/nvmem/u-boot-env.h
> 
> diff --git a/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml b/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml
> new file mode 100644
> index 000000000000..56505c08e622
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/denx,u-boot-env.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: U-Boot environment NVMEM Device Tree Bindings
> +
> +maintainers:
> +  - Marek Behún <kabel@kernel.org>
> +
> +description:
> +  This binding represents U-Boot's environment NVMEM settings which can be
> +  stored on a specific offset of an EEPROM, MMC, NAND or SATA device, or
> +  an UBI volume, or in a file on a filesystem.
> +
> +properties:
> +  compatible:
> +    const: denx,u-boot-env
> +
> +  path:
> +    description:
> +      The path to the file containing the environment if on a filesystem.
> +    $ref: /schemas/types.yaml#/definitions/string
> +
> +patternProperties:
> +  "^[^=]+$":
> +    type: object
> +
> +    description:
> +      This node represents one U-Boot environment variable, which is also one
> +      NVMEM data cell.
> +
> +    properties:
> +      name:
> +        description:
> +          If the variable name contains characters not allowed in device tree node
> +          name, use this property to specify the name, otherwise the variable name
> +          is equal to node name.
> +        $ref: /schemas/types.yaml#/definitions/string
> +
> +      type:
> +        description:
> +          Type of the variable. Since variables, even integers and MAC addresses,
> +          are stored as strings in U-Boot environment, for proper conversion the
> +          type needs to be specified. Use one of the U_BOOT_ENV_TYPE_* prefixed
> +          definitions from include/dt-bindings/nvmem/u-boot-env.h.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0
> +        maximum: 5
> +
> +    required:
> +      - type
> +
> +required:
> +  - compatible
> +
> +additionalProperties: true
> +
> +examples:
> +  - |
> +
> +    #include <dt-bindings/nvmem/u-boot-env.h>
> +
> +    spi-flash {
> +        partitions {
> +            compatible = "fixed-partitions";
> +            #address-cells = <1>;
> +            #size-cells = <1>;
> +
> +            partition@180000 {
> +                compatible = "denx,u-boot-env";
> +                label = "u-boot-env";
> +                reg = <0x180000 0x10000>;
> +
> +                eth0_addr: ethaddr {
> +                    type = <U_BOOT_ENV_TYPE_MAC_ADDRESS>;
> +                };
> +            };
> +        };
> +    };
> +
> +    ethernet-controller {
> +        nvmem-cells = <&eth0_addr>;
> +        nvmem-cell-names = "mac-address";
> +    };
> +
> +...
> diff --git a/include/dt-bindings/nvmem/u-boot-env.h b/include/dt-bindings/nvmem/u-boot-env.h
> new file mode 100644
> index 000000000000..760e5b240619
> --- /dev/null
> +++ b/include/dt-bindings/nvmem/u-boot-env.h
> @@ -0,0 +1,18 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Macros for U-Boot environment NVMEM device tree bindings.
> + *
> + * Copyright (C) 2021 Marek Behún <kabel@kernel.org>
> + */
> +
> +#ifndef __DT_BINDINGS_NVMEM_U_BOOT_ENV_H
> +#define __DT_BINDINGS_NVMEM_U_BOOT_ENV_H
> +
> +#define U_BOOT_ENV_TYPE_STRING		0
> +#define U_BOOT_ENV_TYPE_ULONG		1
> +#define U_BOOT_ENV_TYPE_BOOL		2
> +#define U_BOOT_ENV_TYPE_MAC_ADDRESS	3
> +#define U_BOOT_ENV_TYPE_ULONG_HEX	4
> +#define U_BOOT_ENV_TYPE_ULONG_DEC	5
> +
> +#endif /* __DT_BINDINGS_NVMEM_U_BOOT_ENV_H */
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot environment NVMEM provider
  2021-10-14  8:26 ` Srinivas Kandagatla
@ 2021-10-14 10:06   ` Marek Behún
  2021-10-14 10:30     ` Srinivas Kandagatla
  0 siblings, 1 reply; 9+ messages in thread
From: Marek Behún @ 2021-10-14 10:06 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: robh+dt, devicetree, U-Boot Mailing List, linux-kernel, netdev,
	Luka Kovacic

On Thu, 14 Oct 2021 09:26:27 +0100
Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:

> On 14/10/2021 00:20, Marek Behún wrote:
> > Add device tree bindings for U-Boot environment NVMEM provider.
> > 
> > U-Boot environment can be stored at a specific offset of a MTD
> > device, EEPROM, MMC, NAND or SATA device, on an UBI volume, or in a
> > file on a filesystem.
> > 
> > The environment can contain information such as device's MAC
> > address, which should be used by the ethernet controller node.
> >   
> 
> Have you looked at 
> ./Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml ?

Hello srini,

yes, I have. What about it? :)

That binding won't work for u-boot-env, because the data are stored
in a different way. A cell does not have a constant predetermined
offset on the MTD.
The variables are stored as a sequence of values of format
"name=value", separated by '\0's, for example:
  board=turris_mox\0ethaddr=00:11:22:33:44:55\0bootcmd=run distro_bootcmd\0....
Chaning lengths of values of variables, or deleting variables, moves
the data around. Integers and MAC addresses are stored as strings, and so on.

Also the mtd/partitions/nvmem-cells.yaml doesn't take into account
u-boot-env stored on non-MTD devices.

Marek

  

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot environment NVMEM provider
  2021-10-14 10:06   ` Marek Behún
@ 2021-10-14 10:30     ` Srinivas Kandagatla
  2021-10-14 10:55       ` Marek Behún
  0 siblings, 1 reply; 9+ messages in thread
From: Srinivas Kandagatla @ 2021-10-14 10:30 UTC (permalink / raw)
  To: Marek Behún
  Cc: robh+dt, devicetree, U-Boot Mailing List, linux-kernel, netdev,
	Luka Kovacic



On 14/10/2021 11:06, Marek Behún wrote:
> On Thu, 14 Oct 2021 09:26:27 +0100
> Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
> 
>> On 14/10/2021 00:20, Marek Behún wrote:
>>> Add device tree bindings for U-Boot environment NVMEM provider.
>>>
>>> U-Boot environment can be stored at a specific offset of a MTD
>>> device, EEPROM, MMC, NAND or SATA device, on an UBI volume, or in a
>>> file on a filesystem.
>>>
>>> The environment can contain information such as device's MAC
>>> address, which should be used by the ethernet controller node.
>>>    
>>
>> Have you looked at
>> ./Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml ?
> 
> Hello srini,
> 
> yes, I have. What about it? :)
> 
> That binding won't work for u-boot-env, because the data are stored
> in a different way. A cell does not have a constant predetermined
> offset on the MTD.

Can't you dynamically update the nodes before nvmem-provider is registered?

> The variables are stored as a sequence of values of format
> "name=value", separated by '\0's, for example:
>    board=turris_mox\0ethaddr=00:11:22:33:44:55\0bootcmd=run distro_bootcmd\0....
> Chaning lengths of values of variables, or deleting variables, moves
> the data around. Integers and MAC addresses are stored as strings, and so on.
> 

Do you already have a provider driver for handing this.

How is pre parsing cell info and post processing data planned to be handled?

Currently in nvmem core we do check for "reg" property for each cell, 
unless the provider driver is adding/updating dt entries dynamically 
before registering nvmem provider, It will not work as it is. Alteast 
this is what I suggested in similar case where cell information is in 
tlv format.

Secondly mac-address seems to be delimited, we recently introduced post 
processing callback for provider driver [1], which should help in this case.

If the nvmem-cell names are standard like "mac-address" then you do not 
need to add a new "type" binding to cell too, you can do post-processing 
based on name.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/drivers/nvmem/imx-ocotp.c?id=823571f8c6f8968d8f14e91972fa350ce200f5db


--srini

> Also the mtd/partitions/nvmem-cells.yaml doesn't take into account
> u-boot-env stored on non-MTD devices.
> 
> Marek
> 
>    
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot environment NVMEM provider
  2021-10-14 10:30     ` Srinivas Kandagatla
@ 2021-10-14 10:55       ` Marek Behún
  2021-10-14 11:36         ` Srinivas Kandagatla
  0 siblings, 1 reply; 9+ messages in thread
From: Marek Behún @ 2021-10-14 10:55 UTC (permalink / raw)
  To: Srinivas Kandagatla
  Cc: robh+dt, devicetree, U-Boot Mailing List, linux-kernel, netdev,
	Luka Kovacic

On Thu, 14 Oct 2021 11:30:13 +0100
Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:

> On 14/10/2021 11:06, Marek Behún wrote:
> > On Thu, 14 Oct 2021 09:26:27 +0100
> > Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
> >   
> >> On 14/10/2021 00:20, Marek Behún wrote:  
> >>> Add device tree bindings for U-Boot environment NVMEM provider.
> >>>
> >>> U-Boot environment can be stored at a specific offset of a MTD
> >>> device, EEPROM, MMC, NAND or SATA device, on an UBI volume, or in
> >>> a file on a filesystem.
> >>>
> >>> The environment can contain information such as device's MAC
> >>> address, which should be used by the ethernet controller node.
> >>>      
> >>
> >> Have you looked at
> >> ./Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml
> >> ?  
> > 
> > Hello srini,
> > 
> > yes, I have. What about it? :)
> > 
> > That binding won't work for u-boot-env, because the data are stored
> > in a different way. A cell does not have a constant predetermined
> > offset on the MTD.  
> 
> Can't you dynamically update the nodes before nvmem-provider is
> registered?

Are you talking about dynamically updating nvmem-cell OF nodes, adding
reg properties with actual offsets and lengths found after parsing?

> > The variables are stored as a sequence of values of format
> > "name=value", separated by '\0's, for example:
> >    board=turris_mox\0ethaddr=00:11:22:33:44:55\0bootcmd=run
> > distro_bootcmd\0.... Chaning lengths of values of variables, or
> > deleting variables, moves the data around. Integers and MAC
> > addresses are stored as strings, and so on. 
> 
> Do you already have a provider driver for handing this.

Not yet, I will send the proposal together with a driver in next
round.

> How is pre parsing cell info and post processing data planned to be
> handled?

My plan was to read the variables from the u-boot-env partition, create
a nvmem-cell for each variable, and then pair the ones mentioned in
device tree with their DT nodes, and post-process according to type
(post-processing would be done only for those mentioned in device tree,
others would be left as strings).

> Currently in nvmem core we do check for "reg" property for each cell, 
> unless the provider driver is adding/updating dt entries dynamically 
> before registering nvmem provider

I don't think updaring DT entries dynamically is a correct solution at
all. Is this done in Linux? Updating device properties is something
different, but changing DT properties seems wrong to me.

> It will not work as it is. Alteast this is what I suggested in similar
> case where cell information is in tlv format.

Hmm. OK, I shall try to implement a driver for this and then will
return to you.

> Secondly mac-address seems to be delimited, we recently introduced
> post processing callback for provider driver [1], which should help
> in this case.

Cool, I shall use that.

> If the nvmem-cell names are standard like "mac-address" then you do
> not need to add a new "type" binding to cell too, you can do
> post-processing based on name.

I plan to add functions
  of_nvmem_get_mac_address()
  nvmem_get_mac_address()
which would look at (in this order):
  mac-address, address, mac-address-backup
We have to keep the name "address" for backwards compatibility with one
driver that uses this (drivers/net/ethernet/ni/nixge.c)

Thanks.

Marek

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot environment NVMEM provider
  2021-10-14 10:55       ` Marek Behún
@ 2021-10-14 11:36         ` Srinivas Kandagatla
  0 siblings, 0 replies; 9+ messages in thread
From: Srinivas Kandagatla @ 2021-10-14 11:36 UTC (permalink / raw)
  To: Marek Behún
  Cc: robh+dt, devicetree, U-Boot Mailing List, linux-kernel, netdev,
	Luka Kovacic



On 14/10/2021 11:55, Marek Behún wrote:
> On Thu, 14 Oct 2021 11:30:13 +0100
> Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
> 
>> On 14/10/2021 11:06, Marek Behún wrote:
>>> On Thu, 14 Oct 2021 09:26:27 +0100
>>> Srinivas Kandagatla <srinivas.kandagatla@linaro.org> wrote:
>>>    
>>>> On 14/10/2021 00:20, Marek Behún wrote:
>>>>> Add device tree bindings for U-Boot environment NVMEM provider.
>>>>>
>>>>> U-Boot environment can be stored at a specific offset of a MTD
>>>>> device, EEPROM, MMC, NAND or SATA device, on an UBI volume, or in
>>>>> a file on a filesystem.
>>>>>
>>>>> The environment can contain information such as device's MAC
>>>>> address, which should be used by the ethernet controller node.
>>>>>       
>>>>
>>>> Have you looked at
>>>> ./Documentation/devicetree/bindings/mtd/partitions/nvmem-cells.yaml
>>>> ?
>>>
>>> Hello srini,
>>>
>>> yes, I have. What about it? :)
>>>
>>> That binding won't work for u-boot-env, because the data are stored
>>> in a different way. A cell does not have a constant predetermined
>>> offset on the MTD.
>>
>> Can't you dynamically update the nodes before nvmem-provider is
>> registered?
> 
> Are you talking about dynamically updating nvmem-cell OF nodes, adding
> reg properties with actual offsets and lengths found after parsing?

Yes, atleast for the nodes that are defined in the dt.

> 
>>> The variables are stored as a sequence of values of format
>>> "name=value", separated by '\0's, for example:
>>>     board=turris_mox\0ethaddr=00:11:22:33:44:55\0bootcmd=run
>>> distro_bootcmd\0.... Chaning lengths of values of variables, or
>>> deleting variables, moves the data around. Integers and MAC
>>> addresses are stored as strings, and so on.
>>
>> Do you already have a provider driver for handing this.
> 
> Not yet, I will send the proposal together with a driver in next
> round.
> 
>> How is pre parsing cell info and post processing data planned to be
>> handled?
> 
> My plan was to read the variables from the u-boot-env partition, create
> a nvmem-cell for each variable, and then pair the ones mentioned in
> device tree with their DT nodes, and post-process according to type
Adding cells using nvmem_cell_info should work. I think pairing the one 
with DT entries is something that is missing. Currently nvmem_cell_info 
does not have device_node pointer may be that is something that could be 
added to help here.


> (post-processing would be done only for those mentioned in device tree,
> others would be left as strings).
> 
>> Currently in nvmem core we do check for "reg" property for each cell,
>> unless the provider driver is adding/updating dt entries dynamically
>> before registering nvmem provider
> 
> I don't think updaring DT entries dynamically is a correct solution at
> all. Is this done in Linux? Updating device properties is something
> different, but changing DT properties seems wrong to me.
> 
>> It will not work as it is. Alteast this is what I suggested in similar
>> case where cell information is in tlv format.
> 
> Hmm. OK, I shall try to implement a driver for this and then will
> return to you.

Sounds good.

> 
>> Secondly mac-address seems to be delimited, we recently introduced
>> post processing callback for provider driver [1], which should help
>> in this case.
> 
> Cool, I shall use that.
yes, please it should show up in 5.16 anyway.

> 
>> If the nvmem-cell names are standard like "mac-address" then you do
>> not need to add a new "type" binding to cell too, you can do
>> post-processing based on name.
> 
> I plan to add functions
>    of_nvmem_get_mac_address()
>    nvmem_get_mac_address()
we already have nvmem_get_mac_address() in  ./net/ethernet/eth.c that 
looks for mac-address.

--srini
> which would look at (in this order):
>    mac-address, address, mac-address-backup
> We have to keep the name "address" for backwards compatibility with one
> driver that uses this (drivers/net/ethernet/ni/nixge.c)
> 
> Thanks.
> 
> Marek
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot environment NVMEM provider
  2021-10-13 23:20 [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot environment NVMEM provider Marek Behún
  2021-10-14  8:26 ` Srinivas Kandagatla
@ 2021-10-14 13:33 ` Tom Rini
  2021-10-14 14:31 ` Rob Herring
  2021-10-14 14:41 ` Rob Herring
  3 siblings, 0 replies; 9+ messages in thread
From: Tom Rini @ 2021-10-14 13:33 UTC (permalink / raw)
  To: Marek Behún
  Cc: robh+dt, devicetree, Srinivas Kandagatla, U-Boot Mailing List,
	linux-kernel, netdev, Luka Kovacic

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

On Thu, Oct 14, 2021 at 01:20:48AM +0200, Marek Behún wrote:

> Add device tree bindings for U-Boot environment NVMEM provider.
> 
> U-Boot environment can be stored at a specific offset of a MTD device,
> EEPROM, MMC, NAND or SATA device, on an UBI volume, or in a file on a
> filesystem.
> 
> The environment can contain information such as device's MAC address,
> which should be used by the ethernet controller node.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>  .../bindings/nvmem/denx,u-boot-env.yaml       | 88 +++++++++++++++++++

We already have a vendor prefix for U-Boot, "u-boot" which should be
used here.

-- 
Tom

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot environment NVMEM provider
  2021-10-13 23:20 [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot environment NVMEM provider Marek Behún
  2021-10-14  8:26 ` Srinivas Kandagatla
  2021-10-14 13:33 ` Tom Rini
@ 2021-10-14 14:31 ` Rob Herring
  2021-10-14 14:41 ` Rob Herring
  3 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2021-10-14 14:31 UTC (permalink / raw)
  To: Marek Behún
  Cc: devicetree, linux-kernel, Srinivas Kandagatla, Luka Kovacic,
	netdev, robh+dt, U-Boot Mailing List

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain, Size: 2537 bytes --]

On Thu, 14 Oct 2021 01:20:48 +0200, Marek Behún wrote:
> Add device tree bindings for U-Boot environment NVMEM provider.
> 
> U-Boot environment can be stored at a specific offset of a MTD device,
> EEPROM, MMC, NAND or SATA device, on an UBI volume, or in a file on a
> filesystem.
> 
> The environment can contain information such as device's MAC address,
> which should be used by the ethernet controller node.
> 
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>  .../bindings/nvmem/denx,u-boot-env.yaml       | 88 +++++++++++++++++++
>  include/dt-bindings/nvmem/u-boot-env.h        | 18 ++++
>  2 files changed, 106 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml
>  create mode 100644 include/dt-bindings/nvmem/u-boot-env.h
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.example.dt.yaml: partition@180000: compatible: ['denx,u-boot-env'] is not of type 'object'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.example.dt.yaml: partition@180000: label: ['u-boot-env'] is not of type 'object'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.example.dt.yaml: partition@180000: reg: [[1572864, 65536]] is not of type 'object'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.example.dt.yaml: partition@180000: $nodename: ['partition@180000'] is not of type 'object'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/patch/1540721

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot environment NVMEM provider
  2021-10-13 23:20 [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot environment NVMEM provider Marek Behún
                   ` (2 preceding siblings ...)
  2021-10-14 14:31 ` Rob Herring
@ 2021-10-14 14:41 ` Rob Herring
  3 siblings, 0 replies; 9+ messages in thread
From: Rob Herring @ 2021-10-14 14:41 UTC (permalink / raw)
  To: Marek Behún
  Cc: devicetree, Srinivas Kandagatla, U-Boot Mailing List,
	linux-kernel, netdev, Luka Kovacic

On Wed, Oct 13, 2021 at 6:20 PM Marek Behún <kabel@kernel.org> wrote:
>
> Add device tree bindings for U-Boot environment NVMEM provider.
>
> U-Boot environment can be stored at a specific offset of a MTD device,
> EEPROM, MMC, NAND or SATA device, on an UBI volume, or in a file on a
> filesystem.
>
> The environment can contain information such as device's MAC address,
> which should be used by the ethernet controller node.
>
> Signed-off-by: Marek Behún <kabel@kernel.org>
> ---
>  .../bindings/nvmem/denx,u-boot-env.yaml       | 88 +++++++++++++++++++
>  include/dt-bindings/nvmem/u-boot-env.h        | 18 ++++
>  2 files changed, 106 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml
>  create mode 100644 include/dt-bindings/nvmem/u-boot-env.h
>
> diff --git a/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml b/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml
> new file mode 100644
> index 000000000000..56505c08e622
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/nvmem/denx,u-boot-env.yaml
> @@ -0,0 +1,88 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/nvmem/denx,u-boot-env.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: U-Boot environment NVMEM Device Tree Bindings
> +
> +maintainers:
> +  - Marek Behún <kabel@kernel.org>
> +
> +description:
> +  This binding represents U-Boot's environment NVMEM settings which can be
> +  stored on a specific offset of an EEPROM, MMC, NAND or SATA device, or
> +  an UBI volume, or in a file on a filesystem.
> +
> +properties:
> +  compatible:
> +    const: denx,u-boot-env

'u-boot' is a vendor prefix. Unless you are saying Denx owns u-boot...

> +
> +  path:
> +    description:
> +      The path to the file containing the environment if on a filesystem.
> +    $ref: /schemas/types.yaml#/definitions/string
> +
> +patternProperties:
> +  "^[^=]+$":
> +    type: object
> +
> +    description:
> +      This node represents one U-Boot environment variable, which is also one
> +      NVMEM data cell.
> +
> +    properties:
> +      name:

'name' is already a property for every node, so this would collide. It
used to be in the dtb itself, but current revisions generate it from
the node name.

> +        description:
> +          If the variable name contains characters not allowed in device tree node
> +          name, use this property to specify the name, otherwise the variable name
> +          is equal to node name.
> +        $ref: /schemas/types.yaml#/definitions/string
> +
> +      type:

'type' is really too generic. Any given property name should have 1
meaning and data type.

But I expect based on other comments already, all this is going away anyways.

> +        description:
> +          Type of the variable. Since variables, even integers and MAC addresses,
> +          are stored as strings in U-Boot environment, for proper conversion the
> +          type needs to be specified. Use one of the U_BOOT_ENV_TYPE_* prefixed
> +          definitions from include/dt-bindings/nvmem/u-boot-env.h.
> +        $ref: /schemas/types.yaml#/definitions/uint32
> +        minimum: 0
> +        maximum: 5

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2021-10-14 14:42 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-13 23:20 [PATCH RFC linux] dt-bindings: nvmem: Add binding for U-Boot environment NVMEM provider Marek Behún
2021-10-14  8:26 ` Srinivas Kandagatla
2021-10-14 10:06   ` Marek Behún
2021-10-14 10:30     ` Srinivas Kandagatla
2021-10-14 10:55       ` Marek Behún
2021-10-14 11:36         ` Srinivas Kandagatla
2021-10-14 13:33 ` Tom Rini
2021-10-14 14:31 ` Rob Herring
2021-10-14 14:41 ` Rob Herring

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.