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

[Simon Glass]

Hi Heiko,

On Mon, Oct 29, 2012 at 2:44 AM, Heiko Schocher <hs at denx.de> wrote:
> Hello Simon,
>
>
> On 26.10.2012 18:08, Simon Glass wrote:
>>
>> On Thu, Oct 25, 2012 at 10:48 PM, Heiko Schocher<hs at denx.de>  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:
>
> [...]
>
>>>> 2. The init is a bit odd, in that we can call init() repeatedly.
>>>> Perhaps that function should be renamed to reset, and the init should
>>>> be used for calling just once at the start?
>>>
>>>
>>>
>>> What do you mean here exactly? I couldn´t parse this ...
>>
>>
>> Well there is start-of-day setup, which I think should be called init.
>> This is done once on boot for each i2c adapter.
>
>
> Hmm... I am not sure if this is only done on boot, because we should
> "close" or "deinit" an adapter if not used any more in U-Boot as the
> U-Boot principle says:
>
> http://www.denx.de/wiki/view/U-Boot/DesignPrinciples#2_Keep_it_Fast
>
> So I want to add in future some "deinit" to every adapter, and
> call it from i2c_set_bus() when switching to another i2c adapter ...

Well deinit() should be probably be done before finishing U-Boot, not
after every transaction, since you don't know that the current
transaction will be the last.

When using FDT, you need to look up the available i2c ports in the
driver, and this should be done once at the start. If the i2c core can
call a suitable init function then this is easier, rather than us
having to keep track of whether the init is done or not.

>
>
>> And then there is the i2c_init() which seems to be called whenever we
>> feel like it - e.g. to change speed. I suggest that we use init() to
>> mean start-of-day init and reset(), or similar, to mean any post-init.
>> I am not suggest that for this series, just as a future effort.
>
>
> Yes, that should be changed. We do not need an init() in the i2c
> API, as i2c_set_bus_num() do this for us (and later also the deinit())
>
> We just need a set/get_speed() and a deblock()/reset() ?
>
> Maybe a step in the API cleanup?

Yes, a future step I think. I feel that i2c is one of the darker
corners of U-Boot and so am keen to get this tidied up.

>
>
>>>> 3. Rather than each device having to call i2c_get_bus_num() to find
>>>> out what bus is referred to, why not just pass this information in? In
>>>> fact the adapter pointer can serve for this.
>>>
>>>
>>>
>>> Not the "struct i2c_adapter" must passed, but the "struct
>>> i2c_bus"!
>>>
>>> And each device should know, which i2c bus it uses, or? So at
>>> the end we should have something like i2c_read(struct i2c_bus *bus, ...)
>>> calls ... and the i2c core can detect, if this bus is the
>>> current, if so go on, if not switch to this bus. So at the
>>> end i2c_set_bus_num would be go static ...
>>>
>>>
>>>> 4. So perhaps the i2c read/write functions should change from:
>>>>
>>>> int i2c_read(uchar chip, uint addr, int alen, uchar *buffer, int len)
>>>>
>>>> to:
>>>>
>>>> int i2c_read((struct i2c_adapter *adapter, uint addr, int alen, uchar
>>>> *buffer, int len)
>>>
>>>
>>>
>>> Yep, exactly, see comments to point 3 ...
>>>
>>> That would be the best (and I think in previous discussions I defined
>>> this as one goal ...), but this would be (another) big change,
>>> because this is an *API* change, with maybe a lot of config file
>>> changes ...
>>>
>>> Let us bring in the new i2c framework with all i2c drivers converted,
>>> and then do the next step ... maybe one step more, if mareks device
>>> model is ready, we can switch easy to DM ... and maybe get this API
>>> change for free ...
>>
>>
>> Yes. I certainly understand the need to fit in with what is already
>> there, and avoid a massive API change, which can be performed as a
>> follow-on patch anyway. With your info above I will adjust the tegra
>> driver to work with this and test it.
>
>
> Ok, great! So I post v2 patches after you tested ...
>
> And Yes, we should do this API change, but I tend to do this step after
> the new i2c framework is stable and all i2c drivers are converted to it ...

[snip]

Regards,
Simon

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