Hi Marek,

> On 04/27/2018 06:15 PM, Lukasz Majewski wrote:
> > Hi Marek,
> > 
> > Thanks for prompt feedback.
> >   
> >> On 04/27/2018 04:51 PM, Lukasz Majewski wrote:  
> >>> This commit provides the ability to generate u-boot environment(s)
> >>> as images, which afterwards can be used to produce image (with
> >>> wic) for flashing (eMMC or SPI-NOR).
> >>>
> >>> This change removes the need to run "env default" during
> >>> production phase, as proper environment (including redundant one)
> >>> is already stored on persistent memory (the CRC is also correct).
> >>>
> >>> Signed-off-by: Lukasz Majewski <lukma@denx.de>    
> >>
> >> If your default env is correct, why do you need this ?   
> > 
> > Some users wants to have the working board (i.e. without any
> > warnings that CRC for envs is bad) after flashing the SPI-NOR/eMMC  
> 
> And the board is not working when it tells you your env is not
> populated? 

It complains that CRC is wrong, which is true as without this patch
(and some wic adjustments) we don't have proper envs in expected
non-volatile area.

> Then maybe the board port is broken.

Then it proceeds with default envs - and boots.

> 
> >> I can see some
> >> use with non-default env, but then that can be wrapped into a
> >> separate recipe.  
> > 
> > I can add this functionality as a separate recipe. This is v1, so
> > ideas are welcome.  
> 
> I think that only makes sense.

Ok.

> 
> >>> ---
> >>> This patch depends on "u-boot: Upgrade to 2018.03 release"
> >>> https://patchwork.openembedded.org/patch/149998/
> >>> ---
> >>>  meta/recipes-bsp/u-boot/u-boot.inc | 35
> >>> +++++++++++++++++++++++++++++++++++ 1 file changed, 35
> >>> insertions(+)
> >>>
> >>> diff --git a/meta/recipes-bsp/u-boot/u-boot.inc
> >>> b/meta/recipes-bsp/u-boot/u-boot.inc index
> >>> c2bcf99840.cdd7703db9ea580308d4ebcd6d0ae3209b761daa.2796e503cf
> >>> 100644 --- a/meta/recipes-bsp/u-boot/u-boot.inc +++
> >>> b/meta/recipes-bsp/u-boot/u-boot.inc @@ -305,3 +305,38 @@
> >>> do_deploy () { }
> >>>  
> >>>  addtask deploy before do_build after do_compile
> >>> +
> >>> +# Create new rules to extract default envs
> >>> +UBOOT_ENVS_DEFAULT ?= "uboot-envs-default"
> >>> +DEFAULT_ENVS ?= "u-boot-env-default.txt"
> >>> +DEFAULT_ENVS_SIZE ?= "65536"
> >>> +
> >>> +# Generate default environment
> >>> +do_gen_default_envs[doc] = "Generate image with default U-Boot
> >>> environment(s)" +do_gen_default_envs () {
> >>> +	${B}/source/scripts/get_default_envs.sh ${B} >
> >>> ${B}/${DEFAULT_ENVS} +
> >>> +	# Generate env image
> >>> +	${B}/tools/mkenvimage -s ${DEFAULT_ENVS_SIZE} -o
> >>> ${B}/${UBOOT_ENVS_DEFAULT} ${B}/${DEFAULT_ENVS}    
> >>
> >> Does this actually work during cross build , when mkenvimage
> >> architecture is different than host architecture ?  
> > 
> > Yes. This was tested.  
> 
> How can it work if this binary cannot be executed ? :)

Could you be more specific here?

I do follow normal u-boot recipe build. If mkenvimage is build in a
wrong way (as other tools) - then I suppose that the whole u-boot recipe
is broken.

The get_default_envs.sh script is prepared to use either native or
cross-compiled set of binutils (that thought I had in mind when I said
that it was tested).

> 
> >>> +	# Generate redundant env image
> >>> +	${B}/tools/mkenvimage -r -s ${DEFAULT_ENVS_SIZE} -o
> >>> ${B}/${UBOOT_ENVS_DEFAULT}_r ${B}/${DEFAULT_ENVS}    
> >>
> >> Is redundant env always needed on all systems ?  
> > 
> > No, they are not. However, it shall not be a problem if it is build
> > anyway - user can adjust wic to only put primary env image.  
> 
> So many wasted CPU cycles for a feature barely anyone needs. Nope :)

I will try to craft some OE magic to make it not build when not
needed... :-)

> 
> >>> +	rm ${B}/${DEFAULT_ENVS}
> >>> +}
> >>> +
> >>> +addtask gen_default_envs before do_deploy_default_envs after
> >>> do_compile +
> >>> +# Deploy default environment
> >>> +do_deploy_default_envs[doc] = "Copy images with default U-Boot
> >>> environment to deployment directory" +do_deploy_default_envs () {
> >>> +	install -d ${DEPLOYDIR}
> >>> +
> >>> +	install ${B}/${UBOOT_ENVS_DEFAULT}
> >>> ${DEPLOYDIR}/${UBOOT_ENVS_DEFAULT}
> >>> +	install ${B}/${UBOOT_ENVS_DEFAULT}_r
> >>> ${DEPLOYDIR}/${UBOOT_ENVS_DEFAULT}_r    
> >>
> >> Does this work with multiple machines   
> > 
> > Unfortunately not.
> > 
> > For multiple machines one needs to add ${target}, which would add
> > machine name into path:
> > 
> > target=${MACHINE}_defconfig
> > 
> > and change ${B} -> ${B}/${target}
> > 
> > 
> > I suppose, that I would need to adjust this script to be similar to
> > do_install() and do_compile() in this matter.  
> 
> Yep

Ok.

> 
> >> or will it overwrite the
> >> deployed image ?  
> > 
> > As it is now - it will overwrite the image.  
> Yep
> 

Ok.


Best regards,

Lukasz Majewski

--

DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de