Hi Heiko,
On Wed, Jan 18, 2012 at 11:53 PM, Heiko Schocher <h...@denx.de> wrote:
> Hello Simon,
>
> Simon Glass wrote:
>> Hi Heiko,
>>
>> On Wed, Jan 18, 2012 at 10:35 PM, Heiko Schocher <h...@denx.de> wrote:
>>> Hello Wolfgang, Timur, Simon,
>>>
>>> Wolfgang Denk wrote:
>>>> Dear Simon Glass,
>>>>
>>>> In message
>>>> <capnjgz2klpfmzzwghkdj4el+wixqlv89+cvdvp7pccy+xfa...@mail.gmail.com> you
>>>> wrote:
>>>>>> I was really hoping we could get rid of the concept of a "current" i2c
>>>>>> adapter, and just force all drivers to specify the I2C adapter they
>>>>>> want to use for a given I2C operation. =A0That's how Linux operates, and
>>>>>> it will prevent stuff like this:
>>>>> I agree completely, it was one of the things I was going to ask for.
>>>>> We should add a new parameter to calls instead IMO.
>>>> Let's do one step at a time. Now we go for multi-bus support.
>>> Ok.
>>>
>>>> Implementing a new, better device interface is one of the next steps,
>>>> then.
>>> Some thoughts to the subject "get rid of concept of a current i2c"
>>>
>>> - First, it would be great to get rid of that ;-)
>>>
>>> - 2 reasons why we currently need the info, what is the current
>>> i2c adapter/i2c bus:
>>>
>>> - U-Boot principle says, don't init a device, if you don't use it.
>>> So, if we switch to another i2c adapter or i2c bus (A i2c bus is a
>>> combination if one i2c adpater and n i2c muxes), we must deinit
>>> the current adapter/bus. Ok, current code didn't this too, but
>>> this should a goal to keep in mind ... or we define, that this
>>> is not needed.
>>>
>>> - If we have i2c muxes, and you don't know your current i2c bus,
>>> you must on every i2c access also setup the i2c muxes on this
>>> i2c bus ... would we do this really?
>>
>> Ignoring muxes, we can have more than one i2c adaptor inited at a time
>> (i.e. de-initing is optional).
>
> Ok for the point "more adapters inited at one time".
>
> But we have to setup the muxes every time! We cannot ignore this.
>
>> Perhaps reword this slightly. U-Boot can have knowledge of a current
>> adaptor, mux settings and so on, and use this internally within the
>> i2c layer to optimise performance and redundant i2c traffic. But the
>
> Ok, thats what I mean. The "cur_i2c_bus" pointer should only be used
> in the i2c subsystem! This pointer is in gd_t because it must be
> writeable, also when running from flash ...
Yes that's fine.
>
>> pain is when the concept of a 'current adaptor' is exposed outside the
>> i2c layer.
>
> The "cur_i2c_bus" pointer is only used inside the i2c subsystem.
OK good.
>
>>> if we have the current i2c bus info, we just have to check, if
>>> we are on this bus, and do the i2c access ... if we are not on
>>> this i2c bus, we can deinit it complete (the new i2c_core disconnects
>>> for example all i2c muxes on the i2c bus) and init the new bus.
>>> All this work is done in a central function i2c_set_bus_numer()
>>
>> Yes but this function could become internal perhaps.
>
> Yes, if we make the change, Timur suggested ... but that can be done
> easy in a second step, as Wolfgang mentioned.
Yes indeed, and quite a major one by the sound of it.
>
>>> - Looking in the new multibus i2c_core.c file, we should get rid of
>>>
>>> static unsigned int i2c_cur_bus __attribute__((section(".data"))) =
>>> CONFIG_SYS_SPD_BUS_NUM;
>>>
>>> and "cur_adap" renamed to "cur_i2c_bus" and should be a
>>> "struct i2c_bus_hose" pointer.
>>
>> Sounds good. Heiko do you have time to take over your two patches I
>> sent and tidy them up, or should I continue?
>
> I did some work on this 2 patches, to get it working with soft_i2c.c
> driver ... I try to rework this suggestions in, and send it to the
> ML as a v2 ... if I do not find time, I speak up ;-)
Sounds good.
Regards,
Simon
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
