[U-Boot] [PATCH 1/1] efi_loader: replace efi_div10 by div64_u64

We should use the existing 64bit division instead of reinventing the wheel.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- The patch is based on v2017.11-rc1 with no other patches applied.
Applying it will require rebasing [PATCH 1/1] efi_loader: provide function comments for boot services https://patchwork.ozlabs.org/patch/817010/
But we have an open issue with the second patch anyway. --- lib/efi_loader/efi_boottime.c | 36 ++---------------------------------- 1 file changed, 2 insertions(+), 34 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index 9e741c3cf3..d63c66dd45 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -16,6 +16,7 @@ #include <bootm.h> #include <inttypes.h> #include <watchdog.h> +#include <linux/math64.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -128,39 +129,6 @@ const char *__efi_nesting_dec(void) return indent_string(--nesting_level); }
-/* Low 32 bit */ -#define EFI_LOW32(a) (a & 0xFFFFFFFFULL) -/* High 32 bit */ -#define EFI_HIGH32(a) (a >> 32) - -/* - * 64bit division by 10 implemented as multiplication by 1 / 10 - * - * Decimals of one tenth: 0x1 / 0xA = 0x0.19999... - */ -#define EFI_TENTH 0x199999999999999A -static u64 efi_div10(u64 a) -{ - u64 prod; - u64 rem; - u64 ret; - - ret = EFI_HIGH32(a) * EFI_HIGH32(EFI_TENTH); - prod = EFI_HIGH32(a) * EFI_LOW32(EFI_TENTH); - rem = EFI_LOW32(prod); - ret += EFI_HIGH32(prod); - prod = EFI_LOW32(a) * EFI_HIGH32(EFI_TENTH); - rem += EFI_LOW32(prod); - ret += EFI_HIGH32(prod); - prod = EFI_LOW32(a) * EFI_LOW32(EFI_TENTH); - rem += EFI_HIGH32(prod); - ret += EFI_HIGH32(rem); - /* Round to nearest integer */ - if (rem >= (1 << 31)) - ++ret; - return ret; -} - void efi_signal_event(struct efi_event *event) { if (event->notify_function) { @@ -364,7 +332,7 @@ efi_status_t efi_set_timer(struct efi_event *event, enum efi_timer_delay type, * The parameter defines a multiple of 100ns. * We use multiples of 1000ns. So divide by 10. */ - trigger_time = efi_div10(trigger_time); + trigger_time = div64_u64(trigger_time, 10);
for (i = 0; i < ARRAY_SIZE(efi_events); ++i) { if (event != &efi_events[i])

On 03.10.17 05:17, Heinrich Schuchardt wrote:
We should use the existing 64bit division instead of reinventing the wheel.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
This patch increases the code size by ~200 bytes on armv7 with gcc7 for me. I'm not sure about the runtime - it's probably much slower too.
I do agree however that lib/efi_loader is not necessarily the right place to put base math things.
How about we just rename the efi_div10 to div64_10 and move it to lib/div64.c?
Alex

We should use the existing 64bit division instead of reinventing the wheel.
Cc: Alexander Graf agraf@suse.de Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- v2 replace efi_div10 by do_div as suggested by Alex v1 replace efi_div10 by div_u64 --- lib/efi_loader/efi_boottime.c | 36 ++---------------------------------- 1 file changed, 2 insertions(+), 34 deletions(-)
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c index df75dd9032..66ce92f654 100644 --- a/lib/efi_loader/efi_boottime.c +++ b/lib/efi_loader/efi_boottime.c @@ -7,6 +7,7 @@ */
#include <common.h> +#include <div64.h> #include <efi_loader.h> #include <environment.h> #include <malloc.h> @@ -128,39 +129,6 @@ const char *__efi_nesting_dec(void) return indent_string(--nesting_level); }
-/* Low 32 bit */ -#define EFI_LOW32(a) (a & 0xFFFFFFFFULL) -/* High 32 bit */ -#define EFI_HIGH32(a) (a >> 32) - -/* - * 64bit division by 10 implemented as multiplication by 1 / 10 - * - * Decimals of one tenth: 0x1 / 0xA = 0x0.19999... - */ -#define EFI_TENTH 0x199999999999999A -static u64 efi_div10(u64 a) -{ - u64 prod; - u64 rem; - u64 ret; - - ret = EFI_HIGH32(a) * EFI_HIGH32(EFI_TENTH); - prod = EFI_HIGH32(a) * EFI_LOW32(EFI_TENTH); - rem = EFI_LOW32(prod); - ret += EFI_HIGH32(prod); - prod = EFI_LOW32(a) * EFI_HIGH32(EFI_TENTH); - rem += EFI_LOW32(prod); - ret += EFI_HIGH32(prod); - prod = EFI_LOW32(a) * EFI_LOW32(EFI_TENTH); - rem += EFI_HIGH32(prod); - ret += EFI_HIGH32(rem); - /* Round to nearest integer */ - if (rem >= (1 << 31)) - ++ret; - return ret; -} - /* * Queue an EFI event. * @@ -523,7 +491,7 @@ efi_status_t efi_set_timer(struct efi_event *event, enum efi_timer_delay type, * The parameter defines a multiple of 100ns. * We use multiples of 1000ns. So divide by 10. */ - trigger_time = efi_div10(trigger_time); + do_div(trigger_time, 10);
for (i = 0; i < ARRAY_SIZE(efi_events); ++i) { if (event != &efi_events[i])

On 05.10.17 16:14, Heinrich Schuchardt wrote:
We should use the existing 64bit division instead of reinventing the wheel.
Cc: Alexander Graf agraf@suse.de Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
Awesome, this version actually even *reduces* code size :).
Alex
participants (2)
-
Alexander Graf
-
Heinrich Schuchardt