[U-Boot] [PATCH v2 13/40] i2c: Add high-level API

[Heiko Schocher]

Hello Simon,

Am 27.08.2014 21:10, schrieb Simon Glass:
> Hi Thierry,
>
> On 27 August 2014 05:41, Thierry Reding<thierry.reding at gmail.com>  wrote:
>> On Wed, Aug 27, 2014 at 11:56:41AM +0200, Heiko Schocher wrote:
>>> Hello Thierry,
>>>
>>> Am 27.08.2014 10:51, schrieb Thierry Reding:
>>>> On Wed, Aug 27, 2014 at 09:07:58AM +0200, Heiko Schocher wrote:
>>>>> Hello Thierry,
>>>>>
>>>>> Am 27.08.2014 08:21, schrieb Thierry Reding:
>>>>>> On Wed, Aug 27, 2014 at 07:21:51AM +0200, Heiko Schocher wrote:
>>>>>>> Hello Thierry,
>>>>>>>
>>>>>>> Am 26.08.2014 17:34, schrieb Thierry Reding:
>>>>>>>> From: Thierry Reding<treding at nvidia.com>
>>>>>>>>
>>>>>>>> This API operates on I2C adapters or I2C clients (a new type of object
>>>>>>>
>>>>>>> which is a bad idea ...
>>>>>>>
>>>>>>>> that refers to a particular slave connected to an adapter). This is
>>>>>>>> useful to avoid having to call i2c_set_bus_num() whenever a device is
>>>>>>>> being accessed.
>>>>>>>
>>>>>>> But thats the supose of i2c_set_bus_num()! ... if you use an i2c bus,
>>>>>>> you must check before every access, if you are on it, if not, you must
>>>>>>> switch back to it...
>>>>>>
>>>>>> That's not what code does today. Everybody calls i2c_set_bus_num() once
>>>>>> and then does a bunch of transactions on that bus. Given that U-Boot
>>>>>
>>>>> Yes, sadly. This has historical reasons ...
>>>>>
>>>>>> doesn't run multithreaded this works. If you're really concerned about
>>>>>
>>>>> Yes, U-Boot is singlethread only.
>>>>>
>>>>>> this being a problem, then it should be solved at a different level. It
>>>>>> could be part of i2c_client for example, so that i2c_client_read() and
>>>>>> i2c_client_write() would always perform this step. Burdening users with
>>>>>
>>>>> Exactly, thats right, and this is a goal from the CONFIG_SYS_I2C API!
>>>>>
>>>>> But why do you introduce i2c_client_read/write and do not add this step
>>>>> to i2c_read/write?
>>>>>
>>>>> - convert all i2c drivers, which are not yet converted to CONFIG_SYS_I2C
>>>>>    (get also rid od CONFIG_HARD_I2C)
>>>>> - add busnumber to i2c_read/write API and make i2c_set_bus_num() static ...
>>>>>    and fix all i2c_read/write() calls in U-Boot code ...
>>>>
>>>> I don't think adding a bus number as parameter is useful. Why not just
>>>> use the I2C adapter directly? That way we don't have to keep looking it
>>>> up in an array every time.
>>>
>>> You again just talk from i2c_adapter ... why you ignore i2c muxes?
>>> A bus is not only an i2c adapter ...
>>
>> I know. I keep saying i2c_adapter because in the rough sketch that I
>> have in mind for how this could work eventually, an mux is just another
>> special kind of i2c_adapter.
>>
>>> Currently we have two "versions" of i2c_adapter:
>>>
>>> In a system without muxes, you can say: i2c bus == i2c adapter
>>> but in a system with muxes we have:     i2c bus == i2c_bus_hose
>>>
>>> i2c commands use also a "bus number" starting from 0 ... the bus number
>>> has historical reasons... we could change this ...
>>>
>>> But if we introduce a new API, please with mux functionallity ...
>>> Hmm.. thinking about it ... if you want to introduce a new API, please
>>> start with using the DM for I2C!
>>
>> I can look into it, but it sounds like a task way beyond what I have
>> time for right now.
>>
>
> I can help if you are interested. I have patches for SPI at
> u-boot-dm.git (branch working) and would like to have I2C. They are
> sort-of similar so it might not be to hard to convert Tegra I2C over.
>
> The concept of a 'current' bus is broken IMO. Maybe the command line
> should have this concept, and maintain a 'current hose' or whatever.
> But drives and other code should specific what they need with each
> transaction. With DM that will likely be automatic.

That sounds great! I vote for using DM.

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