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

[ksi at koi8.net]

On Wed, 18 Feb 2009, Heiko Schocher wrote:

> Hello ksi,
> 
> Heiko Schocher schrieb:
> > Hello ksi,
> > 
> > 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 at 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?
> > 
> > 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?
> 
> 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
bootup process in libXXX/board.c anyways. There is no need for any global
variables, even non-writable for proposed code to initialize adapters.

---
******************************************************************
*  KSI at home    KOI8 Net  < >  The impossible we do immediately.  *
*  Las Vegas   NV, USA   < >  Miracles require 24-hour notice.   *
******************************************************************