From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Thu, 19 Feb 2009 07:42:09 +0100 Subject: [U-Boot] [PATCH] 7/12 Multiadapter/multibus I2C, drivers part 4 In-Reply-To: References: <499548C2.7060305@denx.de> <49967FAC.6090905@denx.de> <20090216221151.08966832E893@gemini.denx.de> <499BC42E.50001@denx.de> <499BCDC5.6080000@denx.de> Message-ID: <499CFF41.5000000@denx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.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 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