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

[Stephen Warren]

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:
>>> rebased/reworked the I2C multibus patches from Simon Glass found
>>> here:
>>>
>>> http://www.mail-archive.com/u-boot@lists.denx.de/msg75530.html
>>>
>>> It seems the timing is coming, to bring this in mainline and
>>> move boards over to the new i2c framework. As an example I
>>> converted the soft-i2c driver (and all boards using it) to
>>> the new framework, so this patchseries has to be tested
>>> intensively, as I can check compile only ...
>>
>> I am very happy to see this, thank you.
> 
> I am too ;-) and Sorry that I am only now ready ...
> 
>> I have brought this in and tried to get it running for Tegra. A few
>> points:
>>
>> 1. The methods in struct i2c_adapter should IMO be passed a struct
>> i2c_adapter *, so they can determine which adapter is being accessed.
>> Otherwise I can't see how they would know.
> 
> They can get the current used adapter through the defines in
> include/i2c.h:
> [...]
> #define I2C_ADAP_NR(bus)        i2c_adap[i2c_bus[bus].adapter]
> #define I2C_BUS                 ((struct i2c_bus_hose *)gd->cur_i2c_bus)
> #define I2C_ADAP                i2c_adap[I2C_BUS->adapter]
> #define I2C_ADAP_HWNR           (I2C_ADAP->hwadapnr)
> 
> preparing just the fsl i2c driver and there I do for example:
> drivers/i2c/fsl_i2c.c
> [...]
> static const struct fsl_i2c *i2c_dev[2] = {
>         (struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_FSL_I2C_OFFSET),
> #ifdef CONFIG_SYS_FSL_I2C2_OFFSET
>         (struct fsl_i2c *) (CONFIG_SYS_IMMR + CONFIG_SYS_FSL_I2C2_OFFSET)
> #endif
> };
> [...]
> static int fsl_i2c_probe(uchar chip)
> {
>         struct fsl_i2c *dev = (struct fsl_i2c *)i2c_dev[I2C_ADAP_HWNR];
> [...]
> 
> but of course, we still can change the "struct i2c_adapter" if
> needed ... but we have one more parameter ... Ok, not really a bad
> problem.

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.