Re: [PATCH 1/3] dt-bindings: nvmem: u-boot,env: Add support for u-boot,env-size

[Christian Marangi]

On Wed, Jul 26, 2023 at 10:36:00AM -0600, Rob Herring wrote:
> On Mon, Jul 24, 2023 at 10:26:30AM +0200, Christian Marangi wrote:
> > Add support for u-boot,env-size new property.
> > 
> > Permit to declare a custom size of the U-Boot env that differs than the
> > partition size where the U-Boot env is located.
> > 
> > U-Boot env is validated by calculating the CRC32 on the entire env
> > and in some specific case, the env size might differ from the partition
> > size resulting in wrong CRC32 calculation than the expected one saved at
> > the start of the partition.
> 
> Why can't you just change the partition size? There is no size really 
> because it is just defined in DT.

Hi,
partition may also come by parser at runtime and are not only provided
by DT. One example is cmdlinepart where partition comes from cmd line or
smem-part from qcom smem.

We support this case by declaring these partition in DT and NVMEM
connects the dynamic partition to OF in DT (to reference and use the
NVMEM cell in other nodes)

> > 
> > This happens when U-Boot is compiled by hardcoding a specific env size
> > but the env is actually placed in a bigger partition, resulting in needing
> > to provide a custom value.
> 
> If u-boot is compiled that way, then shouldn't it have that size 
> contained within it? What happens when the DT doesn't match?
> 

When DT doesn't match the expected compiled size, the CRC32 validation
fail.
U-Boot store the env size crc32 in the first few bytes of the uboot env
partition. And we walidate that the saved crc32 actually match the
calculated one to make sure we have a not corrupted env size. If crc32
validation fail the uboot env nvmem module fails to probe.

> > 
> > Declaring this property, this value will be used for NVMEM size instead of
> > the mtd partition.
> > 
> > Add also an example to make it clear the scenario of mismatched
> > partition size and actual U-Boot env.
> 
> If we do have this, then perhaps there is a generic need for a data 
> size property.
> 

Can you give some example of this? I'm more than happy to follow
standard property instead of use custom ones.

> > 
> > Signed-off-by: Christian Marangi <ansuelsmth@xxxxxxxxx>
> > ---
> >  .../devicetree/bindings/nvmem/u-boot,env.yaml | 47 +++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> > 
> > diff --git a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> > index 36d97fb87865..3970725a2c57 100644
> > --- a/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> > +++ b/Documentation/devicetree/bindings/nvmem/u-boot,env.yaml
> > @@ -44,6 +44,24 @@ properties:
> >    reg:
> >      maxItems: 1
> >  
> > +  u-boot,env-size:
> > +    description: |
> > +      Permit to declare a custom size of the U-Boot env that differs than the
> > +      partition size where the U-Boot env is located.
> > +
> > +      U-Boot env is validated by calculating the CRC32 on the entire env
> > +      and in some specific case, the env size might differ from the partition
> > +      size resulting in wrong CRC32 calculation than the expected one saved at
> > +      the start of the partition.
> > +
> > +      This happens when U-Boot is compiled by hardcoding a specific env size
> > +      but the env is actually placed in a bigger partition, resulting in needing
> > +      to provide a custom value.
> > +
> > +      Declaring this property, this value will be used for NVMEM size instead of
> > +      the mtd partition.
> > +    $ref: /schemas/types.yaml#/definitions/uint32
> > +
> >    bootcmd:
> >      type: object
> >      description: Command to use for automatic booting
> > @@ -99,3 +117,32 @@ examples:
> >              };
> >          };
> >      };
> > +  - |
> > +    partitions {
> > +        compatible = "fixed-partitions";
> > +        #address-cells = <1>;
> > +        #size-cells = <1>;
> > +
> > +        partition@0 {
> > +            reg = <0x0 0xc80000>;
> > +            label = "qcadata";
> > +            read-only;
> > +        };
> > +
> > +        partition@c80000 {
> > +            label = "APPSBL";
> > +            reg = <0xc80000 0x500000>;
> > +            read-only;
> > +        };
> > +
> > +        partition@1180000 {
> > +            compatible = "u-boot,env";
> > +            reg = <0x1180000 0x80000>;
> > +
> > +            u-boot,env-size = <0x40000>;
> > +
> > +            mac1: ethaddr {
> > +                #nvmem-cell-cells = <1>;
> > +            };
> > +        };
> > +    };
> > -- 
> > 2.40.1
> > 

-- 
	Ansuel