[U-Boot,V3,05/13] i2c: Create common default i2c_set_bus_num() function

New default, weak i2c_set_bus_num() function.

Signed-off-by: Stefano Babic <sbabic@denx.de>
Cc: Heiko Schocher <hs@denx.de>
---
Changes since V2:

- codestyling: drop newline (Heiko Schocher)

Changes since V1:

- add a weak function i2c_set_bus_num() to cmd_i2c.c instead of adding a
dummy function to mxc_i2c.c. (Heiko Schocher)


 common/cmd_i2c.c |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)

On Monday 10 October 2011 06:50:12 Stefano Babic wrote:
> +int __def_i2c_set_bus_num(unsigned int bus)
> +{
> +	return 0;
> +}
> +int i2c_set_bus_num(unsigned int)
> +	__attribute__((weak, alias("__def_i2c_set_bus_num")));

any reason this can't just be:
__weak int i2c_set_bus_num(unsigned int bus)
{
	return 0;
}

i can understand having a default func when it actually does something, but i 
don't see much value in a stub that returns 0
-mike

On 10/10/2011 05:19 PM, Mike Frysinger wrote:
> On Monday 10 October 2011 06:50:12 Stefano Babic wrote:

Hi Mike,

>> +int __def_i2c_set_bus_num(unsigned int bus)
>> +{
>> +	return 0;
>> +}
>> +int i2c_set_bus_num(unsigned int)
>> +	__attribute__((weak, alias("__def_i2c_set_bus_num")));
> 
> any reason this can't just be:
> __weak int i2c_set_bus_num(unsigned int bus)
> {
> 	return 0;
> }
> 
> i can understand having a default func when it actually does something, but i 
> don't see much value in a stub that returns 0

It seems to me that this is the commonly used style in u-boot. The same
happens for the i2c_init_board, some lines before, and in a lot of other
modules:

void __def_i2c_init_board(void)
{
        return;
}
void i2c_init_board(void)
        __attribute__((weak, alias("__def_i2c_init_board")));

Simply grepping in u-boot code, the default function is defined even if
it does nothing,

Regards,
Stefano

On Monday 10 October 2011 11:31:39 Stefano Babic wrote:
> On 10/10/2011 05:19 PM, Mike Frysinger wrote:
> > On Monday 10 October 2011 06:50:12 Stefano Babic wrote:
> >> +int __def_i2c_set_bus_num(unsigned int bus)
> >> +{
> >> +	return 0;
> >> +}
> >> +int i2c_set_bus_num(unsigned int)
> >> +	__attribute__((weak, alias("__def_i2c_set_bus_num")));
> > 
> > any reason this can't just be:
> > __weak int i2c_set_bus_num(unsigned int bus)
> > {
> > 
> > 	return 0;
> > 
> > }
> > 
> > i can understand having a default func when it actually does something,
> > but i don't see much value in a stub that returns 0
> 
> It seems to me that this is the commonly used style in u-boot. The same
> happens for the i2c_init_board, some lines before, and in a lot of other
> modules:
> 
> void __def_i2c_init_board(void)
> {
>         return;
> }
> void i2c_init_board(void)
>         __attribute__((weak, alias("__def_i2c_init_board")));
> 
> Simply grepping in u-boot code, the default function is defined even if
> it does nothing,

i believe we have two standards.  i'm not sure which gets used more, or if 
it's just a wash.  i think it makes sense to have an accessible default func 
if it does something useful, but not if it's just a stub.

in the __def/weak style above, unless people use --gc-sections when linking (i 
think we've got many people doing this now, but not all), you end up with dead 
code in the binary.  in the code i proposed, the func gets discarded both when 
using --gc-sections and when not.
-mike

On 10/10/2011 07:51 PM, Mike Frysinger wrote:

Hi Mike,

> i believe we have two standards.

Well, two standards means we have  no standard...;-)

>  i'm not sure which gets used more, or if 
> it's just a wash.  i think it makes sense to have an accessible default func 
> if it does something useful, but not if it's just a stub.
> 
> in the __def/weak style above, unless people use --gc-sections when linking (i 
> think we've got many people doing this now, but not all), you end up with dead 
> code in the binary.  in the code i proposed, the func gets discarded both when 
> using --gc-sections and when not.

You are right, but probably it makes no difference. I checked and it
seems that only a few architectute (m68k, sparc and microblaze) do not
set --gc-sections.

After the Heiko's post and our discussion, I think that this patch is
another work-around waiting for the Heiko's patches to go to mainline.
And it becomes a second work-around, because there is already the
I2C_SET_BUS() for boards supporting only one bus. Two work-around are
too much...

I drop this patch and I will make use of I2C_SET_BUS() in pmic. This is
coherent with the actual u-boot code.

Regards,
Stefano

On Tuesday 11 October 2011 13:30:24 Stefano Babic wrote:
> On 10/10/2011 07:51 PM, Mike Frysinger wrote:
> >  i'm not sure which gets used more, or if
> > it's just a wash.  i think it makes sense to have an accessible default
> > func if it does something useful, but not if it's just a stub.
> > 
> > in the __def/weak style above, unless people use --gc-sections when
> > linking (i think we've got many people doing this now, but not all), you
> > end up with dead code in the binary.  in the code i proposed, the func
> > gets discarded both when using --gc-sections and when not.
> 
> You are right, but probably it makes no difference. I checked and it
> seems that only a few architectute (m68k, sparc and microblaze) do not
> set --gc-sections.

i would think the microblaze missing would be an oversight as it's "newer" 
code.  don't know (or honestly, care) how hard it would be to make m68k/sparc 
sane.  but it would be nice to get everyone using gc-sections as we could move 
that into common code and all new arches would start out sane ...
-mike