* [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value @ 2012-05-10 21:37 Simon Glass 2012-05-10 22:56 ` Tom Warren 2012-05-11 19:16 ` Wolfgang Denk 0 siblings, 2 replies; 13+ messages in thread From: Simon Glass @ 2012-05-10 21:37 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 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value 2012-05-10 21:37 [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value Simon Glass @ 2012-05-10 22:56 ` Tom Warren 2012-05-10 23:14 ` Simon Glass ` (2 more replies) 2012-05-11 19:16 ` Wolfgang Denk 1 sibling, 3 replies; 13+ messages in thread From: Tom Warren @ 2012-05-10 22:56 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value 2012-05-10 22:56 ` Tom Warren @ 2012-05-10 23:14 ` Simon Glass 2012-05-10 23:41 ` Tom Rini 2012-05-10 23:45 ` Graeme Russ 2 siblings, 0 replies; 13+ messages in thread From: Simon Glass @ 2012-05-10 23:14 UTC (permalink / raw) To: u-boot 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 > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value 2012-05-10 22:56 ` Tom Warren 2012-05-10 23:14 ` Simon Glass @ 2012-05-10 23:41 ` Tom Rini 2012-05-10 23:45 ` Graeme Russ 2 siblings, 0 replies; 13+ messages in thread From: Tom Rini @ 2012-05-10 23:41 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value 2012-05-10 22:56 ` Tom Warren 2012-05-10 23:14 ` Simon Glass 2012-05-10 23:41 ` Tom Rini @ 2012-05-10 23:45 ` Graeme Russ 2012-05-10 23:49 ` Simon Glass 2012-05-11 16:11 ` Tom Warren 2 siblings, 2 replies; 13+ messages in thread From: Graeme Russ @ 2012-05-10 23:45 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value 2012-05-10 23:45 ` Graeme Russ @ 2012-05-10 23:49 ` Simon Glass 2012-05-11 16:11 ` Tom Warren 1 sibling, 0 replies; 13+ messages in thread From: Simon Glass @ 2012-05-10 23:49 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value 2012-05-10 23:45 ` Graeme Russ 2012-05-10 23:49 ` Simon Glass @ 2012-05-11 16:11 ` Tom Warren 1 sibling, 0 replies; 13+ messages in thread From: Tom Warren @ 2012-05-11 16:11 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value 2012-05-10 21:37 [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value Simon Glass 2012-05-10 22:56 ` Tom Warren @ 2012-05-11 19:16 ` Wolfgang Denk 2012-05-11 19:43 ` Simon Glass 1 sibling, 1 reply; 13+ messages in thread From: Wolfgang Denk @ 2012-05-11 19:16 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value 2012-05-11 19:16 ` Wolfgang Denk @ 2012-05-11 19:43 ` Simon Glass 2012-05-11 20:13 ` Wolfgang Denk 0 siblings, 1 reply; 13+ messages in thread From: Simon Glass @ 2012-05-11 19:43 UTC (permalink / raw) To: u-boot 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value 2012-05-11 19:43 ` Simon Glass @ 2012-05-11 20:13 ` Wolfgang Denk 2012-05-11 21:08 ` Simon Glass 0 siblings, 1 reply; 13+ messages in thread From: Wolfgang Denk @ 2012-05-11 20:13 UTC (permalink / raw) To: u-boot 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value 2012-05-11 20:13 ` Wolfgang Denk @ 2012-05-11 21:08 ` Simon Glass 2012-05-11 21:24 ` Wolfgang Denk 0 siblings, 1 reply; 13+ messages in thread From: Simon Glass @ 2012-05-11 21:08 UTC (permalink / raw) To: u-boot 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 > ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value 2012-05-11 21:08 ` Simon Glass @ 2012-05-11 21:24 ` Wolfgang Denk 2012-05-11 22:31 ` Simon Glass 0 siblings, 1 reply; 13+ messages in thread From: Wolfgang Denk @ 2012-05-11 21:24 UTC (permalink / raw) To: u-boot 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. ^ permalink raw reply [flat|nested] 13+ messages in thread
* [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value 2012-05-11 21:24 ` Wolfgang Denk @ 2012-05-11 22:31 ` Simon Glass 0 siblings, 0 replies; 13+ messages in thread From: Simon Glass @ 2012-05-11 22:31 UTC (permalink / raw) To: u-boot 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. > ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-05-11 22:31 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-05-10 21:37 [U-Boot] [PATCH v7 03/23] Add abs() macro to return absolute value Simon Glass 2012-05-10 22:56 ` Tom Warren 2012-05-10 23:14 ` Simon Glass 2012-05-10 23:41 ` Tom Rini 2012-05-10 23:45 ` Graeme Russ 2012-05-10 23:49 ` Simon Glass 2012-05-11 16:11 ` Tom Warren 2012-05-11 19:16 ` Wolfgang Denk 2012-05-11 19:43 ` Simon Glass 2012-05-11 20:13 ` Wolfgang Denk 2012-05-11 21:08 ` Simon Glass 2012-05-11 21:24 ` Wolfgang Denk 2012-05-11 22:31 ` Simon Glass
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.