Graeme, > -----Original Message----- > From: Graeme Russ [mailto:graeme.r...@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 <twar...@nvidia.com> wrote: > > Simon, > > > >> -----Original Message----- > >> From: Simon Glass [mailto:s...@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 <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 > > > > 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 mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot