diff mbox

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

Message ID 1318239351-10835-1-git-send-email-sbabic@denx.de
State Superseded
Headers show

Commit Message

Stefano Babic Oct. 10, 2011, 9:35 a.m. UTC
New default, weak i2c_set_bus_num() function.

Signed-off-by: Stefano Babic <sbabic@denx.de>
Cc: Heiko Schocher <hs@denx.de>
---
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 |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

Comments

Heiko Schocher Oct. 10, 2011, 10:33 a.m. UTC | #1
Hello Stefano,

Stefano Babic wrote:
> New default, weak i2c_set_bus_num() function.
> 
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> Cc: Heiko Schocher <hs@denx.de>
> ---
> 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 |    8 ++++++++
>  1 files changed, 8 insertions(+), 0 deletions(-)
> 
> diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
> index 3ea75f7..0f925d1 100644
> --- a/common/cmd_i2c.c
> +++ b/common/cmd_i2c.c
> @@ -158,6 +158,14 @@ int __def_i2c_set_bus_speed(unsigned int speed)
>  int i2c_set_bus_speed(unsigned int)
>  	__attribute__((weak, alias("__def_i2c_set_bus_speed")));
>  
> +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")));
> +
> +

Please just one new line ;-)

Beside of this, you get my:

Acked-by: Heiko Schocher <hs@denx.de>

>  /*
>   * get_alen: small parser helper function to get address length
>   * returns the address length

Thanks!

bye,
Heiko
Tabi Timur-B04825 Oct. 10, 2011, 5:53 p.m. UTC | #2
On Mon, Oct 10, 2011 at 4:35 AM, Stefano Babic <sbabic@denx.de> wrote:
> New default, weak i2c_set_bus_num() function.
>
> Signed-off-by: Stefano Babic <sbabic@denx.de>
> Cc: Heiko Schocher <hs@denx.de>

I would prefer to see a new set of I2C functions that take a bus
number as a parameter, so that we can eliminate i2c_set_bus_num() (and
the global variable it modifies) altogether.
Mike Frysinger Oct. 10, 2011, 6:26 p.m. UTC | #3
On Monday 10 October 2011 13:53:48 Tabi Timur-B04825 wrote:
> On Mon, Oct 10, 2011 at 4:35 AM, Stefano Babic <sbabic@denx.de> wrote:
> > New default, weak i2c_set_bus_num() function.
> > 
> > Signed-off-by: Stefano Babic <sbabic@denx.de>
> > Cc: Heiko Schocher <hs@denx.de>
> 
> I would prefer to see a new set of I2C functions that take a bus
> number as a parameter, so that we can eliminate i2c_set_bus_num() (and
> the global variable it modifies) altogether.

i think that'd require a much larger rework of the framework and thus would be 
better to do in addition to Stefano's work rather than in place of ?
-mike
Timur Tabi Oct. 10, 2011, 6:29 p.m. UTC | #4
Mike Frysinger wrote:
> i think that'd require a much larger rework of the framework and thus would be 
> better to do in addition to Stefano's work rather than in place of ?

Hmmm... I guess it would be easier to do the rework eventually if
i2c_set_bus_num() is universal, instead of just for PowerPC.
Heiko Schocher Oct. 11, 2011, 5:37 a.m. UTC | #5
Hello Tabi,

Tabi Timur-B04825 wrote:
> On Mon, Oct 10, 2011 at 4:35 AM, Stefano Babic <sbabic@denx.de> wrote:
>> New default, weak i2c_set_bus_num() function.
>>
>> Signed-off-by: Stefano Babic <sbabic@denx.de>
>> Cc: Heiko Schocher <hs@denx.de>
> 
> I would prefer to see a new set of I2C functions that take a bus
> number as a parameter, so that we can eliminate i2c_set_bus_num() (and
> the global variable it modifies) altogether.

Then please have a look at:

http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus_v2

It is a complete i2c rework with adding multiadapter/multibus
to the i2c framework (and a i2c_core.c) ... It needs a rebase and tests,
but my last tests (Ok, 6 months ago ... :-( ) on 2 powerpc architectures
and one arm looked good ...

bye,
Heiko
Wolfgang Denk Oct. 11, 2011, 5:48 a.m. UTC | #6
Dear Heiko Schocher,

In message <4E93D634.6080707@denx.de> you wrote:
> 
> Then please have a look at:
> 
> http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus_v2
> 
> It is a complete i2c rework with adding multiadapter/multibus
> to the i2c framework (and a i2c_core.c) ... It needs a rebase and tests,
> but my last tests (Ok, 6 months ago ... :-( ) on 2 powerpc architectures
> and one arm looked good ...

How about pulling this into mainline right now?  It makes little sense
to watch this bit-rotting even further.  Every interested party had
more than enough time for testing now.

Best regards,

Wolfgang Denk
Aaron Williams Oct. 11, 2011, 5:51 a.m. UTC | #7
On Monday, October 10, 2011 10:48:07 PM Wolfgang Denk wrote:
> Dear Heiko Schocher,
> 
> In message <4E93D634.6080707@denx.de> you wrote:
> > Then please have a look at:
> > 
> > http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multi
> > bus_v2
> > 
> > It is a complete i2c rework with adding multiadapter/multibus
> > to the i2c framework (and a i2c_core.c) ... It needs a rebase and tests,
> > but my last tests (Ok, 6 months ago ... :-( ) on 2 powerpc architectures
> > and one arm looked good ...
> 
> How about pulling this into mainline right now?  It makes little sense
> to watch this bit-rotting even further.  Every interested party had
> more than enough time for testing now.
> 
> Best regards,
> 
> Wolfgang Denk

This will be useful for us. We have a number of boards which use I2C switches 
and muxes plus many of our devices have multiple I2C buses.

-Aaron
Heiko Schocher Oct. 11, 2011, 5:52 a.m. UTC | #8
Hello Wolfgang,

Wolfgang Denk wrote:
> Dear Heiko Schocher,
> 
> In message <4E93D634.6080707@denx.de> you wrote:
>> Then please have a look at:
>>
>> http://git.denx.de/?p=u-boot/u-boot-i2c.git;a=shortlog;h=refs/heads/multibus_v2
>>
>> It is a complete i2c rework with adding multiadapter/multibus
>> to the i2c framework (and a i2c_core.c) ... It needs a rebase and tests,
>> but my last tests (Ok, 6 months ago ... :-( ) on 2 powerpc architectures
>> and one arm looked good ...
> 
> How about pulling this into mainline right now?  It makes little sense
> to watch this bit-rotting even further.  Every interested party had
> more than enough time for testing now.

Hmm... I had to rebase this and test it ... sorry, didn't found time
to hold this in sync with mainline in the last months ... and after
the rebase I only want to see this in mainline if tests on some
different archs are done (at least powerpc and arm) ...

bye,
Heiko
Stefano Babic Oct. 11, 2011, 7:33 a.m. UTC | #9
On 10/10/2011 08:26 PM, Mike Frysinger wrote:
> On Monday 10 October 2011 13:53:48 Tabi Timur-B04825 wrote:
>> On Mon, Oct 10, 2011 at 4:35 AM, Stefano Babic <sbabic@denx.de> wrote:
>>> New default, weak i2c_set_bus_num() function.
>>>
>>> Signed-off-by: Stefano Babic <sbabic@denx.de>
>>> Cc: Heiko Schocher <hs@denx.de>
>>
>> I would prefer to see a new set of I2C functions that take a bus
>> number as a parameter, so that we can eliminate i2c_set_bus_num() (and
>> the global variable it modifies) altogether.
> 
> i think that'd require a much larger rework of the framework and thus would be 
> better to do in addition to Stefano's work rather than in place of ?

Right - this patch is only due to the fact that the mxc_i2c.c has no
i2c_set_bus_num(), while it is present in several SOCs (not only powerpc).

I see now that the pmic patches calls i2c_set_bus_num() instead of the
macro I2C_SET_BUS(), that is protected in case CONFIG_I2C_MULTI_BUS is
not set (I have only now discovered...). Maybe I should drop my patch
and change the pmic to use this macro.

Regards,
Stefano
diff mbox

Patch

diff --git a/common/cmd_i2c.c b/common/cmd_i2c.c
index 3ea75f7..0f925d1 100644
--- a/common/cmd_i2c.c
+++ b/common/cmd_i2c.c
@@ -158,6 +158,14 @@  int __def_i2c_set_bus_speed(unsigned int speed)
 int i2c_set_bus_speed(unsigned int)
 	__attribute__((weak, alias("__def_i2c_set_bus_speed")));
 
+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")));
+
+
 /*
  * get_alen: small parser helper function to get address length
  * returns the address length