[U-Boot] [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
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) || \
Simon,
-----Original Message----- From: Simon Glass [mailto:sjg@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
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@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)
==
#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
On 05/10/2012 03:56 PM, Tom Warren wrote:
Simon,
-----Original Message----- From: Simon Glass [mailto:sjg@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 Glasssjg@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
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@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
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@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)
==
#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
Graeme,
-----Original Message----- From: Graeme Russ [mailto:graeme.russ@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@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)) &
- ==
#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
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
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@denx.de It seems intuitively obvious to me, which means that it might be wrong. -- Chris Torek
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
Hi Wolfgang,
On Fri, May 11, 2012 at 1:13 PM, Wolfgang Denk wd@denx.de wrote:
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?
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@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@denx.de Reader, suppose you were an idiot. And suppose you were a member of Congress. But I repeat myself. - Mark Twain
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
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@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@denx.de Madness takes its toll. Please have exact change.
participants (5)
-
Graeme Russ -
Simon Glass -
Tom Rini -
Tom Warren -
Wolfgang Denk