[U-Boot] [PATCH v6 03/23] Add abs() macro to return absolute value

[Tom Warren]

Albert,

> -----Original Message-----
> From: Albert ARIBAUD [mailto:albert.u.boot at 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<sjg at 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?

Tom
> 
> Amicalement,
> --
> Albert.
-- 
nvpublic