[U-Boot] [PATCH 0/3] Bring in new I2C framework

Heiko Schocher hs at denx.de
Tue Oct 30 06:57:08 CET 2012


Hello Stephen,

On 29.10.2012 16:34, Stephen Warren wrote:
> On 10/29/2012 03:47 AM, Heiko Schocher wrote:
>> Hello Stephen,
>>
>> On 26.10.2012 18:07, Stephen Warren wrote:
>>> On 10/25/2012 11:48 PM, Heiko Schocher wrote:
>>>> Hello Simon,
>>>>
>>>> On 25.10.2012 23:37, Simon Glass wrote:
>>>>> On Mon, Oct 22, 2012 at 10:40 AM, Heiko Schocher<hs at denx.de>    wrote:
[...]
>>> That rather relies on their being a concept of a "current" I2C adapter.
>>> It seems a little limiting to require that. What if the "current"
>>> adapter is the user-selected adapter for commands to operate on, but
>>> e.g. some power-management driver wants to use I2C to communicate with a
>>> PMIC during the internals of some other command. Sure, you could save
>>> and later restore the I2C core's idea of "current" adapter, but it'd
>>> surely be cleaner to just pass around the I2C adapter ID or struct
>>> pointer everywhere to avoid the need for save/restore.
>>
>> Yes, you are right, but just the same problem with current code!
>> You mixed here two things!
>
> I think you're reading more into what I was saying than what I actually
> said.

Sorry, maybe I misunderstood you ...

> If there are e.g. 4 I2C controllers in an SoC, the driver needs to know
> which one is in use. Passing that information directly to the driver
> functions is much simple than requiring the SoC I2C driver to go grovel
> in some I2C core global variables to find out the same information.

Ah, do you mean we should change the i2c adapter struct from:

struct i2c_adapter {
        void            (*init)(int speed, int slaveaddr);
        int             (*probe)(uint8_t chip);
        int             (*read)(uint8_t chip, uint addr, int alen,
                                uint8_t *buffer, int len);
        int             (*write)(uint8_t chip, uint addr, int alen,
                                uint8_t *buffer, int len);
        uint            (*set_bus_speed)(uint speed);
[...]

to

struct i2c_adapter {
        void            (*init)(struct i2c_adapter *adap, int speed, int slaveaddr);
        int             (*probe)(struct i2c_adapter *adap, uint8_t chip);
        int             (*read)(struct i2c_adapter *adap, uint8_t chip, uint addr, int alen,
                                uint8_t *buffer, int len);
        int             (*write)(struct i2c_adapter *adap, uint8_t chip, uint addr, int alen,
                                uint8_t *buffer, int len);
        uint            (*set_bus_speed)(struct i2c_adapter *adap, uint speed);
[...]
?

We can do this. Simon suggested this too ...

@Simon: Is this Ok with you?

Nevertheless, we need a "cur_i2c_bus" pointer, as the i2c core, needs
to know the current i2c bus for detecting if the bus, which whould be
accessed is the current or not and a switching to another bus is needed.

> This is all unrelated to I2C bus muxes; they shouldn't be implemented as
> part of an SoC I2C driver anyway, so the driver shouldn't know about bus
> muxes before or after this patch - the I2C core should manage that.

Exactly! And that do the new i2c framework in i2c_core.c!

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


More information about the U-Boot mailing list