[PATCH v3 3/3] defconfig: rockpro64: Enable SF EFI var store

[Shantur Rathore]

Hi Simon,

On Sun, Dec 3, 2023 at 5:44 PM Simon Glass <sjg at chromium.org> wrote:
>
> Hi Shantur,
>
> On Sat, 2 Dec 2023 at 16:12, Shantur Rathore <i at shantur.com> wrote:
> >
> > Hi Simon,
> >
> > On Fri, Dec 1, 2023 at 6:44 PM Simon Glass <sjg at chromium.org> wrote:
> > >
> > > Hi Shantur,
> > >
> > > On Sun, 26 Nov 2023 at 15:09, Shantur Rathore <i at shantur.com> wrote:
> > > >
> > > > RockPro64 uses SPI Flash for storing env, also use it store
> > > > EFI variables.
> > > >
> > > > Signed-off-by: Shantur Rathore <i at shantur.com>
> > > > ---
> > > >
> > > >  configs/rockpro64-rk3399_defconfig | 2 ++
> > > >  1 file changed, 2 insertions(+)
> > > >
> > > > diff --git a/configs/rockpro64-rk3399_defconfig b/configs/rockpro64-rk3399_defconfig
> > > > index 4cd6b76665..f550f2ae43 100644
> > > > --- a/configs/rockpro64-rk3399_defconfig
> > > > +++ b/configs/rockpro64-rk3399_defconfig
> > > > @@ -8,6 +8,8 @@ CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
> > > >  CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0x300000
> > > >  CONFIG_ENV_SIZE=0x8000
> > > >  CONFIG_ENV_OFFSET=0x3F8000
> > > > +CONFIG_EFI_VARIABLE_SF_STORE=y
> > > > +CONFIG_EFI_VARIABLE_SF_OFFSET=0x7D0000
> > >
> > > Can we use this offset in binman when creating the SPI-flash image?
> >
> > Unless I missed something, binman is used when something is bundled
> > with a u-boot image.
> > This is similar to ENV_OFFSET letting u-boot know where it can write
> > the EFI variables.
> > I didn't see ENV_OFFSET referred in rk3399-u-boot.dtsi or
> > rk3399-rockpro64-u-boot.dtsi
> >
> > Please correct me if I am wrong.
>
> Well, this is what I see in arch/arm/dts/rk3399-u-boot.dtsi :
>
> #if defined(CONFIG_ROCKCHIP_SPI_IMAGE) && defined(CONFIG_HAS_ROM)
> &binman {
>    multiple-images;
>    rom {
>       filename = "u-boot.rom";
>       size = <0x400000>;
>       pad-byte = <0xff>;
>
>       mkimage {
>          args = "-n rk3399 -T rkspi";
>          u-boot-spl {
>          };
>       };
>       u-boot-img {
>          offset = <0x40000>;
>       };
>       u-boot {
>          offset = <0x300000>;
>       };
>       fdtmap {
>       };
>    };
> };
> #endif /* CONFIG_ROCKCHIP_SPI_IMAGE && CONFIG_HAS_ROM */
>
> So it looks like 0x3f8000 fits in there somewhere.
>

Thanks for clarifying. Now I see how ENV space is declared as used by u-boot.


> In fact, I wonder if we should add something like:
>
> u-boot-env {
>    offset = <CONFIG_ENV_OFFSET>;
> };
>
> so the environment gets filled into the SPI flash when we write the image?
>

Do we want to reset the environment to default on every u-boot update?
I believe this would overwrite the environment space isn't it.

> I suppose we could also add the EFI variable store, although that
> would require adding a new entry type to binman.
>

I am not sure about this but surely EFI vars can be moved to a location
just after the u-boot environment and increase the u-boot image size.

Would you still need an entry in dts?