Re: [yocto-patches] [meta-rockchip PATCH 1/2] bsp: rkbin: rkbin-ddr: use glob pattern for finding DDR init

[Trevor Woerner <twoerner@gmail.com>]

On Thu 2024-11-28 @ 03:00:12 PM, Quentin Schulz wrote:
> Hi Trevor,
> 
> On 11/28/24 2:51 PM, Trevor Woerner wrote:
> > How about a variable to hold the version e.g. 1.18 (or whatever)?
> > 
> 
> Considering the filename has changed already for RK3588, I would suggest the
> whole filename?
> 
> Or two variables?

I think that would be great, thanks!

> 
> DDRBIN_VER:rk3588s ?= "v1.18"
> DDRBIN_FILE:rk3588s ?=
> "rk3588_ddr_lp4_2112MHz_lp5_2400MHz_${DDRBIN_VER}.bin"
> 
> So that in most cases you only need to update DDRBIN_VER but it would be
> possible to change also the whole filename. This could be useful if you want
> to use the eyescan one for some reason, e.g. rk3308 has many possible
> frequencies for DDR with different uarts as well.

Do you know what those eyescan frequencies/binaries are for?

> 
> > On Thu, Nov 28, 2024 at 5:23 AM Quentin Schulz <quentin.schulz@cherry.de>
> > wrote:
> > 
> > > Hi Trevor,
> > > 
> > > On 11/28/24 1:55 AM, Trevor Woerner wrote:
> > > > On Wed 2024-11-27 @ 04:12:58 PM, Quentin Schulz via
> > > lists.yoctoproject.org wrote:
> > > > > From: Quentin Schulz <quentin.schulz@cherry.de>
> > > > > 
> > > > > To avoid both having to modify this file every time the
> > > > > rockchip-rbin.inc SRCREV is bumped and also having to override it were
> > > > > a user to have to mix and match versions for different products, let's
> > > > > just use a glob pattern.
> > > > > 
> > > > > Note that the glob pattern should only match one path, which is the case
> > > > > right now.
> > > > 
> > > > I'm not overly fond of the glob method. It was only back in June of this
> > > > year that I pushed a patch to move away from globs to using full
> > > filenames
> > > > explicitly. The savings seem pretty thin compared to the potential
> > > surprise
> > > > factor when, one day, multiple files might be selected.
> > > > 
> > > > Do you feel strongly about switching back to the glob?
> > > > 
> > > 
> > > I feel strongly against the status quo :)
> > > 
> > > We can add a check that only one file is matched by the glob pattern if
> > > necessary and fail otherwise with bbfatal/bberror for example.
> > > 
> > > If we don't use globs, I need to override the do_deploy:<soc> to replace
> > > the path if I'm building a different version.
> > > 
> > > Here's what I needed to do on Scarthgap for example in my own layer:
> > > 
> > > """
> > > diff --git a/meta-bsp/recipes-bsp/rkbin/rockchip-rkbin_git.bbappend
> > > b/meta-bsp/recipes-bsp/rkbin/rockchip-rkbin_git.bbappend
> > > index
> > > 9d381a6afadf7dd66855f6635b5f1f053eecba15..299bb2ac476bf9d988925cbd983a66158a8bdf4c
> > > 
> > > 100644
> > > --- a/meta-bsp/recipes-bsp/rkbin/rockchip-rkbin_git.bbappend
> > > +++ b/meta-bsp/recipes-bsp/rkbin/rockchip-rkbin_git.bbappend
> > > @@ -1,5 +1,12 @@
> > >    FILESEXTRAPATHS:prepend := "${THISDIR}/${PN}:"
> > > 
> > > +# Remove once the following series is merged and backported:
> > > +#
> > > 
> > > https://lore.kernel.org/yocto-patches/20241127-rkbin-bump-v1-0-b90b6c04a88f@cherry.de/
> > > +SRCREV:tiger-haikou
> > > <https://lore.kernel.org/yocto-patches/20241127-rkbin-bump-v1-0-b90b6c04a88f@cherry.de/+SRCREV:tiger-haikou>
> > > = "7c35e21a8529b3758d1f051d1a5dc62aae934b2b"
> > > +SRCREV:jaguar = "7c35e21a8529b3758d1f051d1a5dc62aae934b2b"
> > > +LIC_FILES_CHKSUM:tiger-haikou =
> > > "file://LICENSE;md5=11e3673115959bf596feaaa6ea7ce9a5"
> > > +LIC_FILES_CHKSUM:jaguar =
> > > "file://LICENSE;md5=11e3673115959bf596feaaa6ea7ce9a5"
> > > +
> > >    # The files are created locally by running:
> > >    #   ./tools/ddrbin_tool tools/ddrbin_param.txt RKBIN
> > >    #
> > > @@ -24,3 +35,14 @@ do_deploy:append:rk3588s() {
> > >            fi
> > >        fi
> > >    }
> > > +
> > > +# Can be removed once the following patch is merged and backported
> > > +#
> > > 
> > > https://lore.kernel.org/yocto-patches/20241127-rkbin-bump-v1-1-b90b6c04a88f@cherry.de/
> > > +do_deploy:rk3588s(
> > > <https://lore.kernel.org/yocto-patches/20241127-rkbin-bump-v1-1-b90b6c04a88f@cherry.de/+do_deploy:rk3588s(>)
> > > {
> > > +    # Prebuilt TF-A
> > > +    install -m 644 ${S}/bin/rk35/rk3588_bl31_v*.elf
> > > ${DEPLOYDIR}/bl31-rk3588.elf
> > > +    # Prebuilt OPTEE-OS
> > > +    install -m 644 ${S}/bin/rk35/rk3588_bl32_v*.bin
> > > ${DEPLOYDIR}/tee-rk3588.bin
> > > +    # Prebuilt U-Boot TPL (DDR init)
> > > +    install -m 644
> > > ${S}/bin/rk35/rk3588_ddr_lp4_2112MHz_lp5_2400MHz_v*.bin
> > > ${DEPLOYDIR}/ddr-rk3588.bin
> > > +}
> > > """
> > > 
> > > The last part of the diff shouldn't be needed.
> > > 
> > > If you don't want a glob pattern, we could maybe use a variable to store
> > > the path to match so that it can be overridden in bbappends?
> > > 
> > > What do you think?
> > > 
> > > Cheers,
> > > Quentin
> > > 
> > 
>