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 <p...@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