Re: [PATCH] Use hex for bitstream_size as expected by u-boot

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 10 Nov 2023, Julien Grall wrote:
> On 10/11/2023 10:08, Julien Grall wrote:
> > On 10/11/2023 10:01, Michal Orzel wrote:
> > > Hi Julien,
> > > 
> > > On 10/11/2023 10:27, Julien Grall wrote:
> > > > 
> > > > 
> > > > Hi Stefano,
> > > > 
> > > > On 10/11/2023 01:44, Stefano Stabellini wrote:
> > > > > Reported-by: Brian Woods <brian@xxxxxxxxx>
> > > > > Signed-off-by: Stefano Stabellini <stefano.stabellini@xxxxxxx>
> > > > > 
> > > > > diff --git a/scripts/uboot-script-gen b/scripts/uboot-script-gen
> > > > > index b284887..6e52da5 100755
> > > > > --- a/scripts/uboot-script-gen
> > > > > +++ b/scripts/uboot-script-gen
> > > > > @@ -790,7 +790,7 @@ bitstream_load_and_config()
> > > > >            check_file_type "$BITSTREAM" "BIT data"
> > > > >            bitstream_addr=$memaddr
> > > > >            load_file $BITSTREAM "fpga_bitstream"
> > > > > -        bitstream_size=$filesize
> > > > > +        bitstream_size=`printf "0x%X\n" $filesize`
> > > > 
> > > > Looking at [1], there is no indication that the size parameter for "fpga
> > > > load" should be hexadecimal. At the contrary, all the example I have
> > > > found seems to use $filesize.
> > > U-boot expects size to be passed in hex format. You can see it here:
> > > https://github.com/u-boot/u-boot/blob/master/cmd/fpga.c#L60C20-L60C27
> > > 
> > > Also, AFAICT $filesize var that gets updated after image load (e.g. tftpb)
> > > is in hex format.
> > 
> > Hmmm, now that you are saying this... I vaguely recall we had some issues in
> > the past with the size. The format was not consistent across U-boot.
> > 
> > Maybe this is the same problem here?
> 
> I had a chat with Michal on Matrix. 'filesize' is a variable internal to the
> image-builder rather than U-boot. Hence the confusion.
> 
> The variable will be set in decimal. So it needs to be converted to
> hexadecimal. So the code seems to be correct.
> 
> That said, I think some clarifications is needed in the commit message to help
> understanding the code and at least giving a hint whether this code was ever
> tested (or this was introduced by a follow-up change).

Makes sense, I added an explanation on commit. Passing the parameter as
hex with 0x should make it work on both old and new versions of u-boot.