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@kernel.org Date: Fri Jul 23 11:14:27 2021 +0200
SPL: Add support for specifying offset between header and imageThat 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