public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] [PATCH v6 03/23] Add abs() macro to return absolute value
@ 2012-05-04 18:25 Simon Glass
  2012-05-04 19:27 ` Albert ARIBAUD
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2012-05-04 18:25 UTC (permalink / raw)
  To: u-boot

This macro is generally useful to make it available in common.

Signed-off-by: Simon Glass <sjg@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.
+ */
+#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) || \
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH v6 03/23] Add abs() macro to return absolute value
  2012-05-04 18:25 [U-Boot] [PATCH v6 03/23] Add abs() macro to return absolute value Simon Glass
@ 2012-05-04 19:27 ` Albert ARIBAUD
  2012-05-07 17:53   ` Tom Warren
                     ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Albert ARIBAUD @ 2012-05-04 19:27 UTC (permalink / raw)
  To: u-boot

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@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) || \


Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH v6 03/23] Add abs() macro to return absolute value
  2012-05-04 19:27 ` Albert ARIBAUD
@ 2012-05-07 17:53   ` Tom Warren
  2012-05-09 22:56   ` Tom Warren
  2012-05-10 19:37   ` Simon Glass
  2 siblings, 0 replies; 6+ messages in thread
From: Tom Warren @ 2012-05-07 17:53 UTC (permalink / raw)
  To: u-boot

Simon,

> -----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@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 patch is all that's keeping me from submitting a new pull request for u-boot-tegra/master to Albert. I've fixed the other 2 build errors w/your updated patches.

Thanks,

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH v6 03/23] Add abs() macro to return absolute value
  2012-05-04 19:27 ` Albert ARIBAUD
  2012-05-07 17:53   ` Tom Warren
@ 2012-05-09 22:56   ` Tom Warren
  2012-05-10 19:33     ` Albert ARIBAUD
  2012-05-10 19:37   ` Simon Glass
  2 siblings, 1 reply; 6+ messages in thread
From: Tom Warren @ 2012-05-09 22:56 UTC (permalink / raw)
  To: u-boot

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@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@a later date?

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH v6 03/23] Add abs() macro to return absolute value
  2012-05-09 22:56   ` Tom Warren
@ 2012-05-10 19:33     ` Albert ARIBAUD
  0 siblings, 0 replies; 6+ messages in thread
From: Albert ARIBAUD @ 2012-05-10 19:33 UTC (permalink / raw)
  To: u-boot

Hi Tom,

Le 10/05/2012 00:56, Tom Warren a ?crit :
> 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@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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [U-Boot] [PATCH v6 03/23] Add abs() macro to return absolute value
  2012-05-04 19:27 ` Albert ARIBAUD
  2012-05-07 17:53   ` Tom Warren
  2012-05-09 22:56   ` Tom Warren
@ 2012-05-10 19:37   ` Simon Glass
  2 siblings, 0 replies; 6+ messages in thread
From: Simon Glass @ 2012-05-10 19:37 UTC (permalink / raw)
  To: u-boot

Hi Albert,

On Fri, May 4, 2012 at 12:27 PM, Albert ARIBAUD
<albert.u.boot@aribaud.net>wrote:

> 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@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?


Sorry I am not sure about that, not my code... I will submit a patch with a
really simply implementation.



>
> +#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)
>> || \
>>
>
>
> Amicalement,
> --
> Albert.
>

Regards,
Simon

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2012-05-10 19:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-05-04 18:25 [U-Boot] [PATCH v6 03/23] Add abs() macro to return absolute value Simon Glass
2012-05-04 19:27 ` Albert ARIBAUD
2012-05-07 17:53   ` Tom Warren
2012-05-09 22:56   ` Tom Warren
2012-05-10 19:33     ` Albert ARIBAUD
2012-05-10 19:37   ` Simon Glass

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox