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

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

[Simon Glass]

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

Changes in v7:
- Use a really simple abs() macro for now

 arch/arm/cpu/armv7/omap4/clocks.c       |    2 --
 drivers/bios_emulator/x86emu/prim_ops.c |    5 -----
 include/common.h                        |    3 +++
 3 files changed, 3 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 4b5841e..2f2a869 100644
--- a/include/common.h
+++ b/include/common.h
@@ -222,6 +222,9 @@ typedef void (interrupt_handler_t)(void *);
 #define MIN(x, y)  min(x, y)
 #define MAX(x, y)  max(x, y)
 
+/* Return the absolute value of a number */
+#define abs(x)	((x) < 0 ? -(x) : (x))
+
 #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

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

[Tom Warren]

Simon,

> -----Original Message-----
> From: Simon Glass [mailto:sjg at chromium.org]
> Sent: Thursday, May 10, 2012 2:38 PM
> To: U-Boot Mailing List
> Cc: Tom Warren; Stephen Warren; Simon Glass
> Subject: [PATCH v7 03/23] Add abs() macro to return absolute value
> 
> 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

Adding Tom Rini and Graeme Russ to CC for OMAP4 and x86 signoff, although it's a trivial change.

I've applied this, run a MAKEALL w/o errors/warnings, and pushed it to u-boot-tegra/master, ready to generate a new pull request for ARM master when I get the Acks on this final patch.

I'd like to get this in before EOW, so Simon and I can finish up w/T20 LCD support and the SPI/UART fix and have a complete Tegra2 implementation ready for use.

Thanks,

Tom
> 
> Changes in v7:
> - Use a really simple abs() macro for now
> 
>  arch/arm/cpu/armv7/omap4/clocks.c       |    2 --
>  drivers/bios_emulator/x86emu/prim_ops.c |    5 -----
>  include/common.h                        |    3 +++
>  3 files changed, 3 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 4b5841e..2f2a869
> 100644
> --- a/include/common.h
> +++ b/include/common.h
> @@ -222,6 +222,9 @@ typedef void (interrupt_handler_t)(void *);  #define
> MIN(x, y)  min(x, y)  #define MAX(x, y)  max(x, y)
> 
> +/* Return the absolute value of a number */
> +#define abs(x)	((x) < 0 ? -(x) : (x))
> +
>  #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
-- 
nvpublic

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

[Simon Glass]

Hi Tom,

On Thu, May 10, 2012 at 3:56 PM, Tom Warren <TWarren@nvidia.com> wrote:

> Simon,
>
> > -----Original Message-----
> > From: Simon Glass [mailto:sjg at chromium.org]
> > Sent: Thursday, May 10, 2012 2:38 PM
> > To: U-Boot Mailing List
> > Cc: Tom Warren; Stephen Warren; Simon Glass
> > Subject: [PATCH v7 03/23] Add abs() macro to return absolute value
> >
> > 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
>
> Adding Tom Rini and Graeme Russ to CC for OMAP4 and x86 signoff, although
> it's a trivial change.
>
> I've applied this, run a MAKEALL w/o errors/warnings, and pushed it to
> u-boot-tegra/master, ready to generate a new pull request for ARM master
> when I get the Acks on this final patch.
>
> I'd like to get this in before EOW, so Simon and I can finish up w/T20 LCD
> support and the SPI/UART fix and have a complete Tegra2 implementation
> ready for use.
>

Yes that series needs a few clean-ups, but I will see what I can do.

Regards,
Simon



>
> Thanks,
>
> Tom
> >
> > Changes in v7:
> > - Use a really simple abs() macro for now
> >
> >  arch/arm/cpu/armv7/omap4/clocks.c       |    2 --
> >  drivers/bios_emulator/x86emu/prim_ops.c |    5 -----
> >  include/common.h                        |    3 +++
> >  3 files changed, 3 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 4b5841e..2f2a869
> > 100644
> > --- a/include/common.h
> > +++ b/include/common.h
> > @@ -222,6 +222,9 @@ typedef void (interrupt_handler_t)(void *);  #define
> > MIN(x, y)  min(x, y)  #define MAX(x, y)  max(x, y)
> >
> > +/* Return the absolute value of a number */
> > +#define abs(x)       ((x) < 0 ? -(x) : (x))
> > +
> >  #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
> --
> nvpublic
>
>

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

[Tom Rini]

On 05/10/2012 03:56 PM, Tom Warren wrote:
> Simon,
>
>> -----Original Message-----
>> From: Simon Glass [mailto:sjg at chromium.org]
>> Sent: Thursday, May 10, 2012 2:38 PM
>> To: U-Boot Mailing List
>> Cc: Tom Warren; Stephen Warren; Simon Glass
>> Subject: [PATCH v7 03/23] Add abs() macro to return absolute value
>>
>> 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
>
> Adding Tom Rini and Graeme Russ to CC for OMAP4 and x86 signoff, although it's a trivial change.

Acked-by: Tom Rini <trini@ti.com>

-- 
Tom

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

[Graeme Russ]

Hi Tom,

On Fri, May 11, 2012 at 8:56 AM, Tom Warren <TWarren@nvidia.com> wrote:
> Simon,
>
>> -----Original Message-----
>> From: Simon Glass [mailto:sjg at chromium.org]
>> Sent: Thursday, May 10, 2012 2:38 PM
>> To: U-Boot Mailing List
>> Cc: Tom Warren; Stephen Warren; Simon Glass
>> Subject: [PATCH v7 03/23] Add abs() macro to return absolute value
>>
>> 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
>
> Adding Tom Rini and Graeme Russ to CC for OMAP4 and x86 signoff, although

/drivers/bios_emulator/x86emu is not under my control - I think this is
used mostly by PPC. But I do have a couple of questions/comments

> it's a trivial change.

Hmm, int abs(in v)  is in prim_ops.c and not declared static - Is there
a matching declaration in a header somewhere or is it really a static
function which has not been declared as such?

> I've applied this, run a MAKEALL w/o errors/warnings, and pushed it to

Is that a complplete MAKEALL for all boards?

> u-boot-tegra/master, ready to generate a new pull request for ARM master
> when I get the Acks on this final patch.
>
> I'd like to get this in before EOW, so Simon and I can finish up w/T20
> LCD support and the SPI/UART fix and have a complete Tegra2
> implementation ready for use.
>>
>> Changes in v7:
>> - Use a really simple abs() macro for now
>>
>> ?arch/arm/cpu/armv7/omap4/clocks.c ? ? ? | ? ?2 --
>> ?drivers/bios_emulator/x86emu/prim_ops.c | ? ?5 -----
>> ?include/common.h ? ? ? ? ? ? ? ? ? ? ? ?| ? ?3 +++
>> ?3 files changed, 3 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;
>> -}

What on earth has your mailer done here?

>>
>> ?/*----------------------------- Implementation ----------------------------
>> */
>>
>> diff --git a/include/common.h b/include/common.h index 4b5841e..2f2a869
>> 100644
>> --- a/include/common.h
>> +++ b/include/common.h
>> @@ -222,6 +222,9 @@ typedef void (interrupt_handler_t)(void *); ?#define
>> MIN(x, y) ?min(x, y) ?#define MAX(x, y) ?max(x, y)
>>
>> +/* Return the absolute value of a number */
>> +#define abs(x) ? ? ? ((x) < 0 ? -(x) : (x))
>> +

Is that really the the extent of abs() in U-Boot (sorry, I don't have the
code at hand to do a search)?

>> ?#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
> --
> nvpublic


Regards,

Graeme

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

[Simon Glass]

Hi Graeme,

On Thu, May 10, 2012 at 4:45 PM, Graeme Russ <graeme.russ@gmail.com> wrote:

> Hi Tom,
>
> On Fri, May 11, 2012 at 8:56 AM, Tom Warren <TWarren@nvidia.com> wrote:
> > Simon,
> >
> >> -----Original Message-----
> >> From: Simon Glass [mailto:sjg at chromium.org]
> >> Sent: Thursday, May 10, 2012 2:38 PM
> >> To: U-Boot Mailing List
> >> Cc: Tom Warren; Stephen Warren; Simon Glass
> >> Subject: [PATCH v7 03/23] Add abs() macro to return absolute value
> >>
> >> 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
> >
> > Adding Tom Rini and Graeme Russ to CC for OMAP4 and x86 signoff, although
>
> /drivers/bios_emulator/x86emu is not under my control - I think this is
> used mostly by PPC. But I do have a couple of questions/comments
>
> > it's a trivial change.
>
> Hmm, int abs(in v)  is in prim_ops.c and not declared static - Is there
> a matching declaration in a header somewhere or is it really a static
> function which has not been declared as such?
>

The latter, so far as I could see. I built sequoia with CONFIG_VIDEO to
check.


>
> > I've applied this, run a MAKEALL w/o errors/warnings, and pushed it to
>
> Is that a complplete MAKEALL for all boards?
>
> > u-boot-tegra/master, ready to generate a new pull request for ARM master
> > when I get the Acks on this final patch.
> >
> > I'd like to get this in before EOW, so Simon and I can finish up w/T20
> > LCD support and the SPI/UART fix and have a complete Tegra2
> > implementation ready for use.
> >>
> >> Changes in v7:
> >> - Use a really simple abs() macro for now
> >>
> >>  arch/arm/cpu/armv7/omap4/clocks.c       |    2 --
> >>  drivers/bios_emulator/x86emu/prim_ops.c |    5 -----
> >>  include/common.h                        |    3 +++
> >>  3 files changed, 3 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;
> >> -}
>
> What on earth has your mailer done here?
>
> >>
> >>  /*----------------------------- Implementation
> ----------------------------
> >> */
> >>
> >> diff --git a/include/common.h b/include/common.h index 4b5841e..2f2a869
> >> 100644
> >> --- a/include/common.h
> >> +++ b/include/common.h
> >> @@ -222,6 +222,9 @@ typedef void (interrupt_handler_t)(void *);  #define
> >> MIN(x, y)  min(x, y)  #define MAX(x, y)  max(x, y)
> >>
> >> +/* Return the absolute value of a number */
> >> +#define abs(x)       ((x) < 0 ? -(x) : (x))
> >> +
>
> Is that really the the extent of abs() in U-Boot (sorry, I don't have the
> code at hand to do a search)?
>

It seems so.


>
> >>  #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
> > --
> > nvpublic
>
>
> Regards,
>
> Graeme
>

Regards,
Simon

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

[Tom Warren]

Graeme,

> -----Original Message-----
> From: Graeme Russ [mailto:graeme.russ at gmail.com]
> Sent: Thursday, May 10, 2012 4:46 PM
> To: Tom Warren
> Cc: Simon Glass; U-Boot Mailing List; Stephen Warren; Tom Rini; Albert
> ARIBAUD
> Subject: Re: [PATCH v7 03/23] Add abs() macro to return absolute value
> 
> Hi Tom,
> 
> On Fri, May 11, 2012 at 8:56 AM, Tom Warren <TWarren@nvidia.com> wrote:
> > Simon,
> >
> >> -----Original Message-----
> >> From: Simon Glass [mailto:sjg at chromium.org]
> >> Sent: Thursday, May 10, 2012 2:38 PM
> >> To: U-Boot Mailing List
> >> Cc: Tom Warren; Stephen Warren; Simon Glass
> >> Subject: [PATCH v7 03/23] Add abs() macro to return absolute value
> >>
> >> 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
> >
> > Adding Tom Rini and Graeme Russ to CC for OMAP4 and x86 signoff,
> > although
> 
> /drivers/bios_emulator/x86emu is not under my control - I think this is used
> mostly by PPC. But I do have a couple of questions/comments

Thanks for the review, regardless. Using 'git blame', it appears that Jason Jin @ Freescale wrote most of this file.

> 
> > it's a trivial change.
> 
> Hmm, int abs(in v)  is in prim_ops.c and not declared static - Is there a
> matching declaration in a header somewhere or is it really a static function
> which has not been declared as such?

As Simon said, it appears to be a static func that wasn't declared as such, so no header to worry about.

> 
> > I've applied this, run a MAKEALL w/o errors/warnings, and pushed it to
> 
> Is that a complplete MAKEALL for all boards?

I did a MAKEALL -c armv7. Simon did the larger build, since it's his change.

> 
> > u-boot-tegra/master, ready to generate a new pull request for ARM
> > master when I get the Acks on this final patch.
> >
> > I'd like to get this in before EOW, so Simon and I can finish up w/T20
> > LCD support and the SPI/UART fix and have a complete Tegra2
> > implementation ready for use.
> >>
> >> Changes in v7:
> >> - Use a really simple abs() macro for now
> >>
> >> ?arch/arm/cpu/armv7/omap4/clocks.c ? ? ? | ? ?2 --
> >> ?drivers/bios_emulator/x86emu/prim_ops.c | ? ?5 -----
> >> ?include/common.h ? ? ? ? ? ? ? ? ? ? ? ?| ? ?3 +++
> >> ?3 files changed, 3 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;
> >> -}
> 
> What on earth has your mailer done here?

No idea - sorry about that. Outlook sucks.

> 
> >>
> >> ?/*----------------------------- Implementation
> >> ---------------------------- */
> >>
> >> diff --git a/include/common.h b/include/common.h index
> >> 4b5841e..2f2a869
> >> 100644
> >> --- a/include/common.h
> >> +++ b/include/common.h
> >> @@ -222,6 +222,9 @@ typedef void (interrupt_handler_t)(void *);
> >> #define MIN(x, y) ?min(x, y) ?#define MAX(x, y) ?max(x, y)
> >>
> >> +/* Return the absolute value of a number */ #define abs(x)
> >> +((x) < 0 ? -(x) : (x))
> >> +
> 
> Is that really the the extent of abs() in U-Boot (sorry, I don't have the
> code at hand to do a search)?

As Simon said, it appears that that's it for abs() - only OMAP4 and x86emu.

> 
> >> ?#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
> > --
> > nvpublic
> 
> 
> Regards,
> 
> Graeme
-- 
nvpublic

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

[Wolfgang Denk]

Dear Simon Glass,

In message <1336685875-13177-4-git-send-email-sjg@chromium.org> you wrote:
> This macro is generally useful to make it available in common.

I agree with the idea of the patch, but I object against the
current implementation.

> +/* Return the absolute value of a number */
> +#define abs(x)	((x) < 0 ? -(x) : (x))

NAK.  This macro can have nasty side effects.  Consider something like

	foo = abs(bar++);

or

	foo = abs(in_be32(reg));

etc.

Why do you re-invent the wheel (poorly) instead of - for example -
copying the definition from Linux ("include/linux/kernel.h") ?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
It seems intuitively obvious to me, which  means  that  it  might  be
wrong.                                                 -- Chris Torek

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

[Simon Glass]

Hi,

On Fri, May 11, 2012 at 12:16 PM, Wolfgang Denk <wd@denx.de> wrote:

> Dear Simon Glass,
>
> In message <1336685875-13177-4-git-send-email-sjg@chromium.org> you wrote:
> > This macro is generally useful to make it available in common.
>
> I agree with the idea of the patch, but I object against the
> current implementation.
>
> > +/* Return the absolute value of a number */
> > +#define abs(x)       ((x) < 0 ? -(x) : (x))
>
> NAK.  This macro can have nasty side effects.  Consider something like
>
>        foo = abs(bar++);
>
> or
>
>        foo = abs(in_be32(reg));
>
> etc.
>
> Why do you re-invent the wheel (poorly) instead of - for example -
> copying the definition from Linux ("include/linux/kernel.h") ?
>

That was the similar to the first implementation, which always returned
long, but Albert didn't like it do I did something simple.

So I will go back to the more complicated way, and this time just copy what
the kernel does. The difference is that it doesn't use typeof().

Regards,
Simon


> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> It seems intuitively obvious to me, which  means  that  it  might  be
> wrong.                                                 -- Chris Torek
>

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

[Wolfgang Denk]

Dear Simon Glass,

In message <CAPnjgZ2hep7ApaNMakKrAEpCPffQ74BogGo=GnWTy47akewXTQ@mail.gmail.com> you wrote:
>
> So I will go back to the more complicated way, and this time just copy what
> the kernel does. The difference is that it doesn't use typeof().

I have to admit that I tend to prefer the typeof() version, and I
wonder why the kernel doesn;t use that.  I guess there are reasons for
that.  Does anybody know what these might be?

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Reader, suppose you were an idiot. And suppose you were a  member  of
Congress. But I repeat myself.                           - Mark Twain

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

[Simon Glass]

Hi Wolfgang,

On Fri, May 11, 2012 at 1:13 PM, Wolfgang Denk <wd@denx.de> wrote:

> Dear Simon Glass,
>
> In message <CAPnjgZ2hep7ApaNMakKrAEpCPffQ74BogGo=
> GnWTy47akewXTQ at mail.gmail.com> you wrote:
> >
> > So I will go back to the more complicated way, and this time just copy
> what
> > the kernel does. The difference is that it doesn't use typeof().
>
> I have to admit that I tend to prefer the typeof() version, and I
> wonder why the kernel doesn;t use that.  I guess there are reasons for
> that.  Does anybody know what these might be?
>

Not me, but:

commit 71a9048448de302d1e968f336de01060d02fae71
Author: Andrew Morton <akpm@linux-foundation.org>
Date:   Wed Jan 12 16:59:35 2011 -0800

    include/linux/kernel.h: abs(): fix handling of 32-bit unsigneds on
64-bit

    Michal reports:

    In the framebuffer subsystem the abs() macro is often used as a part of
    the calculation of a Manhattan metric, which in turn is used as a
measure
    of similarity between video modes.  The arguments of abs() are sometimes
    unsigned numbers.  This worked fine until commit a49c59c0 ("Make sure
the
    value in abs() does not get truncated if it is greater than 2^32:) ,
which
    changed the definition of abs() to prevent truncation.  As a result of
    this change, in the following piece of code:

    u32 a = 0, b = 1;
    u32 c = abs(a - b);

    'c' will end up with a value of 0xffffffff instead of the expected 0x1.

    A problem caused by this change and visible by the end user is that
    framebuffer drivers relying on functions from modedb.c will fail to find
    high resolution video modes similar to that explicitly requested by the
    user if an exact match cannot be found (see e.g.

    Fix this by special-casing `long' types within abs().

    This patch reduces x86_64 code size a bit - drivers/video/uvesafb.o
shrunk
    by 15 bytes, presumably because it is doing abs() on 4-byte quantities,
    and expanding those to 8-byte longs adds code.

    testcase:

    #define oldabs(x) ({                                \
                long __x = (x);                 \
                (__x < 0) ? -__x : __x;         \
        })

    #define newabs(x) ({                                                \
                long ret;                                       \
                if (sizeof(x) == sizeof(long)) {                \
                        long __x = (x);                         \
                        ret = (__x < 0) ? -__x : __x;           \
                } else {                                        \
                        int __x = (x);                          \
                        ret = (__x < 0) ? -__x : __x;           \
                }                                               \
                ret;                                            \
        })

    typedef unsigned int u32;

    main()
    {
        u32 a = 0;
        u32 b = 1;
        u32 oldc = oldabs(a - b);
        u32 newc = newabs(a - b);

        printf("%u %u\n", oldc, newc);
    }

    akpm:/home/akpm> gcc t.c
    akpm:/home/akpm> ./a.out
    4294967295 1

    Reported-by: Michal Januszewski <michalj@gmail.com>
    Cc: Rolf Eike Beer <eike-kernel at sf-tec.de
    Cc: Geert Uytterhoeven <geert@linux-m68k.org>
    Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Regards,
Simon



>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> Reader, suppose you were an idiot. And suppose you were a  member  of
> Congress. But I repeat myself.                           - Mark Twain
>

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

[Wolfgang Denk]

Dear Simon Glass,

In message <CAPnjgZ1jSrcdNVgN5aMG+z-94-vpH2ym3B=D6pJ6Bxib7oek2A@mail.gmail.com> you wrote:
>
> > I have to admit that I tend to prefer the typeof() version, and I
> > wonder why the kernel doesn;t use that.  I guess there are reasons for
> > that.  Does anybody know what these might be?
> >
> 
> Not me, but:

Thanks.  So there is indeed a problem with the "obvious"
implementation :-(

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Madness takes its toll. Please have exact change.

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

[Simon Glass]

Hi,

On Fri, May 11, 2012 at 2:24 PM, Wolfgang Denk <wd@denx.de> wrote:

> Dear Simon Glass,
>
> In message <CAPnjgZ1jSrcdNVgN5aMG+z-94-vpH2ym3B=
> D6pJ6Bxib7oek2A at mail.gmail.com> you wrote:
> >
> > > I have to admit that I tend to prefer the typeof() version, and I
> > > wonder why the kernel doesn;t use that.  I guess there are reasons for
> > > that.  Does anybody know what these might be?
> > >
> >
> > Not me, but:
>
> Thanks.  So there is indeed a problem with the "obvious"
> implementation :-(
>

Yes, there are always problems, normally solved by additional complexity
and lines of code. I will do a patch with the kernel version as I
mentioned, as perhaps we can go with that.

Regards,
Simon


>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> Madness takes its toll. Please have exact change.
>