Add abs() macro to return absolute value

PATCH Superseded Simon Glass 5 participants 13 messages May 10, 2012 – May 11, 2012 Arm (32-bit)

lore

Share & tools

lore

Simon Glass

author

2012-05-10 21:37 UTC #0 lore raw

PATCH v7 3/23 Add abs() macro to return absolute value

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

Signed-off-by Simon Glass <[email protected]>

---
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(-)

arch/arm/cpu/armv7/omap4/clocks.c +0 -2

index e2189f7..ce3f59c 100644
@@ -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] = {

drivers/bios_emulator/x86emu/prim_ops.c +0 -5

index 7553087..5f6c795 100644
@@ -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 ----------------------------*/

include/common.h +3 -0

index 4b5841e..2f2a869 100644
@@ -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) || \

Signature · 1 line

1.7.7.3

Tom Warren

2012-05-10 22:56 UTC #1 lore raw

PATCH v7 3/23 Add abs() macro to return absolute value

Simon,

Quoted 9 lines

-----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.

Quoted 7 lines

Signed-off-by Simon Glass <[email protected]>
---
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

Quoted 56 lines

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(-)

arch/arm/cpu/armv7/omap4/clocks.c +0 -2

index e2189f7..ce3f59c 100644
@@ -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] = {

drivers/bios_emulator/x86emu/prim_ops.c +0 -3

index 7553087..5f6c795 100644
@@ -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 ----------------------------

include/common.h index 4b5841e..2f2a869 renamed from include/common.h +0 -0

100644

include/common.h +3 -1

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

Signature · 1 line

nvpublic

Simon Glass

author

2012-05-10 23:14 UTC #2 lore raw

PATCH v7 3/23 Add abs() macro to return absolute value

Hi Tom,

Quoted 22 lines · Tom Warren

Simon,
Quoted 9 lines
-----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.
Quoted 7 lines
Signed-off-by Simon Glass <[email protected]>
---
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.

Quoted 8 lines

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

Quoted 69 lines

Thanks,
Tom
Quoted 15 lines
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(-)
arch/arm/cpu/armv7/omap4/clocks.c +0 -0
index e2189f7..ce3f59c 100644
arch/arm/cpu/armv7/omap4/clocks.c line 49
Quoted 6 lines
arch/arm/cpu/armv7/omap4/clocks.c +0 -2
@@ -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;
drivers/bios_emulator/x86emu/prim_ops.c static u32 x86emu_parity_tab[8] = · line 119
Quoted 10 lines
arch/arm/cpu/armv7/omap4/clocks.c +0 -0
 const u32 sys_clk_array[8] = {
drivers/bios_emulator/x86emu/prim_ops.c +0 -0
index 7553087..5f6c795 100644
@@ -118,11 +118,6 @@ static u32 x86emu_parity_tab[8] =
 
 #define PARITY(x)   (((x86emu_parity_tab[(x) / 32] >> ((x) % 32)) & 1)
==
drivers/bios_emulator/x86emu/prim_ops.c static u32 x86emu_parity_tab[8] = · line 122
Quoted 3 lines
drivers/bios_emulator/x86emu/prim_ops.c +0 -1
 0)
 #define XOR2(x)          (((x) ^ ((x)>>1)) & 0x1)
-/*----------------------------- Implementation
----------------------------
drivers/bios_emulator/x86emu/prim_ops.c static u32 x86emu_parity_tab[8] =
Quoted 5 lines
drivers/bios_emulator/x86emu/prim_ops.c +0 -2
 */ -int abs(int v) -{
-     return (v>0)?v:-v;
-}
 
 /*----------------------------- Implementation
----------------------------
Quoted 7 lines
*/
include/common.h index 4b5841e..2f2a869 renamed from include/common.h +0 -0
100644
include/common.h typedef void (interrupt_handler_t)(void *); #define · line 229
Quoted 8 lines
include/common.h +3 -0
@@ -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)
|| \
Quoted 2 lines
include/common.h +0 -1
--
1.7.7.3
--
nvpublic

Tom Rini

2012-05-10 23:41 UTC #3 lore raw

PATCH v7 3/23 Add abs() macro to return absolute value

In reply to Tom Warren

Quoted 16 lines · Tom Warren

Simon,
Quoted 14 lines
-----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<[email protected]>
---
Changes in v3:
- Add new patch to put abs() in common.h

Quoted 4 lines

Quoted 2 lines
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 <[email protected]>

Signature · 1 line

Tom

Graeme Russ

2012-05-10 23:45 UTC #4 lore raw

PATCH v7 3/23 Add abs() macro to return absolute value

In reply to Tom Warren

Hi Tom,

Quoted 16 lines · Tom Warren

Simon,
Quoted 14 lines
-----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 <[email protected]>
---
Changes in v3:
- Add new patch to put abs() in common.h

Quoted 4 lines

Quoted 2 lines
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

Quoted 1 line

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?

Quoted 1 line

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

Is that a complplete MAKEALL for all boards?

Quoted 36 lines

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.
Quoted 30 lines
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(-)
arch/arm/cpu/armv7/omap4/clocks.c +0 -0
index e2189f7..ce3f59c 100644
@@ -46,8 +46,6 @@
 ?#define puts(s)
?#endif
arch/arm/cpu/armv7/omap4/clocks.c +0 -2
-#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] = {
drivers/bios_emulator/x86emu/prim_ops.c +0 -0
index 7553087..5f6c795 100644
@@ -118,11 +118,6 @@ static u32 x86emu_parity_tab[8] =
 

Quoted 7 lines

Quoted 7 lines
?#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?

Quoted 11 lines

Quoted 11 lines
?/*----------------------------- Implementation ---------------------------- */
include/common.h index 4b5841e..2f2a869 renamed from include/common.h +0 -0
100644
include/common.h +0 -0
@@ -222,6 +222,9 @@ typedef void (interrupt_handler_t)(void *); ?#define
 MIN(x, y) ?min(x, y) ?#define MAX(x, y) ?max(x, y)
 

Quoted 3 lines

include/common.h typedef void (interrupt_handler_t)(void *); · line 227
Quoted 3 lines
include/common.h +3 -0
+/* 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)?

Quoted 7 lines

Quoted 5 lines
include/common.h +0 -0
 ?#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

Simon Glass

author

2012-05-10 23:49 UTC #5 lore raw

PATCH v7 3/23 Add abs() macro to return absolute value

Hi Graeme,

Quoted 24 lines · Graeme Russ

Hi Tom,
Quoted 16 lines · Tom Warren
Simon,
Quoted 14 lines
-----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 <[email protected]>
---
Changes in v3:
- Add new patch to put abs() in common.h
Quoted 4 lines
Quoted 2 lines
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

Quoted 9 lines

/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
Quoted 1 line
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.

Quoted 66 lines

Quoted 2 lines
I've applied this, run a MAKEALL w/o errors/warnings, and pushed it to
Is that a complplete MAKEALL for all boards?
Quoted 24 lines
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.
Quoted 18 lines
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(-)
arch/arm/cpu/armv7/omap4/clocks.c +0 -0
index e2189f7..ce3f59c 100644
@@ -46,8 +46,6 @@
 #define puts(s)
 #endif
 
Quoted 3 lines
arch/arm/cpu/armv7/omap4/clocks.c
Quoted 3 lines
arch/arm/cpu/armv7/omap4/clocks.c +0 -2
-#define abs(x) (((x) < 0) ? ((x)*-1) : (x))
-
 struct omap4_prcm_regs *const prcm = (struct omap4_prcm_regs
*)0x4A004100;
Quoted 10 lines
drivers/bios_emulator/x86emu/prim_ops.c static u32 x86emu_parity_tab[8] = · line 119
Quoted 10 lines
arch/arm/cpu/armv7/omap4/clocks.c +0 -0
 const u32 sys_clk_array[8] = {
drivers/bios_emulator/x86emu/prim_ops.c +0 -0
index 7553087..5f6c795 100644
@@ -118,11 +118,6 @@ static u32 x86emu_parity_tab[8] =
 
 #define PARITY(x)   (((x86emu_parity_tab[(x) / 32] >> ((x) % 32)) & 1)
==
Quoted 3 lines
drivers/bios_emulator/x86emu/prim_ops.c static u32 x86emu_parity_tab[8] = · line 122
Quoted 3 lines
drivers/bios_emulator/x86emu/prim_ops.c +0 -1
 0)
 #define XOR2(x)          (((x) ^ ((x)>>1)) & 0x1)
-/*----------------------------- Implementation
----------------------------
Quoted 3 lines
drivers/bios_emulator/x86emu/prim_ops.c static u32 x86emu_parity_tab[8] = · line 125
Quoted 3 lines
drivers/bios_emulator/x86emu/prim_ops.c +0 -2
 */ -int abs(int v) -{
-     return (v>0)?v:-v;
-}
What on earth has your mailer done here?
Quoted 2 lines
drivers/bios_emulator/x86emu/prim_ops.c static u32 x86emu_parity_tab[8] = · line 123
Quoted 2 lines
drivers/bios_emulator/x86emu/prim_ops.c +0 -0
 /*----------------------------- Implementation
----------------------------
Quoted 9 lines
Quoted 9 lines
*/
include/common.h index 4b5841e..2f2a869 renamed from include/common.h +0 -0
100644
include/common.h +0 -0
@@ -222,6 +222,9 @@ typedef void (interrupt_handler_t)(void *);  #define
 MIN(x, y)  min(x, y)  #define MAX(x, y)  max(x, y)
 

Quoted 7 lines

Quoted 3 lines
include/common.h typedef void (interrupt_handler_t)(void *); · line 227
Quoted 3 lines
include/common.h +3 -0
+/* 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.

Quoted 7 lines

Quoted 4 lines
include/common.h typedef void (interrupt_handler_t)(void *);
Quoted 4 lines
include/common.h +0 -0
 #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)
|| \
Quoted 2 lines
Quoted 2 lines
include/common.h +0 -1
--
1.7.7.3

Quoted 8 lines

Quoted 2 lines
--
nvpublic
Regards,
Graeme

Regards,
Simon

Tom Warren

2012-05-11 16:11 UTC #6 lore raw

PATCH v7 3/23 Add abs() macro to return absolute value

In reply to Graeme Russ

Graeme,

Quoted 27 lines

-----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,
Quoted 16 lines · Tom Warren
Simon,
Quoted 14 lines
-----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 <[email protected]>
---
Changes in v3:
- Add new patch to put abs() in common.h

Quoted 8 lines

Quoted 5 lines
Quoted 2 lines
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.

Quoted 6 lines

Quoted 2 lines
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.

Quoted 4 lines

Quoted 2 lines
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.

Quoted 38 lines

Quoted 38 lines
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.
Quoted 31 lines
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(-)
arch/arm/cpu/armv7/omap4/clocks.c +0 -0
index e2189f7..ce3f59c 100644
@@ -46,8 +46,6 @@
 ?#define puts(s)
?#endif
arch/arm/cpu/armv7/omap4/clocks.c +0 -2
-#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] = {
drivers/bios_emulator/x86emu/prim_ops.c +0 -0
index 7553087..5f6c795 100644
@@ -118,11 +118,6 @@ static u32 x86emu_parity_tab[8] =
 

Quoted 10 lines

Quoted 8 lines
drivers/bios_emulator/x86emu/prim_ops.c
Quoted 8 lines
?#define PARITY(x) ? (((x86emu_parity_tab[(x) / 32] >> ((x) % 32)) &
1) ==
0)
?#define XOR2(x) ? ? ? ? ?(((x) ^ ((x)>>1)) & 0x1)
drivers/bios_emulator/x86emu/prim_ops.c +0 -4
-/*----------------------------- 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.

Quoted 13 lines

Quoted 13 lines
Quoted 13 lines
?/*----------------------------- Implementation ---------------------------- */
include/common.h index renamed from include/common.h +0 -0
4b5841e..2f2a869
100644
include/common.h +0 -0
@@ -222,6 +222,9 @@ typedef void (interrupt_handler_t)(void *);
 #define MIN(x, y) ?min(x, y) ?#define MAX(x, y) ?max(x, y)
 

Quoted 6 lines

Quoted 3 lines
include/common.h typedef void (interrupt_handler_t)(void *); · line 226
Quoted 3 lines
include/common.h +3 -0
+/* 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.

Quoted 14 lines

Quoted 9 lines
Quoted 7 lines
include/common.h +0 -0
 ?#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

Signature · 1 line

nvpublic

Wolfgang Denk

2012-05-11 19:16 UTC #7 lore raw

PATCH v7 3/23 Add abs() macro to return absolute value

In reply to Simon Glass

Dear Simon Glass,

Quoted 1 line · In message <[email protected]> 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.

include/common.h typedef void (interrupt_handler_t)(void *); · line 226

Quoted 2 lines

include/common.h +2 -0

+/* 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++);

	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

Signature · 5 lines

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: [email protected]
It seems intuitively obvious to me, which  means  that  it  might  be
wrong.                                                 -- Chris Torek

Simon Glass

author

2012-05-11 19:43 UTC #8 lore raw

PATCH v7 3/23 Add abs() macro to return absolute value

Hi,

Quoted 15 lines · Wolfgang Denk

Dear Simon Glass,
Quoted 1 line · In message <[email protected]> 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.
include/common.h typedef void (interrupt_handler_t)(void *); · line 226
Quoted 2 lines
include/common.h +2 -0
+/* 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++);

Quoted 9 lines

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

Quoted 11 lines

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

Wolfgang Denk

2012-05-11 20:13 UTC #9 lore raw

PATCH v7 3/23 Add abs() macro to return absolute value

Dear Simon Glass,

Quoted 3 lines · In message <CAPnjgZ2hep7ApaNMakKrAEpCPffQ74BogGo=GnWTy47akewXTQ@mail.gmail.com>…

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

Signature · 5 lines

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

Simon Glass

author

2012-05-11 21:08 UTC #10 lore raw

PATCH v7 3/23 Add abs() macro to return absolute value

Hi Wolfgang,

Quoted 5 lines · Wolfgang Denk

Dear Simon Glass,
In message <CAPnjgZ2hep7ApaNMakKrAEpCPffQ74BogGo=
Quoted 1 line · GnWTy47akewXTQ at mail.gmail.com> you wrote:

Quoted 8 lines

Quoted 1 line
So I will go back to the more complicated way, and this time just copy
what
Quoted 1 line
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 <[email protected]>
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 <[email protected]>
    Cc: Rolf Eike Beer <eike-kernel at sf-tec.de
    Cc: Geert Uytterhoeven <[email protected]>
    Signed-off-by: Andrew Morton <[email protected]>
    Signed-off-by: Linus Torvalds <[email protected]>

Regards,
Simon

Quoted 12 lines

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

Wolfgang Denk

2012-05-11 21:24 UTC #11 lore raw

PATCH v7 3/23 Add abs() macro to return absolute value

Dear Simon Glass,

Quoted 7 lines · In message <CAPnjgZ1jSrcdNVgN5aMG+z-94-vpH2ym3B=D6pJ6Bxib7oek2A@mail.gmail.com>…

Quoted 5 lines
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

Signature · 4 lines

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.

Simon Glass

author

2012-05-11 22:31 UTC #12 lore raw

PATCH v7 3/23 Add abs() macro to return absolute value

Hi,

Quoted 10 lines · Wolfgang Denk

Dear Simon Glass,
In message <CAPnjgZ1jSrcdNVgN5aMG+z-94-vpH2ym3B=
Quoted 6 lines · D6pJ6Bxib7oek2A at mail.gmail.com> you wrote:
Quoted 5 lines
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?

Quoted 5 lines

Quoted 1 line
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

Quoted 11 lines

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.