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

[Heiko Schocher]

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:
>>>> 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.

Yes, you are right, but just the same problem with current code!
You mixed here two things!

The idea behind the current i2c adapter was/is, that the i2c
core need to know, which bus is "current" because there is the
possibility that on one adapter are more i2c busses, because
of using i2c muxes ... and we must know, on which bus we are
currently, because if we want to switch to another bus, we must
first disable the old way (and maybe disable the i2c adapter too).
-> If we want this feature, we need a current adapter. If we say,
    Ok, we do not want this disabling... we can get rid of it, yes!

But I think it is safer to disable the i2c muxes, before
switching to another bus ... so this "current i2c adapter"
is an i2c core internal! We should of course change the i2c
API that we pass the bus to the i2c API, but, as I said this
to simon, this is another big change, and I want to have this
step after getting in the new i2c framework (and maybe hope,
that when we convert to mareks DM, we get this for free),
because we must also adapt for example all dtt, RTC, because
they need to know on which bus they resist, and we have to
config this ...

The problem from storing/restoring the "current cmdline i2c
bus", is another problem, which is an independent patch!
You can make with current code a patch, which holds the current
i2c cmdline bus in a variable, and then get rid of this store/
restore calls as for example found in cmd_date.c ...

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