Patchwork [U-Boot,1/7] i2c: Use __weak instead of __attribute__((weak, alias))

login
register
mail settings
Submitter Marek Vasut
Date Nov. 13, 2012, 12:34 a.m.
Message ID <1352766871-892-1-git-send-email-marex@denx.de>
Download mbox | patch
Permalink /patch/198508/
State Awaiting Upstream
Delegated to: Heiko Schocher
Headers show

Comments

Marek Vasut - Nov. 13, 2012, 12:34 a.m.
Use __weak from linux/compiler.h instead of __attribute__((weak, alias))
to define overridable function. This patch is intended as a cleanup patch
to bring some consistency into the code.

Signed-off-by: Marek Vasut <marex@denx.de>
Cc: Heiko Schocher <hs@denx.de>
---
 common/cmd_i2c.c |   16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)
Wolfgang Denk - Nov. 13, 2012, 7:16 a.m.
Dear Marek Vasut,

In message <1352766871-892-1-git-send-email-marex@denx.de> you wrote:
> Use __weak from linux/compiler.h instead of __attribute__((weak, alias))
> to define overridable function. This patch is intended as a cleanup patch
> to bring some consistency into the code.
> 
> Signed-off-by: Marek Vasut <marex@denx.de>
> Cc: Heiko Schocher <hs@denx.de>
> ---
>  common/cmd_i2c.c |   16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)

The actual definition of __weak is in include/linux/compiler-gcc.h and
looks like this:

	#define __weak	__attribute__((weak))

which means you omit the ", alias" part of the existing code.

Are you 100% sure that this has no impacts on the behaviour?

In my understanding, "weak" and "weak, alias" are not exactly the
same...

Best regards,

Wolfgang Denk
Marek Vasut - Nov. 13, 2012, 1:48 p.m.
Dear Wolfgang Denk,

> Dear Marek Vasut,
> 
> In message <1352766871-892-1-git-send-email-marex@denx.de> you wrote:
> > Use __weak from linux/compiler.h instead of __attribute__((weak, alias))
> > to define overridable function. This patch is intended as a cleanup patch
> > to bring some consistency into the code.
> > 
> > Signed-off-by: Marek Vasut <marex@denx.de>
> > Cc: Heiko Schocher <hs@denx.de>
> > ---
> > 
> >  common/cmd_i2c.c |   16 +++++++---------
> >  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> The actual definition of __weak is in include/linux/compiler-gcc.h and
> looks like this:
> 
> 	#define __weak	__attribute__((weak))
> 
> which means you omit the ", alias" part of the existing code.
> 
> Are you 100% sure that this has no impacts on the behaviour?

Yes

> In my understanding, "weak" and "weak, alias" are not exactly the
> same...

Can you please elaborate? The point of this alias here is to call __def_i2c_*() 
in case the overriding function isn't defined. Otherwise call the overriding 
function. The __def_i2c_*() is never called directly.

> Best regards,
> 
> Wolfgang Denk

Best regards,
Marek Vasut

Patch

diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
index 4438db5..c8990ef 100644
--- a/common/cmd_i2c.c
+++ b/common/cmd_i2c.c
@@ -82,6 +82,7 @@ 
 #include <i2c.h>
 #include <malloc.h>
 #include <asm/byteorder.h>
+#include <linux/compiler.h>
 
 /* Display values from last command.
  * Memory modify remembered values are different from display memory.
@@ -133,30 +134,27 @@  DECLARE_GLOBAL_DATA_PTR;
 #define DISP_LINE_LEN	16
 
 /* implement possible board specific board init */
-static void __def_i2c_init_board(void)
+__weak
+void i2c_init_board(void)
 {
 	return;
 }
-void i2c_init_board(void)
-	__attribute__((weak, alias("__def_i2c_init_board")));
 
 /* TODO: Implement architecture-specific get/set functions */
-static unsigned int __def_i2c_get_bus_speed(void)
+__weak
+unsigned int i2c_get_bus_speed(void)
 {
 	return CONFIG_SYS_I2C_SPEED;
 }
-unsigned int i2c_get_bus_speed(void)
-	__attribute__((weak, alias("__def_i2c_get_bus_speed")));
 
-static int __def_i2c_set_bus_speed(unsigned int speed)
+__weak
+int i2c_set_bus_speed(unsigned int speed)
 {
 	if (speed != CONFIG_SYS_I2C_SPEED)
 		return -1;
 
 	return 0;
 }
-int i2c_set_bus_speed(unsigned int)
-	__attribute__((weak, alias("__def_i2c_set_bus_speed")));
 
 /*
  * get_alen: small parser helper function to get address length