Hi Tom,

Le 10/05/2012 00:56, Tom Warren a écrit :
Albert,

-----Original Message-----
From: Albert ARIBAUD [mailto:albert.u.b...@aribaud.net]
Sent: Friday, May 04, 2012 12:28 PM
To: Simon Glass
Cc: U-Boot Mailing List; Tom Warren
Subject: Re: [U-Boot] [PATCH v6 03/23] Add abs() macro to return absolute
value

Hi Simon,

Le 04/05/2012 20:25, Simon Glass a écrit :
This macro is generally useful to make it available in common.

Signed-off-by: Simon Glass<s...@chromium.org>
---
Changes in v3:
- Add new patch to put abs() in common.h

Changes in v6:
- Update x86emu and omap4 to use the abs() macro

   arch/arm/cpu/armv7/omap4/clocks.c       |    2 --
   drivers/bios_emulator/x86emu/prim_ops.c |    5 -----
   include/common.h                        |   13 +++++++++++++
   3 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/arch/arm/cpu/armv7/omap4/clocks.c
b/arch/arm/cpu/armv7/omap4/clocks.c
index e2189f7..ce3f59c 100644
--- a/arch/arm/cpu/armv7/omap4/clocks.c
+++ b/arch/arm/cpu/armv7/omap4/clocks.c
@@ -46,8 +46,6 @@
   #define puts(s)
   #endif

-#define abs(x) (((x)<   0) ? ((x)*-1) : (x))
-
   struct omap4_prcm_regs *const prcm = (struct omap4_prcm_regs
*)0x4A004100;

   const u32 sys_clk_array[8] = {
diff --git a/drivers/bios_emulator/x86emu/prim_ops.c
b/drivers/bios_emulator/x86emu/prim_ops.c
index 7553087..5f6c795 100644
--- a/drivers/bios_emulator/x86emu/prim_ops.c
+++ b/drivers/bios_emulator/x86emu/prim_ops.c
@@ -118,11 +118,6 @@ static u32 x86emu_parity_tab[8] =

   #define PARITY(x)   (((x86emu_parity_tab[(x) / 32]>>   ((x) % 32))&   1)
== 0)
   #define XOR2(x)          (((x) ^ ((x)>>1))&   0x1)
-/*----------------------------- Implementation
----------------------------*/ -int abs(int v) -{
-       return (v>0)?v:-v;
-}

   /*----------------------------- Implementation
----------------------------*/

diff --git a/include/common.h b/include/common.h index
74d9704..92eac2c 100644
--- a/include/common.h
+++ b/include/common.h
@@ -229,6 +229,19 @@ ulong timer_get_boot_us(void);
   #define MIN(x, y)  min(x, y)
   #define MAX(x, y)  max(x, y)

+/*
+ * Return the absolute value of a number. This handles unsigned ints,
+shorts
+ * and chars and returns a signed long.
+ */

What is the rationale for forcing a signed long return type? Such macros as
max, min, abs etc usually try to avoid any forced typing. Can't you get rid
of the ret variable and use __x directly as the return value?
+#define abs(x) ({                                      \
+               long ret;                               \
+               {                                       \
+                       typeof((x)) __x = (x);          \
+                       ret = (__x<   0) ? -__x : __x;       \
+               }                                       \
+               ret;                                    \
+       })
+
   #if defined(CONFIG_ENV_IS_EMBEDDED)
   #define TOTAL_MALLOC_LEN     CONFIG_SYS_MALLOC_LEN
   #elif ( ((CONFIG_ENV_ADDR+CONFIG_ENV_SIZE)<
CONFIG_SYS_MONITOR_BASE) || \

This is the last fix needed to get my pull request to compile w/o 
errors/warnings w/MAKEALL.

To get this unstuck, how about if I change the macro to:

#define abs(X)                          \
        ({ typeof (X) __x = (X);                \
                (__x<   0) ? -__x : __x; })

And put it locally in drivers/power/tps6586x.c (the only file that
uses it in that patchset). That way I get a clean build w/o the OMAP4
warnings of a double-defined abs(), and we can move everybody to use
abs() in include/common.h at a later date?

I don't have the final word on drivers/power :) but the best is you send out a patch and if no one rejects it, then it should go in.

Tom

Amicalement,
--
Albert.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to