[U-Boot] [PATCH] 7/12 Multiadapter/multibus I2C, drivers part 4

[Heiko Schocher <hs@denx.de>]

Hello ksi,

ksi at koi8.net wrote:
> On Wed, 18 Feb 2009, Heiko Schocher wrote:
>> Heiko Schocher schrieb:
>>> ksi at koi8.net wrote:
>>>> On Mon, 16 Feb 2009, Wolfgang Denk wrote:
>>>>
>>>>> Dear ksi at koi8.net,
>>>>>
>>>>> In message <Pine.LNX.4.64ksi.0902142019520.6240@home-gw.koi8.net> you wrote:
>>> [...]
>>>>>> And remember, the devil is in details. How are you going to assign
>>>>>> (initialize) that innocent looking "cur_adap_nr->hwadapnr"? How are you
>>>>>> going to work on an adapter other that "current" in a situation when you can
>>>>>> NOT change "current" adapter (e.g. perform all I2C layer initialization
>>>>>> while still running from flash?) Remember, this is plain C and there is no
>>>>> What makes you insist that we cannot change a variable if we need to
>>>>> be able to change one?
>>>> It is NOT just variable. My approach uses i2c _BUS_, not _ADAPTER_. And
>>>> number of busses can be bigger than number of adapters (e.g. when some
>>>> busses a reached via muxes or switches.) When doing i2c_set_current_bus()
>>>> you are switching _NOT_ adapters, but busses. That involves not only
>>>> changing that global variable but also reprogramming muxes/switches for
>>>> i2c_set_current_bus() to be consistent and hardware independent. Otherwise
>>> You have no i2c_set_current_bus() in your code! I think you mean
>>> i2c_set_current_bus(), right?
>>>
>>> And this function fails when running from flash! So, how can you switch
>>> busses with your patches when running from flash?

PLease answer my question!!!

How can you switch busses when running from flash???

>>> Here your function:
>>>
>>> int i2c_set_bus_num(unsigned int bus)
>>> {
>>> #ifndef CONFIG_SYS_I2C_DIRECT_BUS
>>>         int             i;
>>>         u_int8_t        buf;
>>> #endif
>>>
>>>         if ((bus >= CONFIG_SYS_NUM_I2C_BUSSES) || !(gd->flags & GD_FLG_RELOC))
>>>                 return(-1);
>>> [...]
>>>
>>> This function wouldn;t work from flash ...
>> And one more reason, why your function will not work from flash:
>>
>> later in your i2c_set_bus_num function:
>> [...]
>>         if ((i2c_bus[i2c_cur_bus].next_hop[0].chip != 0) &&
>>                         (ADAP(i2c_cur_bus)->init_done != 0)) {
>>
>> You only switch muxes if init_done != 0 ... but init_done is
>> not writeable when running from flash, so it is "= 0" when
>> code is not relocated ... how work this for you?

Again, how work this!

>> But this can be solved, if we always init the hardwareadapter
>> when switching to it and running from flash ... and if we are
>> relocated, we can analyse this flag, if init_done = 0, we init
>> this hardware adapter ...
>>
>> My proposal for the i2c_set_bus_num(unsigned int bus) function:
>>
>> int i2c_set_bus_num(unsigned int bus)
>> {
>> #ifndef CONFIG_SYS_I2C_DIRECT_BUS
>> 	int		i;
>> 	u_int8_t	buf;
>> #endif
>>
>> 	if (bus >= CONFIG_SYS_NUM_I2C_BUSSES)
>> 		return(-1);
>>
>> #ifndef CONFIG_SYS_I2C_DIRECT_BUS
>> 	/* Disconnect current bus (turn off muxes if any) */
>> 	if ((i2c_bus[i2c_cur_bus].next_hop[0].chip != 0) &&
>> 			(ADAP(i2c_cur_bus)->init_done != 0)) {
>>
>> 		i = CONFIG_SYS_I2C_MAX_HOPS;
>>
>> 		do {
>> 			u_int8_t	chip;
>> 			
>> 			if ((chip = i2c_bus[i2c_cur_bus].next_hop[--i].chip) == 0)
>> 				continue;
>>
>> 			ADAP(i2c_cur_bus)->write(chip, 0, 0, &buf, 1);
>>
>> 		} while (i > 0);
>> 	}
>> #endif
>>
>> 	cur_adap_nr = (i2c_adap_t *)&ADAP(bus);
>> 	if (ADAP(bus)->init_done == 0) {
>> 		i2c_init_bus(bus, ADAP(bus)->speed, ADAP(bus)->slaveaddr);
>> 	}
>>
>> #ifndef CONFIG_SYS_I2C_DIRECT_BUS
>> 	/* Connect requested bus if behind muxes */
>> 	if (i2c_bus[bus].next_hop[0].chip != 0) {
>>
>> 		/* Set all muxes along the path to that bus */
>> 		for (i = 0; i < CONFIG_SYS_I2C_MAX_HOPS; i++) {
>>
>> 			if (i2c_bus[bus].next_hop[i].chip == 0) break;
>>
>> 			i2c_mux_set(i2c_bus[bus].adapter,
>> 					i2c_bus[bus].next_hop[i].mux.id,
>> 					i2c_bus[bus].next_hop[i].chip,
>> 					i2c_bus[bus].next_hop[i].channel);
>> 		}
>> 	}
>> #endif
>>
>> 	i2c_cur_bus = bus;
>> 	return(0);
>> }
>>
>> This function should also work, when running from flash. + a hardware adapter
>> is only initialized when we switch to it, so no need to initialize all
>> hardwareadapters somewhere ...
>> (requirement: cur_adap_nr, i2c_cur_bus is writeable when running in flash)
> 
> You are multiplying entities. i2c_init() is invoked as a part of system

But Wolfgang suggested to init an I2C adapter just if we use it, and
above function will do this. Again read my EMails. I wrote:

>> This function should also work, when running from flash. + a hardware adapter
>> is only initialized when we switch to it, so no need to initialize all
>> hardwareadapters somewhere ...

So no need for calling i2c_init in libXXX/board.c or somewhere. We must just
look that for example in dtt_init is also a i2c_set_bus_num () called.

> bootup process in libXXX/board.c anyways. There is no need for any global
> variables, even non-writable for proposed code to initialize adapters.

Not for initialize adapters, because you want to init all adapters.
But if we want to do what Wolfgang suggested, we don;t need to call
i2c_init for all adapters.

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