Please pull u-boot-marvell/master

[Pali Rohár]

On Sunday 01 August 2021 10:33:33 Tom Rini wrote:
> On Sun, Aug 01, 2021 at 12:46:49PM +0200, Pali Rohár wrote:
> > On Sunday 01 August 2021 07:26:51 Stefan Roese wrote:
> > > On 01.08.21 05:28, Tom Rini wrote:
> > > > On Sat, Jul 31, 2021 at 12:04:01PM +0200, Stefan Roese wrote:
> > > > 
> > > > > Hi Tom,
> > > > > 
> > > > > please pull the next batch of Marvell MVEBU related patches. Here the
> > > > > summary log:
> > > > 
> > > > First off, I've applied the whole series to u-boot/master and pushed.
> > > > 
> > > > Second, I see from:
> > > > commit 5fce2875569d6e859443af7af3477c3aebfee383
> > > > Author: Pali Rohár <pali at kernel.org>
> > > > Date:   Fri Jul 23 11:14:27 2021 +0200
> > > > 
> > > >      SPL: Add support for specifying offset between header and image
> > > > 
> > > > That a number of boards are now doing:
> > > >              variscite_dart6ul: spl/u-boot-spl:all +144 spl/u-boot-spl:text +144
> > > >                 spl-u-boot-spl: add: 3/0, grow: 2/-1 bytes: 142/-4 (138)
> > > >                   function                                   old     new   delta
> > > >                   memmove                                      -      42     +42
> > > >                   spl_mmc_load                               320     356     +36
> > > >                   __aeabi_uidivmod                             -      24     +24
> > > >                   __aeabi_idivmod                              -      24     +24
> > > >                   spl_parse_image_header                      24      40     +16
> > > >                   board_init_r                               220     216      -4
> > > > 
> > > > Which I think is because we need to use do_div and so rather than '/' and '%'
> > > > directly in the code.  Thanks!
> > > 
> > > Pali, could you please take a look at this?
> > 
> > And what we can do here? 32-bit arm does not have 32-bit division
> > instruction, so it is needed to use some sort of *idiv* function.
> > 
> > do_div() is macro which is doing 64-bit division by using 32-bit C
> > operations '/' and '%', therefore it does not help with anything as this
> > code is doing 32-bit math (not 64-bit).
> > 
> > Moreover in do_div() implementation is already check that first passed
> > argument is of 64-bit type, so we cannot use it for 32-bit values.
> > 
> > Also note that in files which are touched by this commit are already
> > used 32-bit division operations via C '/' operator.
> > 
> > So I really do not know what is expected to do here...
> 
> Thanks for checking.  I saw block stuff and that typically does involve
> a 64bit value somewhere along the way.  So if the answer is:
> - There's no 64-bit math here, really.
> - There's no existing shift macros we can use instead (or that ends up
>   being larger!)
> - There's no existing shift macros we just need to import from the
>   kernel.
> 
> Then we're good.

Well, "offset" member in mentioned commit is defined as "u32 offset". So
it is not 64-bit for sure.

And about shift macros...
Problematic part is probably mmc->read_bl_len and stor_dev->blksz.
I guess that storage block size is always power of two. Or are are some
mmc / sata / usb / ... devices which have block / erase size which is
not power of two?

So these two members (read_bl_len and blksz) could stored as log2()
value and then code can use shifts instead of division.

But such change is big in U-Boot as it would touch whole U-Boot
codebase.

Maybe mmc and storage maintainers could decide if such thing is useful
and try to do it?

> 
> -- 
> Tom