Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 18 Oct 2022 11:19:00 +0100
From:   Conor Dooley <conor.dooley@microchip.com>
To:     Rob Herring <robh@kernel.org>
CC:     =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= <zajec5@gmail.com>,
        =?utf-8?B?UmFmYcWCIE1pxYJlY2tp?= <rafal@milecki.pl>,
        William Zhang <william.zhang@broadcom.com>,
        Srinivas Kandagatla <srinivas.kandagatla@linaro.org>,
        Joel Peshkin <joel.peshkin@broadcom.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>,
        <bcm-kernel-feedback-list@broadcom.com>,
        <linux-arm-kernel@lists.infradead.org>,
        Rob Herring <robh+dt@kernel.org>,
        Tom Rini <trini@konsulko.com>, <u-boot@lists.denx.de>,
        <devicetree@vger.kernel.org>, <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH V2] dt-bindings: nvmem: u-boot,env: add Broadcom's
 variant binding
Message-ID: <Y059lG8ZOXXzc4N+@wendy>
References: <20220930163631.27040-1-zajec5@gmail.com>
 <166578177913.2909910.7600034251484225468.robh@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <166578177913.2909910.7600034251484225468.robh@kernel.org>
Precedence: bulk

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 {
                 };
             };
         };
-- 
2.38.0