[PATCH 0/3] rockchip: remove u-boot.rom generation and use binaries from simple-bin in simple-bin-image

[Simon Glass]

Hi Quentin,

On Thu, 20 Feb 2025 at 08:34, Quentin Schulz <quentin.schulz at cherry.de> wrote:
>
> Hi Simon,
>
> On 2/17/25 4:37 PM, Simon Glass wrote:
> > Hi Quentin,
> >
> > On Mon, 17 Feb 2025 at 08:37, Quentin Schulz <quentin.schulz at cherry.de> wrote:
> >>
> >> Hi Simon,
> >>
> >> On 2/15/25 2:17 PM, Simon Glass wrote:
> >>> Hi Quentin,
> >>>
> >>> On Thu, 13 Feb 2025 at 06:54, Simon Glass <sjg at chromium.org> wrote:
> >>>>
> >>>> Hi Quentin,
> >>>>
> >>>> On Tue, 11 Feb 2025 at 03:29, Quentin Schulz <foss+uboot at 0leil.net> wrote:
> >>>>>
> >>>>> This gets rid of u-boot.rom generation as that was used only on Rockchip
> >>>>> Chromebooks and their maintainer (Simon) seems to agree[1] that
> >>>>> u-boot-rockchip-spi.bin should do the job now so we don't need to
> >>>>> generate it anymore. This was agreed for RK3399 Chromebooks, I'm
> >>>>> attempting to do the same for RK3288 Chromebooks as well so we don't
> >>>>> have anything requiring that u-boot.rom anymore.
> >>>>>
> >>>>> Since HAS_ROM is only guarding this u-boot.rom generation, they are
> >>>>> removed too from the individual configs.
> >>>>>
> >>>>> Finally, this also fixes an issue reported by Naoki[2] where binman
> >>>>> would try to find symbols from proper to install in xPL binary only for
> >>>>> it to not find it as we do not generate (on Aarch64) the proper binary
> >>>>> in simple-bin-spi image node, only in simple-bin, so it cannot have
> >>>>> access to the symbol. As to why this triggered only when some seemingly
> >>>>> unrelated symbols are enabled, I do not know.
> >>>>>
> >>>>> The first two commits are cleanups. If they are controversial, they can
> >>>>> be dropped and I'll apply the same fixes for rockchip-u-boot.dtsi to
> >>>>> {rk3288,rk3399}-u-boot.dtsi binman nodes.
> >>>>>
> >>>>> Note that none of the patches were tested outside of a simple build
> >>>>> test.
> >>>>
> >>>> Are you able to run them through my lab?
> >>>>
> >>>>>
> >>>>> [1] https://lore.kernel.org/u-boot/CAFLszTh-SewFod8dEOF3+e-wCE1qFF0CyxxR8CbQwy3BRW3k6w@mail.gmail.com
> >>>>> [2] https://lore.kernel.org/u-boot/20250129132529.807031-3-naoki@radxa.com/
> >>>>>
> >>>>> Signed-off-by: Quentin Schulz <quentin.schulz at cherry.de>
> >>>>> ---
> >>>>> Quentin Schulz (3):
> >>>>>         rockchip: rk3399: do not generate u-boot.rom anymore
> >>>>>         rockchip: rk3288: do not generate u-boot.rom anymore
> >>>>>         rockchip: avoid rebuilding most binaries for u-boot-rockchip-spi.bin
> >>>>>
> >>>>>    arch/arm/dts/rk3288-u-boot.dtsi       | 24 ------------------------
> >>>>>    arch/arm/dts/rk3399-u-boot.dtsi       | 35 -----------------------------------
> >>>>>    arch/arm/dts/rockchip-u-boot.dtsi     | 10 ++++++++++
> >>>>>    arch/arm/mach-rockchip/rk3288/Kconfig |  5 -----
> >>>>>    arch/arm/mach-rockchip/rk3399/Kconfig |  2 --
> >>>>>    5 files changed, 10 insertions(+), 66 deletions(-)
> >>>>> ---
> >>>>> base-commit: 636fcc96c3d7e2b00c843e6da78ed3e9e3bdf4de
> >>>>> change-id: 20250211-has_rom-u-boot-rockchip-spi-bin-df31b06ad2f3
> >>>>>
> >>>>> Best regards,
> >>>>> --
> >>>>> Quentin Schulz <quentin.schulz at cherry.de>
> >>>>>
> >>>>
> >>>
> >>> I'm not sure if you have tried this yet.
> >>>
> >>
> >> No I haven't.
> >>
> >>> This series produces a SPI file but the size is not the right size
> >>> (4MB). How could we solve that?
> >>>
> >>
> >> Which board(s) do you have an issue with? What is supposed to be the
> >> right size?
> >
> > kevin and bob, both 4MB - see CONFIG_ROM_SIZE
> >
>
> CONFIG_ROM_SIZE is x86-specific and is empty for me.

OK

>
> Why does it need to be 4MiB? Should we really care if it's less (it's
> currently 2.1MiB)?
>
> Also, I believe it should be 8MiB? c.f. /binman/rom/size property in
> arch/arm/dts/rk3399-gru-u-boot.dtsi (which I forgot to remove).

OK

>
> So I guess the answer to "how could we solve that" is to add
> /binman/simple-bin-spi/size property to all Chromebooks, so in
> arch/arm/dts/rk3399-gru-u-boot.dtsi and
> arch/arm/dts/rk3288-veyron-u-boot.dtsi).

Yes, that should work.

Regards,
Simon