| Message ID | 20220930163631.27040-1-zajec5@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [V2] dt-bindings: nvmem: u-boot, env: add Broadcom's variant binding | expand |
On Fri, 30 Sep 2022 18:36:31 +0200, Rafał Miłecki wrote: > From: Rafał Miłecki <rafal@milecki.pl> > > Broadcom uses U-Boot for a lot of their bcmbca familiy chipsets. U-Boot > stores its configuration in an environment data block. > > Such blocks are usually stored on flash as a separated partition at > hardcoded address. Broadcom however decided to: > 1. Store env data block inside U-Boot partition > 2. Avoid sticking to hardcoded offsets > 3. Use custom header with "uEnv" magic and env data length > > Example (length 0x4000): > $ hexdump -n 32 -C -s 0x40000 /dev/mtdblock0 > 00040000 76 6e 45 75 00 40 00 00 34 89 7a 82 49 4d 41 47 |vnEu.@..4.z.IMAG| > 00040010 45 3d 4e 41 4e 44 3a 31 4d 2c 31 30 32 34 4d 00 |E=NAND:1M,1024M.| > (0x40000 offset is unit specific and can change) > > Starting with the commit 118f3fbe517f4 ("dt-bindings: mtd: partitions: > support label/name only partition") DT can describe partitions matching > them by a name (without specifying actual address). With that feature > and this binding change it's possible to: > 1. Specify DT node for Broadcom's U-Boot env data subpartition > 2. Add nodes for specific environment data variables > 3. Reference them as NVMEM cells > > This binding is unlikely to help Broadcom's U-Boot. U-Boot SPL needs to > find environment data early (before it accesses DTB) and it does that by > looking for an "uEnv" magic. Dirty way. > > This binding can however be used by operating systems. It allows > describing cleanly U-Boot, its env data and variables. It tells > operating system about Broadcom-specific env data so it can parse it. > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > --- > V2: Work on better commit body & add example > --- > .../devicetree/bindings/nvmem/u-boot,env.yaml | 21 +++++++++++++++++++ > 1 file changed, 21 insertions(+) > Applied, thanks!
On Fri, Oct 14, 2022 at 04:09:40PM -0500, Rob Herring wrote: > On Fri, 30 Sep 2022 18:36:31 +0200, Rafał Miłecki wrote: > > From: Rafał Miłecki <rafal@milecki.pl> > > > > Broadcom uses U-Boot for a lot of their bcmbca familiy chipsets. U-Boot > > stores its configuration in an environment data block. > > > > Such blocks are usually stored on flash as a separated partition at > > hardcoded address. Broadcom however decided to: > > 1. Store env data block inside U-Boot partition > > 2. Avoid sticking to hardcoded offsets > > 3. Use custom header with "uEnv" magic and env data length > > > > Example (length 0x4000): > > $ hexdump -n 32 -C -s 0x40000 /dev/mtdblock0 > > 00040000 76 6e 45 75 00 40 00 00 34 89 7a 82 49 4d 41 47 |vnEu.@..4.z.IMAG| > > 00040010 45 3d 4e 41 4e 44 3a 31 4d 2c 31 30 32 34 4d 00 |E=NAND:1M,1024M.| > > (0x40000 offset is unit specific and can change) > > > > Starting with the commit 118f3fbe517f4 ("dt-bindings: mtd: partitions: > > support label/name only partition") DT can describe partitions matching > > them by a name (without specifying actual address). With that feature > > and this binding change it's possible to: > > 1. Specify DT node for Broadcom's U-Boot env data subpartition > > 2. Add nodes for specific environment data variables > > 3. Reference them as NVMEM cells > > > > This binding is unlikely to help Broadcom's U-Boot. U-Boot SPL needs to > > find environment data early (before it accesses DTB) and it does that by > > looking for an "uEnv" magic. Dirty way. > > > > This binding can however be used by operating systems. It allows > > describing cleanly U-Boot, its env data and variables. It tells > > operating system about Broadcom-specific env data so it can parse it. > > > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > > --- > > V2: Work on better commit body & add example > > --- > > .../devicetree/bindings/nvmem/u-boot,env.yaml | 21 +++++++++++++++++++ > > 1 file changed, 21 insertions(+) > > > > Applied, thanks! Hey Rob, Maybe my tooling is out of date or w/e but this is breaking dt_binding_check for me. I applied the below to fix the build, which I was about to send, before realising that you'd applied it and wondered if I was missing something. Thanks, Conor. -- >8 -- From b1b57f70a07b02f0119cc1543af49df294e0372c Mon Sep 17 00:00:00 2001 From: Conor Dooley <conor.dooley@microchip.com> Date: Tue, 18 Oct 2022 11:03:17 +0100 Subject: [PATCH] dt-bindings: nvmem: fix error in u-boot,env The duplicate mac address trips up dt_binding_check, causing it to fail: u-boot,env.example.dts:67.34-68.23: ERROR (duplicate_label): /example-1/partitions/partition@0/partition-u-boot-env/ethaddr: Duplicate label 'mac' on /example-1/partitions/partition@0/partition-u-boot-env/ethaddr and /example-0/partitions/partition@40000/ethaddr ERROR: Input tree has errors, aborting (use -f to force output) The unreferenced labels don't appear to add anything to the dt-schema examples, so just remove them. Fixes: c34f9f549927 ("dt-bindings: nvmem: u-boot,env: add Broadcom's variant binding") Signed-off-by: Conor Dooley <conor.dooley@microchip.com> --- Hopefully I've not missed the obvious & there's some u-boot parsing that depends on having this label & I've gone and ruined your example... CC: Rafał Miłecki <rafal@milecki.pl> CC: Srinivas Kandagatla <srinivas.kandagatla@linaro.org> CC: Rob Herring <robh+dt@kernel.org> CC: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org> CC: devicetree@vger.kernel.org CC: linux-kernel@vger.kernel.org --- Documentation/devicetree/bindings/nvmem/u-boot,env.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml index 987957e3ffc8..ebefd565c518 100644 --- a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml +++ b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml @@ -71,7 +71,7 @@ examples: compatible = "u-boot,env"; reg = <0x40000 0x10000>; - mac: ethaddr { + ethaddr { }; }; }; @@ -89,7 +89,7 @@ examples: partition-u-boot-env { compatible = "brcm,env"; - mac: ethaddr { + ethaddr { }; }; };
On Tue, Oct 18, 2022 at 5:19 AM Conor Dooley <conor.dooley@microchip.com> wrote: > > On Fri, Oct 14, 2022 at 04:09:40PM -0500, Rob Herring wrote: > > On Fri, 30 Sep 2022 18:36:31 +0200, Rafał Miłecki wrote: > > > From: Rafał Miłecki <rafal@milecki.pl> > > > > > > Broadcom uses U-Boot for a lot of their bcmbca familiy chipsets. U-Boot > > > stores its configuration in an environment data block. > > > > > > Such blocks are usually stored on flash as a separated partition at > > > hardcoded address. Broadcom however decided to: > > > 1. Store env data block inside U-Boot partition > > > 2. Avoid sticking to hardcoded offsets > > > 3. Use custom header with "uEnv" magic and env data length > > > > > > Example (length 0x4000): > > > $ hexdump -n 32 -C -s 0x40000 /dev/mtdblock0 > > > 00040000 76 6e 45 75 00 40 00 00 34 89 7a 82 49 4d 41 47 |vnEu.@..4.z.IMAG| > > > 00040010 45 3d 4e 41 4e 44 3a 31 4d 2c 31 30 32 34 4d 00 |E=NAND:1M,1024M.| > > > (0x40000 offset is unit specific and can change) > > > > > > Starting with the commit 118f3fbe517f4 ("dt-bindings: mtd: partitions: > > > support label/name only partition") DT can describe partitions matching > > > them by a name (without specifying actual address). With that feature > > > and this binding change it's possible to: > > > 1. Specify DT node for Broadcom's U-Boot env data subpartition > > > 2. Add nodes for specific environment data variables > > > 3. Reference them as NVMEM cells > > > > > > This binding is unlikely to help Broadcom's U-Boot. U-Boot SPL needs to > > > find environment data early (before it accesses DTB) and it does that by > > > looking for an "uEnv" magic. Dirty way. > > > > > > This binding can however be used by operating systems. It allows > > > describing cleanly U-Boot, its env data and variables. It tells > > > operating system about Broadcom-specific env data so it can parse it. > > > > > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > > > --- > > > V2: Work on better commit body & add example > > > --- > > > .../devicetree/bindings/nvmem/u-boot,env.yaml | 21 +++++++++++++++++++ > > > 1 file changed, 21 insertions(+) > > > > > > > Applied, thanks! > > Hey Rob, > Maybe my tooling is out of date or w/e but this is breaking > dt_binding_check for me. > > I applied the below to fix the build, which I was about to send, before > realising that you'd applied it and wondered if I was missing something. Indeed, it is broken. I've applied your fix. Thanks. Rob
On 18.10.2022 12:19, Conor Dooley wrote: > On Fri, Oct 14, 2022 at 04:09:40PM -0500, Rob Herring wrote: >> On Fri, 30 Sep 2022 18:36:31 +0200, Rafał Miłecki wrote: >>> From: Rafał Miłecki <rafal@milecki.pl> >>> >>> Broadcom uses U-Boot for a lot of their bcmbca familiy chipsets. U-Boot >>> stores its configuration in an environment data block. >>> >>> Such blocks are usually stored on flash as a separated partition at >>> hardcoded address. Broadcom however decided to: >>> 1. Store env data block inside U-Boot partition >>> 2. Avoid sticking to hardcoded offsets >>> 3. Use custom header with "uEnv" magic and env data length >>> >>> Example (length 0x4000): >>> $ hexdump -n 32 -C -s 0x40000 /dev/mtdblock0 >>> 00040000 76 6e 45 75 00 40 00 00 34 89 7a 82 49 4d 41 47 |vnEu.@..4.z.IMAG| >>> 00040010 45 3d 4e 41 4e 44 3a 31 4d 2c 31 30 32 34 4d 00 |E=NAND:1M,1024M.| >>> (0x40000 offset is unit specific and can change) >>> >>> Starting with the commit 118f3fbe517f4 ("dt-bindings: mtd: partitions: >>> support label/name only partition") DT can describe partitions matching >>> them by a name (without specifying actual address). With that feature >>> and this binding change it's possible to: >>> 1. Specify DT node for Broadcom's U-Boot env data subpartition >>> 2. Add nodes for specific environment data variables >>> 3. Reference them as NVMEM cells >>> >>> This binding is unlikely to help Broadcom's U-Boot. U-Boot SPL needs to >>> find environment data early (before it accesses DTB) and it does that by >>> looking for an "uEnv" magic. Dirty way. >>> >>> This binding can however be used by operating systems. It allows >>> describing cleanly U-Boot, its env data and variables. It tells >>> operating system about Broadcom-specific env data so it can parse it. >>> >>> Signed-off-by: Rafał Miłecki <rafal@milecki.pl> >>> --- >>> V2: Work on better commit body & add example >>> --- >>> .../devicetree/bindings/nvmem/u-boot,env.yaml | 21 +++++++++++++++++++ >>> 1 file changed, 21 insertions(+) >>> >> >> Applied, thanks! > > Hey Rob, > Maybe my tooling is out of date or w/e but this is breaking > dt_binding_check for me. > > I applied the below to fix the build, which I was about to send, before > realising that you'd applied it and wondered if I was missing something. Thanks for catching that and submitting a fix! I guess I didn't run dt_binding_check this time or I did it before adding an example. Sorry for that!
On Tue, Oct 18, 2022 at 03:58:38PM +0200, Rafał Miłecki wrote: > On 18.10.2022 12:19, Conor Dooley wrote: > > On Fri, Oct 14, 2022 at 04:09:40PM -0500, Rob Herring wrote: > > > On Fri, 30 Sep 2022 18:36:31 +0200, Rafał Miłecki wrote: > > > > From: Rafał Miłecki <rafal@milecki.pl> > > > > > > > > Broadcom uses U-Boot for a lot of their bcmbca familiy chipsets. U-Boot > > > > stores its configuration in an environment data block. > > > > > > > > Such blocks are usually stored on flash as a separated partition at > > > > hardcoded address. Broadcom however decided to: > > > > 1. Store env data block inside U-Boot partition > > > > 2. Avoid sticking to hardcoded offsets > > > > 3. Use custom header with "uEnv" magic and env data length > > > > > > > > Example (length 0x4000): > > > > $ hexdump -n 32 -C -s 0x40000 /dev/mtdblock0 > > > > 00040000 76 6e 45 75 00 40 00 00 34 89 7a 82 49 4d 41 47 |vnEu.@..4.z.IMAG| > > > > 00040010 45 3d 4e 41 4e 44 3a 31 4d 2c 31 30 32 34 4d 00 |E=NAND:1M,1024M.| > > > > (0x40000 offset is unit specific and can change) > > > > > > > > Starting with the commit 118f3fbe517f4 ("dt-bindings: mtd: partitions: > > > > support label/name only partition") DT can describe partitions matching > > > > them by a name (without specifying actual address). With that feature > > > > and this binding change it's possible to: > > > > 1. Specify DT node for Broadcom's U-Boot env data subpartition > > > > 2. Add nodes for specific environment data variables > > > > 3. Reference them as NVMEM cells > > > > > > > > This binding is unlikely to help Broadcom's U-Boot. U-Boot SPL needs to > > > > find environment data early (before it accesses DTB) and it does that by > > > > looking for an "uEnv" magic. Dirty way. > > > > > > > > This binding can however be used by operating systems. It allows > > > > describing cleanly U-Boot, its env data and variables. It tells > > > > operating system about Broadcom-specific env data so it can parse it. > > > > > > > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > > > > --- > > > > V2: Work on better commit body & add example > > > > --- > > > > .../devicetree/bindings/nvmem/u-boot,env.yaml | 21 +++++++++++++++++++ > > > > 1 file changed, 21 insertions(+) > > > > > > > > > > Applied, thanks! > > > > Hey Rob, > > Maybe my tooling is out of date or w/e but this is breaking > > dt_binding_check for me. > > > > I applied the below to fix the build, which I was about to send, before > > realising that you'd applied it and wondered if I was missing something. > > Thanks for catching that and submitting a fix! No worries. > > I guess I didn't run dt_binding_check this time or I did it before > adding an example. Sorry for that! BTW, subsequent to sending the fix I double checked my dt_binding_check logs and I saw: Documentation/devicetree/bindings/nvmem/u-boot,env.example.dtb: partition@0: Unevaluated properties are not allowed ('partition-u-boot-env' was unexpected) From schema: Documentation/devicetree/bindings/mtd/partitions/u-boot.yaml I think unevaluated property detection got better in v2022.08 of dt-schema so that is probably worth fixing too. I'll leave that one up to you ;) Thanks, Conor.
On Tue, Oct 18, 2022 at 9:10 AM Conor Dooley <conor.dooley@microchip.com> wrote: > > On Tue, Oct 18, 2022 at 03:58:38PM +0200, Rafał Miłecki wrote: > > On 18.10.2022 12:19, Conor Dooley wrote: > > > On Fri, Oct 14, 2022 at 04:09:40PM -0500, Rob Herring wrote: > > > > On Fri, 30 Sep 2022 18:36:31 +0200, Rafał Miłecki wrote: > > > > > From: Rafał Miłecki <rafal@milecki.pl> > > > > > > > > > > Broadcom uses U-Boot for a lot of their bcmbca familiy chipsets. U-Boot > > > > > stores its configuration in an environment data block. > > > > > > > > > > Such blocks are usually stored on flash as a separated partition at > > > > > hardcoded address. Broadcom however decided to: > > > > > 1. Store env data block inside U-Boot partition > > > > > 2. Avoid sticking to hardcoded offsets > > > > > 3. Use custom header with "uEnv" magic and env data length > > > > > > > > > > Example (length 0x4000): > > > > > $ hexdump -n 32 -C -s 0x40000 /dev/mtdblock0 > > > > > 00040000 76 6e 45 75 00 40 00 00 34 89 7a 82 49 4d 41 47 |vnEu.@..4.z.IMAG| > > > > > 00040010 45 3d 4e 41 4e 44 3a 31 4d 2c 31 30 32 34 4d 00 |E=NAND:1M,1024M.| > > > > > (0x40000 offset is unit specific and can change) > > > > > > > > > > Starting with the commit 118f3fbe517f4 ("dt-bindings: mtd: partitions: > > > > > support label/name only partition") DT can describe partitions matching > > > > > them by a name (without specifying actual address). With that feature > > > > > and this binding change it's possible to: > > > > > 1. Specify DT node for Broadcom's U-Boot env data subpartition > > > > > 2. Add nodes for specific environment data variables > > > > > 3. Reference them as NVMEM cells > > > > > > > > > > This binding is unlikely to help Broadcom's U-Boot. U-Boot SPL needs to > > > > > find environment data early (before it accesses DTB) and it does that by > > > > > looking for an "uEnv" magic. Dirty way. > > > > > > > > > > This binding can however be used by operating systems. It allows > > > > > describing cleanly U-Boot, its env data and variables. It tells > > > > > operating system about Broadcom-specific env data so it can parse it. > > > > > > > > > > Signed-off-by: Rafał Miłecki <rafal@milecki.pl> > > > > > --- > > > > > V2: Work on better commit body & add example > > > > > --- > > > > > .../devicetree/bindings/nvmem/u-boot,env.yaml | 21 +++++++++++++++++++ > > > > > 1 file changed, 21 insertions(+) > > > > > > > > > > > > > Applied, thanks! > > > > > > Hey Rob, > > > Maybe my tooling is out of date or w/e but this is breaking > > > dt_binding_check for me. > > > > > > I applied the below to fix the build, which I was about to send, before > > > realising that you'd applied it and wondered if I was missing something. > > > > Thanks for catching that and submitting a fix! > > No worries. > > > > > I guess I didn't run dt_binding_check this time or I did it before > > adding an example. Sorry for that! > > BTW, subsequent to sending the fix I double checked my dt_binding_check > logs and I saw: > Documentation/devicetree/bindings/nvmem/u-boot,env.example.dtb: partition@0: Unevaluated properties are not allowed ('partition-u-boot-env' was unexpected) > From schema: Documentation/devicetree/bindings/mtd/partitions/u-boot.yaml > > I think unevaluated property detection got better in v2022.08 of > dt-schema so that is probably worth fixing too. I'll leave that one up > to you ;) Sigh. The simple fix is add 'partition-u-boot-env' to u-boot.yaml. But now I have no idea if that is really complete as this Broadcom stuff is coming in bit by bit. So I've now just dropped everything. Please resend with it all fixed. Rob
diff --git a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml index e96bca99f2d9..987957e3ffc8 100644 --- a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml +++ b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml @@ -38,6 +38,8 @@ properties: const: u-boot,env-redundant-bool - description: Two redundant blocks with active having higher counter const: u-boot,env-redundant-count + - description: Broadcom's variant with custom header + const: brcm,env reg: maxItems: 1 @@ -73,3 +75,22 @@ examples: }; }; }; + - | + partitions { + compatible = "fixed-partitions"; + #address-cells = <1>; + #size-cells = <1>; + + partition@0 { + reg = <0x0 0x100000>; + compatible = "brcm,u-boot"; + label = "u-boot"; + + partition-u-boot-env { + compatible = "brcm,env"; + + mac: ethaddr { + }; + }; + }; + };