[PATCH v4 08/10] rockchip: binman: Use the skip-at-start prop in simple-bin image

[Simon Glass]

Hi Quentin,

On Mon, 14 Apr 2025 at 09:10, Quentin Schulz <quentin.schulz at cherry.de> wrote:
>
> Hi Simon,
>
> On 4/9/25 4:30 PM, Simon Glass wrote:
> > Hi Quentin,
> >
> > On Wed, 9 Apr 2025 at 07:35, Quentin Schulz <quentin.schulz at cherry.de> wrote:
> >>
> >> Hi Simon,
> >>
> >> On 4/9/25 3:33 PM, Simon Glass wrote:
> >>> Hi Quentin,
> >>>
> >>> On Wed, 9 Apr 2025 at 07:32, Quentin Schulz <quentin.schulz at cherry.de> wrote:
> >>>>
> >>>> Hi Simon,
> >>>>
> >>>> On 4/9/25 3:22 PM, Simon Glass wrote:
> >>>>> Hi Quentin,
> >>>>>
> >>>>> On Wed, 9 Apr 2025 at 04:57, Quentin Schulz <quentin.schulz at cherry.de> wrote:
> >>>>>>
> >>>>>> Hi Jonas, Simon,
> >>>>>>
> >>>>>> On 3/29/25 4:06 PM, Jonas Karlman wrote:
> >>>>>>> From: Simon Glass <sjg at chromium.org>
> >>>>>>>
> >>>>>>> The simple-bin image is normally written to MMC media at block 64, which
> >>>>>>> is a 32K offset from start of storage media.
> >>>>>>>
> >>>>>>> Set the skip-at-start property to 0x8000 (32 KiB) so that fdtmap and
> >>>>>>> other embedded binman symbols in the output binary is referencing image
> >>>>>>> offsets correctly.
> >>>>>>>
> >>>>>>
> >>>>>> Shouldn't we have this commit BEFORE we add the `fdtmap` node since we
> >>>>>> know it's wrong before this commit?
> >>>>>>
> >>>>>>> Signed-off-by: Simon Glass <sjg at chromium.org>
> >>>>>>> Signed-off-by: Jonas Karlman <jonas at kwiboo.se>
> >>>>>>> ---
> >>>>>>> Changes in v4:
> >>>>>>> - Drop defconfig changes
> >>>>>>> - Split from "VBE serial part H: Implement VBE on Rockchip RK3399"
> >>>>>>>
> >>>>>>> Changes in v2:
> >>>>>>> - Move this patch to the end of the series
> >>>>>>> - Drop 0x8000 offset for SPI
> >>>>>>> ---
> >>>>>>>      arch/arm/dts/rockchip-u-boot.dtsi | 3 ++-
> >>>>>>>      1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>>>>
> >>>>>>> diff --git a/arch/arm/dts/rockchip-u-boot.dtsi b/arch/arm/dts/rockchip-u-boot.dtsi
> >>>>>>> index fb38b7b80c43..65b81bf58626 100644
> >>>>>>> --- a/arch/arm/dts/rockchip-u-boot.dtsi
> >>>>>>> +++ b/arch/arm/dts/rockchip-u-boot.dtsi
> >>>>>>> @@ -154,6 +154,7 @@
> >>>>>>>          simple-bin {
> >>>>>>>                  filename = "u-boot-rockchip.bin";
> >>>>>>>                  pad-byte = <0xff>;
> >>>>>>> +             skip-at-start = <0x8000>;
> >>>>>>>
> >>>>>>>                  mkimage {
> >>>>>>>                          filename = "idbloader.img";
> >>>>>>> @@ -178,7 +179,7 @@
> >>>>>>>      #else
> >>>>>>>                  u-boot-img {
> >>>>>>>      #endif
> >>>>>>> -                     offset = <CONFIG_SPL_PAD_TO>;
> >>>>>>> +                     offset = <(CONFIG_SPL_PAD_TO + 0x8000)>;
> >>>>>>
> >>>>>> This is confusing. The documentation states:
> >>>>>>
> >>>>>> """
> >>>>>> offset:
> >>>>>>         This sets the offset of an entry within the image or section containing
> >>>>>>         it.
> >>>>>> """
> >>>>>>
> >>>>>> My understanding is that it should be relative to the beginning of the
> >>>>>> image but this now needs the knowledge of where it will be stored on the
> >>>>>> MMC device (via the value in skip-at-start).
> >>>>>>
> >>>>>> Why is skip-at-start automatically deducted from offset?
> >>>>>
> >>>>> This is how binman works[1]. We are trying to use the feature designs
> >>>>
> >>>> Why is it deducted?
> >>>
> >>> Are you asking why skip-at-start is deducted from the offset?
> >>>
> >>
> >> Yes
> >
> > It is confusing, unfortunately.
> >
> > When you use an offset (say 0x78000) then normally the entry will
> > start at that offset in the image.
> >
> > When you use skip-at-start 0x8000, its value is added to all top-level
> > offsets in the image, so the offset becomes 0x80000
> >
> > BUT the image built by binman does not contain the first 0x8000 bytes.
> > It is expected that the image is written to offset 0x8000 so that the
> > offsets will be correct when used within U-Boot itself.
> >
>
> I would assume all offsets are relative to the beginning of the image in
> the binary? Adding skip-at-start doesn't add 0x8000 bytes at the
> beginning of the binary file, so why would the offsets need to be modified?
>
> Also, while 0x8000 is the typical address the image can be flashed, it
> is not necessarily where it will be as the BootROM tries a few other
> offsets if it cannot find one at 32KiB offset in the storage medium.
> This seems to me to be a case of "somewhat helps in one case, but makes
> things more confusing in others"? Will we need different offsets
> depending on where the FIT is flashed? What happens for A/B updates then?
>
> I understand that we currently essentially have skip-at-start = 0; and
> that it is bad because it doesn't reflect the actual address, but how is
> that worse than hardcoding a different offset?

It's probably best that we discuss this on a call. You could also give
it a try, e.g. try accessing data in the U-Boot image from SPL and see
how it works for you.

Regards,
Simon