Bring in new I2C framework

+ * mux_id is the multiplexer chip type from defined in i2c.h. So far only
+ * NXP (Philips) PCA954x multiplexers are supported. Switches are NOT
+ * supported (anybody uses them?)

+int i2c_probe(u_int8_t chip)

From: Heiko Schocher <[email protected]>

This Patch introduce the new i2c_core file, which holds the I2C core functions, for the rework of the multibus/ multiadapter support.
Also adds changes in i2c.h for the new I2C multibus/multiadapter support. This new support can be activated with the CONFIG_SYS_I2C define.

Signed-off-by Heiko Schocher <[email protected]>

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

---

?arch/arm/include/asm/global_data.h ? ? ? ?| ? ?3 +
?arch/avr32/include/asm/global_data.h ? ? ?| ? ?3 +
?arch/blackfin/include/asm/global_data.h ? | ? ?4 +-
?arch/m68k/include/asm/global_data.h ? ? ? | ? ?3 +
?arch/microblaze/include/asm/global_data.h | ? ?3 +
?arch/mips/include/asm/global_data.h ? ? ? | ? ?3 +
?arch/nios2/include/asm/global_data.h ? ? ?| ? ?3 +
?arch/powerpc/include/asm/global_data.h ? ?| ? ?3 +
?arch/sh/include/asm/global_data.h ? ? ? ? | ? ?3 +
?arch/sparc/include/asm/global_data.h ? ? ?| ? ?3 +
?arch/x86/include/asm/global_data.h ? ? ? ?| ? ?3 +
?drivers/i2c/Makefile ? ? ? ? ? ? ? ? ? ? ?| ? ?1 +
?drivers/i2c/i2c_core.c ? ? ? ? ? ? ? ? ? ?| ?360 +++++++++++++++++++++++++++++
?include/i2c.h ? ? ? ? ? ? ? ? ? ? ? ? ? ? | ?199 +++++++++++++++-

?14 files changed, 584 insertions(+), 10 deletions(-) ?create mode 100644 drivers/i2c/i2c_core.c

arch/arm/include/asm/global_data.h +0 -0

index 23a6077..924cea2 100644
@@ -87,6 +87,9 @@ typedef ? ? ? struct ?global_data {

? ? ? ?unsigned long ? post_log_res; /* success of POST test */ ? ? ? ?unsigned long ? post_init_f_time; /* When post_init_f started */ ?#endif

arch/arm/include/asm/global_data.h typedef struct global_data { · line 92

Quoted 9 lines · Simon Glass

arch/arm/include/asm/global_data.h +3 -0

@@ -87,6 +87,9 @@ typedef       struct  global_data {
       unsigned long   post_log_res; /* success of POST test */
       unsigned long   post_init_f_time; /* When post_init_f started */
 #endif
+#if defined(CONFIG_SYS_I2C)
+       void            *cur_adap;      /* current used i2c adapter */
+#endif

I was really hoping we could get rid of the concept of a "current" i2c adapter, and just force all drivers to specify the I2C adapter they want to use for a given I2C operation. That's how Linux operates, and it will prevent stuff like this:

void *old;
void *new;

old = get_current_i2c_adapter();
set_i2c_adapter(new);
// do I2C stuff
set_i2c_adapter(old);

that's only needed if you expect the pointer to stay valid across calls. i don't think it does for most (all?) drivers.

Quoted 29 lines · Simon Glass

From: Heiko Schocher <[email protected]>

This Patch introduce the new i2c_core file, which holds the I2C core functions, for the rework of the multibus/ multiadapter support.
Also adds changes in i2c.h for the new I2C multibus/multiadapter support. This new support can be activated with the CONFIG_SYS_I2C define.

Signed-off-by Heiko Schocher <[email protected]>

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

---

?arch/arm/include/asm/global_data.h ? ? ? ?| ? ?3 +
?arch/avr32/include/asm/global_data.h ? ? ?| ? ?3 +
?arch/blackfin/include/asm/global_data.h ? | ? ?4 +-
?arch/m68k/include/asm/global_data.h ? ? ? | ? ?3 +
?arch/microblaze/include/asm/global_data.h | ? ?3 +
?arch/mips/include/asm/global_data.h ? ? ? | ? ?3 +
?arch/nios2/include/asm/global_data.h ? ? ?| ? ?3 +
?arch/powerpc/include/asm/global_data.h ? ?| ? ?3 +
?arch/sh/include/asm/global_data.h ? ? ? ? | ? ?3 +
?arch/sparc/include/asm/global_data.h ? ? ?| ? ?3 +
?arch/x86/include/asm/global_data.h ? ? ? ?| ? ?3 +
?drivers/i2c/Makefile ? ? ? ? ? ? ? ? ? ? ?| ? ?1 +
?drivers/i2c/i2c_core.c ? ? ? ? ? ? ? ? ? ?| ?360 +++++++++++++++++++++++++++++
?include/i2c.h ? ? ? ? ? ? ? ? ? ? ? ? ? ? | ?199 +++++++++++++++-

?14 files changed, 584 insertions(+), 10 deletions(-) ?create mode 100644 drivers/i2c/i2c_core.c

arch/arm/include/asm/global_data.h typedef struct global_data { · line 92

Quoted 11 lines

arch/arm/include/asm/global_data.h +0 -0

index 23a6077..924cea2 100644
@@ -87,6 +87,9 @@ typedef ? ? ? struct ?global_data {

? ? ? ?unsigned long ? post_log_res; /* success of POST test */ ? ? ? ?unsigned long ? post_init_f_time; /* When post_init_f started */ ?#endif

arch/arm/include/asm/global_data.h +3 -0

+#if defined(CONFIG_SYS_I2C)
+ ? ? ? void ? ? ? ? ? ?*cur_adap; ? ? ?/* current used i2c adapter */
+#endif

I was really hoping we could get rid of the concept of a "current" i2c adapter, and just force all drivers to specify the I2C adapter they want to use for a given I2C operation. ?That's how Linux operates, and it will prevent stuff like this:

void *old;
void *new;

old = get_current_i2c_adapter();
set_i2c_adapter(new);
// do I2C stuff
set_i2c_adapter(old);

--
Timur Tabi
Linux kernel developer at Freescale

I agree completely, it was one of the things I was going to ask for. We should add a new parameter to calls instead IMO.

Quoted 2 lines · Simon Glass

I agree completely, it was one of the things I was going to ask for. We should add a new parameter to calls instead IMO.

I would be in favor of a patch that replaces all of the I2C calls. ?It would be a massive patch, but it solve a lot of problems in one shot.

--
Timur Tabi
Linux kernel developer at Freescale

I agree. Do you know of such a patch? :-)

Quoted 5 lines

I was really hoping we could get rid of the concept of a "current" i2c adapter, and just force all drivers to specify the I2C adapter they want to use for a given I2C operation. =A0That's how Linux operates, and it will prevent stuff like this:

I agree completely, it was one of the things I was going to ask for. We should add a new parameter to calls instead IMO.

Dear Simon Glass,

Quoted 6 lines · In message <CAPnjgZ2kLPfMzzwgHkDJ4eL+wixqLv89+CvdVP7PCcy+XFaqNQ@mail.gmail.com>…

Quoted 4 lines

I was really hoping we could get rid of the concept of a "current" i2c adapter, and just force all drivers to specify the I2C adapter they want to use for a given I2C operation. =A0That's how Linux operates, and it will prevent stuff like this:

I agree completely, it was one of the things I was going to ask for. We should add a new parameter to calls instead IMO.

Let's do one step at a time. Now we go for multi-bus support.

Implementing a new, better device interface is one of the next steps, then.

Hello Wolfgang, Timur, Simon,

Quoted 11 lines · Wolfgang Denk

Dear Simon Glass,

Quoted 6 lines · In message <CAPnjgZ2kLPfMzzwgHkDJ4eL+wixqLv89+CvdVP7PCcy+XFaqNQ@mail.gmail.com>…

Quoted 4 lines

I was really hoping we could get rid of the concept of a "current" i2c adapter, and just force all drivers to specify the I2C adapter they want to use for a given I2C operation. =A0That's how Linux operates, and it will prevent stuff like this:

I agree completely, it was one of the things I was going to ask for. We should add a new parameter to calls instead IMO.

Let's do one step at a time. ?Now we go for multi-bus support.

Ok.

Quoted 2 lines

Implementing a new, better device interface is one of the next steps, then.

Some thoughts to the subject "get rid of concept of a current i2c"

- First, it would be great to get rid of that ;-)

- 2 reasons why we currently need the info, what is the current ?i2c adapter/i2c bus:

- U-Boot principle says, don't init a device, if you don't use it. ?So, if we switch to another i2c adapter or i2c bus (A i2c bus is a ?combination if one i2c adpater and n i2c muxes), we must deinit ?the current adapter/bus. Ok, current code didn't this too, but ?this should a goal to keep in mind ... or we define, that this ?is not needed.

- If we have i2c muxes, and you don't know your current i2c bus, ?you must on every i2c access also setup the i2c muxes on this ?i2c bus ... would we do this really?

?if we have the current i2c bus info, we just have to check, if ?we are on this bus, and do the i2c access ... if we are not on ?this i2c bus, we can deinit it complete (the new i2c_core disconnects ?for example all i2c muxes on the i2c bus) and init the new bus. ?All this work is done in a central function i2c_set_bus_numer()

- Looking in the new multibus i2c_core.c file, we should get rid of

?static unsigned int i2c_cur_bus __attribute__((section(".data"))) = ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?CONFIG_SYS_SPD_BUS_NUM;

?and "cur_adap" renamed to "cur_i2c_bus" and should be a ?"struct i2c_bus_hose" pointer.

bye,
Heiko
--
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Hi Heiko,

Quoted 31 lines · Heiko Schocher

Hello Wolfgang, Timur, Simon,

Quoted 10 lines · Wolfgang Denk

Dear Simon Glass,

Quoted 6 lines · In message <CAPnjgZ2kLPfMzzwgHkDJ4eL+wixqLv89+CvdVP7PCcy+XFaqNQ@mail.gmail.com>…

Quoted 4 lines

I was really hoping we could get rid of the concept of a "current" i2c adapter, and just force all drivers to specify the I2C adapter they want to use for a given I2C operation. =A0That's how Linux operates, and it will prevent stuff like this:

I agree completely, it was one of the things I was going to ask for. We should add a new parameter to calls instead IMO.

Let's do one step at a time. Now we go for multi-bus support.

Ok.

Quoted 2 lines

Implementing a new, better device interface is one of the next steps, then.

Some thoughts to the subject "get rid of concept of a current i2c"

- First, it would be great to get rid of that ;-)

- 2 reasons why we currently need the info, what is the current i2c adapter/i2c bus:

- U-Boot principle says, don't init a device, if you don't use it. So, if we switch to another i2c adapter or i2c bus (A i2c bus is a combination if one i2c adpater and n i2c muxes), we must deinit the current adapter/bus. Ok, current code didn't this too, but this should a goal to keep in mind ... or we define, that this is not needed.

Quoted 3 lines

- If we have i2c muxes, and you don't know your current i2c bus, you must on every i2c access also setup the i2c muxes on this i2c bus ... would we do this really?

Ignoring muxes, we can have more than one i2c adaptor inited at a time (i.e. de-initing is optional).

Perhaps reword this slightly. U-Boot can have knowledge of a current adaptor, mux settings and so on, and use this internally within the i2c layer to optimise performance and redundant i2c traffic. But the

pain is when the concept of a 'current adaptor' is exposed outside the i2c layer.

Quoted 5 lines

if we have the current i2c bus info, we just have to check, if we are on this bus, and do the i2c access ... if we are not on this i2c bus, we can deinit it complete (the new i2c_core disconnects for example all i2c muxes on the i2c bus) and init the new bus. All this work is done in a central function i2c_set_bus_numer()

Yes but this function could become internal perhaps.

Quoted 7 lines

- Looking in the new multibus i2c_core.c file, we should get rid of

static unsigned int i2c_cur_bus __attribute__((section(".data"))) =

                               CONFIG_SYS_SPD_BUS_NUM;

and "cur_adap" renamed to "cur_i2c_bus" and should be a "struct i2c_bus_hose" pointer.

Sounds good. Heiko do you have time to take over your two patches I sent and tidy them up, or should I continue?

Hello Simon,

Quoted 34 lines · Simon Glass

Hi Heiko,

Quoted 31 lines · Heiko Schocher

Hello Wolfgang, Timur, Simon,

Quoted 10 lines · Wolfgang Denk

Dear Simon Glass,

Quoted 6 lines · In message <CAPnjgZ2kLPfMzzwgHkDJ4eL+wixqLv89+CvdVP7PCcy+XFaqNQ@mail.gmail.com>…

Quoted 4 lines

I was really hoping we could get rid of the concept of a "current" i2c adapter, and just force all drivers to specify the I2C adapter they want to use for a given I2C operation. =A0That's how Linux operates, and it will prevent stuff like this:

I agree completely, it was one of the things I was going to ask for. We should add a new parameter to calls instead IMO.

Let's do one step at a time. ?Now we go for multi-bus support.

Ok.

Quoted 2 lines

Implementing a new, better device interface is one of the next steps, then.

Some thoughts to the subject "get rid of concept of a current i2c"

- First, it would be great to get rid of that ;-)

- 2 reasons why we currently need the info, what is the current ?i2c adapter/i2c bus:

- U-Boot principle says, don't init a device, if you don't use it. ?So, if we switch to another i2c adapter or i2c bus (A i2c bus is a ?combination if one i2c adpater and n i2c muxes), we must deinit ?the current adapter/bus. Ok, current code didn't this too, but ?this should a goal to keep in mind ... or we define, that this ?is not needed.

Quoted 6 lines

Quoted 3 lines

- If we have i2c muxes, and you don't know your current i2c bus, ?you must on every i2c access also setup the i2c muxes on this ?i2c bus ... would we do this really?

Ignoring muxes, we can have more than one i2c adaptor inited at a time (i.e. de-initing is optional).

Ok for the point "more adapters inited at one time".

But we have to setup the muxes every time! We cannot ignore this.

Quoted 3 lines

Perhaps reword this slightly. U-Boot can have knowledge of a current adaptor, mux settings and so on, and use this internally within the i2c layer to optimise performance and redundant i2c traffic. But the

Ok, thats what I mean. The "cur_i2c_bus" pointer should only be used in the i2c subsystem! This pointer is in gd_t because it must be writeable, also when running from flash ...

Quoted 3 lines

pain is when the concept of a 'current adaptor' is exposed outside the i2c layer.

The "cur_i2c_bus" pointer is only used inside the i2c subsystem.

Quoted 8 lines

Quoted 6 lines

?if we have the current i2c bus info, we just have to check, if ?we are on this bus, and do the i2c access ... if we are not on ?this i2c bus, we can deinit it complete (the new i2c_core disconnects ?for example all i2c muxes on the i2c bus) and init the new bus. ?All this work is done in a central function i2c_set_bus_numer()

Yes but this function could become internal perhaps.

Yes, if we make the change, Timur suggested ... but that can be done easy in a second step, as Wolfgang mentioned.

Quoted 9 lines

Quoted 8 lines

- Looking in the new multibus i2c_core.c file, we should get rid of

?static unsigned int i2c_cur_bus __attribute__((section(".data"))) = ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?CONFIG_SYS_SPD_BUS_NUM;

?and "cur_adap" renamed to "cur_i2c_bus" and should be a ?"struct i2c_bus_hose" pointer.

Quoted 2 lines

Sounds good. Heiko do you have time to take over your two patches I sent and tidy them up, or should I continue?

I did some work on this 2 patches, to get it working with soft_i2c.c driver ... I try to rework this suggestions in, and send it to the ML as a v2 ... if I do not find time, I speak up ;-)

bye,
Heiko
--
DENX Software Engineering GmbH, ? ? MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Perhaps reword this slightly. U-Boot can have knowledge of a current adaptor, mux settings and so on, and use this internally within the i2c layer to optimise performance and redundant i2c traffic. But the pain is when the concept of a 'current adaptor' is exposed outside the i2c layer.

Dear Simon Glass,

Quoted 6 lines · In message <CAPnjgZ1RoyguaAScOQiw8PoQkzVp5rim7KJ6zzHLNPwuBYK48Q@mail.gmail.com>…

Perhaps reword this slightly. U-Boot can have knowledge of a current adaptor, mux settings and so on, and use this internally within the i2c layer to optimise performance and redundant i2c traffic. But the pain is when the concept of a 'current adaptor' is exposed outside the i2c layer.

As mentioned before, this is what we currently have as "device model" in U-Boot - not only I2C: we have the same "current device" concept with IDE, USB, ...

It makes no sense trying to change it here; it would only cause even more incompatibilities. ?If you want to fix that, then help working on the new driver model. ?Marek is planning for this.

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 Good morning. This is the telephone company. Due ?to ?repairs, ?we're giving ?you ?advance notice that your service will be cut off indefi- nitely at ten o'clock. That's two minutes from now.

As mentioned before, this is what we currently have as "device model" in U-Boot - not only I2C: we have the same "current device" concept with IDE, USB, ...

Quoted 3 lines · Wolfgang Denk

As mentioned before, this is what we currently have as "device model" in U-Boot - not only I2C: we have the same "current device" concept with IDE, USB, ...

The difference is that I2C operations are typically done internally by other code, whereas IDE, USB, etc are done by the user on the command line. It's not unusual for boot code to access multiple I2C devices on different buses, so we're switching I2C buses a lot. People generally don't try to access two networks or two USB devices back-to-back, but that's exactly what we do with I2C.

The other problem is that I2C operations are necessary prior to relocation, but IDE, USB, etc generally are not. That's why we have this:

static unsigned int i2c_bus_num __attribute__ ((section (".data"))) = CONFIG_SYS_SPD_BUS_NUM;

We need to initialize i2c_bus_num to the I2C bus that SPD is on, because i2c_bus_num is not writable until after relocation, and DDR initialization requires I2C.

A board that has SPD on two different I2C buses could not be supported by U-Boot today.

This is just a rough patch to show how this might be done.

Not to be applied, please.

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

---
 drivers/i2c/tegra2_i2c.c   |   53 +++++++++++++++++++------------------------
 include/configs/seaboard.h |    2 +
 2 files changed, 25 insertions(+), 30 deletions(-)

drivers/i2c/tegra2_i2c.c +2 -0

index b42d9ac..93f3269 100644
@@ -30,7 +30,9 @@
 #include <asm/arch/gpio.h>
 #include <asm/arch/pinmux.h>
 #include <asm/arch/tegra2_i2c.h>
+
 #include <fdtdec.h>
+#include <i2c.h>
 
 DECLARE_GLOBAL_DATA_PTR;
 

drivers/i2c/tegra2_i2c.c +1 -1

 {
 	struct i2c_bus *i2c_bus;
 

@@ -309,7 +306,7 @@ int i2c_set_bus_speed(unsigned int speed)
 	i2c_bus->speed = speed;
 	i2c_init_controller(i2c_bus);
 
-	return 0;
+	return 0;	/* TODO: return actual speed */
 }
 
 static int i2c_get_config(const void *blob, int node, struct i2c_bus *i2c_bus)

This series provides Heiko's upgraded I2C framework from a few years ago. I hope that we can bring this in and move boards over to it as time permits, rather than switching everything in one fell swoop which never happens.

To show it working I have enabled it for Tegra in a very rough way. It seems fine with my limited testing.

In terms of changes, I have just fixed some checkpatch errors and fiddled with a couple of function signatures.

I will start a thread on the list with a few thoughts on this series at some point.