[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 at 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 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
> >
> > 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