Re: [yocto-patches] [meta-rockchip][PATCH v2 1/2] enable stored U-Boot environment

[Trevor Woerner <twoerner@gmail.com>]

Gosh darn! Yes you're right. I kept saying WORKDIR over and over but I meant
DEPLOYDIR. I must have been distracted or something because in my mind, the
whole time, I was thinking $TMPDIR/deploy/images/$MACHINE but I kept writing
WORKDIR for some unknown reason.

On Thu 2024-04-25 @ 02:21:57 PM, Quentin Schulz via lists.yoctoproject.org wrote:
> Hi Trevor,
> 
> On 4/24/24 19:43, Trevor Woerner via lists.yoctoproject.org wrote:
> > On Thu 2024-04-18 @ 10:43:10 AM, Quentin Schulz wrote:
> > > Hi Trevor,
> > > 
> > > On 4/17/24 16:19, Trevor Woerner via lists.yoctoproject.org wrote:
> > > [...]
> > > > > 
> > > > > > +}
> > > > > > +
> > > > > > +do_deploy:append:rk-u-boot-env() {
> > > > > > +	UBOOT_ENV_SIZE="$(cat ${B}/.config | grep "^CONFIG_ENV_SIZE=" | cut
> > > > > -d'=' -f2)"
> > > > > > +	mkenvimage -s ${UBOOT_ENV_SIZE} ${B}/u-boot-initial-env -o
> > > > > ${WORKDIR}/u-boot.env
> > > > > > +
> > > > > > +	install -d ${DEPLOYDIR}
> > > > > > +	install -m 0644 ${WORKDIR}/u-boot.env ${DEPLOYDIR}
> > > > > Just install at the top and then write the file directly to DEPLOYDIR? Or,
> > > > > actually, create the file in do_compile if possible?
> > > > 
> > > > But I want this file handled correctly by do_deploy when sstate is being used?
> > > > So don't I need to do this in the do_deploy task?
> > > > 
> > > 
> > > Not sure to follow what's the concern about sstate? do_deploy sstate
> > > monitors everything in DEPLOYDIR so everything you install in there shall be
> > > under sstate?
> > > 
> > > For me do_deploy is for moving a file from one location to another, in
> > > DEPLOYDIR, nothing else. Everything that involves compilation/configuration
> > > is in do_compile/do_configure, so it's a bit odd to me to see a call to
> > > mkenvimage in do_deploy.
> > > 
> > > I guess you could have an issue with sstate if you use WORKDIR for
> > > u-boot.env since this wouldn't be under do_compile's sstate? But then just
> > > use ${B} for that instead?
> > > 
> > > Can you tell me a bit more about your concern here, I'm not getting the big
> > > picture yet :/
> > 
> > Firstly I want these files handled correctly by the oe-core U-Boot packaging
> > so they get included in the image/sstate correctly.
> > 
> 
> Using WORKDIR as a way to pass files between tasks is a terrible idea,
> WORKDIR isn't properly cleaned. However... this is an issue in the
> "upstream" U-Boot recipe, not something you can deal with in your bbappend,
> so let's ignore this issue here.

Yep, agreed.

> I'm also not entirely sure how the sstate works for do_compile... Couldn't
> find any sstate-indirs and sstate-outdirs for do_compile/do_install with
> bitbake-getvar or a quick check with grep. Do you have any idea how/where
> this is handled?
> 
> > Second, I'm pretty sure that anything you want inserted into a wic image (i.e.
> > all the "--sourceparams=..." that are specified in the wks file) need to be placed in
> > ${WORKDIR} for wic to find them when assembling the image. I might be wrong
> > about this, or there might be other directories that are searched, but I do
> > know that putting these artifacts in ${WORKDIR} will get them included in the
> > wic file as expected.
> > 
> 
> I hope you're wrong about this.

Yep entirely. I was totally thinking DEPLOYDIR the whole time, but writing
WORKDIR.

> wic is run as part of the image recipe. It
> should therefore not assume anything about the location of WORKDIR of the
> u-boot package recipe. The recommended way for exchanging data between
> recipes is to use either SYSROOT_DIRS (and DEPENDS) or DEPLOYDIR (and
> DEPLOY_DIR_IMAGE from the other recipe; + do_task[depends] +=
> "recipea:do_deploy"), so I assume this is what's used by wic.
> 
> The `wic create` command called in do_image_wic (IMAGE_CMD:wic) seems to be
> trying to get data from three directories: IMAGE_ROOTFS, DEPLOY_DIR_IMAGE,
> STAGING_DATADIR (and BUILDDIR passed as an environment variable).
> 
> U-Boot upstream recipe takes fw_env.config from WORKDIR and puts it into
> DEPLOYDIR as part of the do_deploy task. Therefore, we need to put it there
> (WORKDIR) now, so that we don't mess with the current implementation of
> U-Boot recipe.
> 
> However I don't think there's a reason for putting u-boot.env into the
> WORKDIR before installing it into DEPLOYDIR.

Sounds good.

> > Thirdly, the default U-Boot environment is placed in "u-boot-initial-env" (a
> > text file). Later "u-boot-initial-env" is converted into a U-Boot environment
> > file (i.e. apparently it has a checksum prepended to it) via `mkenvimage`.
> > If all of that takes place in do_compile(), then the user will never have an
> > opportunity to inject any variables into the U-Boot environment before it is
> > converted to its binary form. So by doing this operation in do_deploy(),
> > it gives the user an opportunity to inject things between the creation of
> > "u-boot-initial-env" in do_compile(), and it's conversion to its binary form
> > (i.e. the form that is including into the wic image) in do_deploy().
> > 
> > I've had rauc working with my rockchip boards for about 5-6 months now, and I
> > know when I put that together initially I wanted to inject extra variables
> > during the build. I'll have to revisit whether or not I still need to do that,
> > but even if I don't, doesn't mean it wouldn't be a nice feature for a user.
> > 
> 
> Agreed that's a nice thing to have.
> 
> We could use a do_compile postfunc for the mkenvimage part though. And then
> you could always do_compile:append to modify u-boot-initial-env file before
> your postfunc to create the mkenvimage runs.

Okay.

> We could also simply have another variable called e.g.
> UBOOT_INITIAL_ENV_EXTRA_VARS whose content is added at the end of
> fw_env.config in the upstream recipe (or in your bbappend before calling
> mkenvimage). That would allow to extend it.
> 
> I think it'd make sense to add support for creating the u-boot env "fs" (via
> mkenvimage) to the upstream recipe as I see this as something people could
> reuse on other vendors?

There was something else strange that I think I noticed in the upstream
recipe. If I'm not mistaken, you can do an environment or you could do a
script, but not both. I'll have to double-check that and maybe take a stab at
fixing that as well (if I'm correct in my reading of the recipe).

> Thanks for taking the time to list the reasons, I see better what you're
> attempting to do in this series now :)
> 
> Cheers,
> Quentin
> 
> 
> -=-=-=-=-=-=-=-=-=-=-=-
> Links: You receive all messages sent to this group.
> View/Reply Online (#63): https://lists.yoctoproject.org/g/yocto-patches/message/63
> Mute This Topic: https://lists.yoctoproject.org/mt/105454848/900817
> Group Owner: yocto-patches+owner@lists.yoctoproject.org
> Unsubscribe: https://lists.yoctoproject.org/g/yocto-patches/leave/13168745/900817/63955952/xyzzy [twoerner@gmail.com]
> -=-=-=-=-=-=-=-=-=-=-=-
> 
>