Re: [PATCH V2] dt-bindings: nvmem: u-boot,env: add Broadcom's variant binding

[Conor Dooley]

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@xxxxxxxxxx>
> > > > 
> > > > 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.@xxxxxxxxxx|
> > > > 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@xxxxxxxxxx>
> > > > ---
> > > > 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.