From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Thu, 01 Aug 2013 17:06:03 +0200 Subject: [U-Boot] [PATCH v3 8/9] tegra: i2c: Enable new CONFIG_SYS_I2C framework In-Reply-To: References: <1367668903-29653-1-git-send-email-hs@denx.de> <1367668903-29653-9-git-send-email-hs@denx.de> <51F6946F.8010500@wwwdotorg.org> <51F740FD.4080505@denx.de> <51F81261.7020008@wwwdotorg.org> <51F81B51.6010305@wwwdotorg.org> <51F83104.5090508@wwwdotorg.org> <51F83570.1010909@wwwdotorg.org> <51F8A4CA.7070702@denx.de> <51F9662D.1070602@wwwdotorg.org> <51F9E4CB.9040004@denx.de> <51F9F477.5060607@wwwdotorg.org> <51F9FA02.1060403@denx.de> <20130801085302.3d1ce0f1@lilith> <51FA1E77.9040600@denx.de> Message-ID: <51FA795B.3060002@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 Simon, Am 01.08.2013 16:22, schrieb Simon Glass: > Hi, > > On Thu, Aug 1, 2013 at 2:38 AM, Heiko Schocher wrote: > >> Hello Albert, >> >> Am 01.08.2013 08:53, schrieb Albert ARIBAUD: >> >> Hi Heiko, >>> >>> On Thu, 01 Aug 2013 08:02:42 +0200, Heiko Schocher wrote: >>> >>> I suppose you could. It seems conceptually /far/ simpler to just scan >>>>> the DT once up-front rather than having to defer all this stuff until >>>>> >>>> >>>> on the other hand we ring for every ms boot time ... and here we want >>>> to scan a complete dt with maybe a lot of nodes, we do not want to >>>> use? >>>> >>> >>> Scanning all of DT seems to imply it has no strict or standard >>> ordering. Could we mandate, suggest, of make it so that all entries in >>> the DT needed at _f time are put first, and even maybe place an "end of >>> _f" custom marker in DT to delimit them? (I assume that, for the sake of >>> >> >> I do not know, if this is possible, as I think the DT used in U-Boot >> should be the same as used in linux ... or? >> >> >> Postel-ism, anything in DT which is not understandable is skipped, so >>> other users of the DT than us would not even be annoyed by such a >>> marker) >>> >>> This way, we'd avoid wasting time scanning most of the DT in this case. >>> >> >> Hmm.. why should we introduce such things, instead of scanning the >> node only if we need it? >> >> We have "only" the problem, that we could not write to data at this >> moment ... but this problem should be solved in a seperate topic. >> I2C is usable before relocation, the problem is in conjunction with >> dt, that we can not save for example the base address of the controller, >> which we get from the DT ... If I understand it correct! >> >> So we need an option when using dt, that we have (small ram) in which >> we can write some parameters parsed from dt ... >> >> I think this problem have all subsystems used before relocation. >> (for example: environment on a spi flash?) >> >> > I think using a small RAM is a good approach. At least it is better than > pretending there is no RAM at all. We currently have no facility to > allocate RAM before relocation, other than to use the .data section. We can > use global_data but that won't scale well for many drivers adding their own > stuff in there. Samsung's driver uses .data, I don't think it is a big deal. > > One option I should mention is to decode the I2C FDT nodes each time it is > needed prior to relocation (i.e. to the stack), use it for the transaction, > and throw it away. This is quite painful IMO this it involves putting calls > in the driver to check if we are pre-reloc and have a stack space used > every time. On tegra20 this would be 130 bytes or so. I mention it because > console working this way for a while (decoding the console again on every > byte). > > Options as I see it: > > - just fudge it for now and use .data (deal with it later with driver model) > - change the meaning of board_init_f() such that memory may be available > (e.g. if run from SPL) > - decode the FDT nodes every time we need them (ick) Why? after relocation it is needed exactly once. You can do something like that in the i2c_init function: + bus = tegra_i2c_get_bus(adap); + if (bus) + return 0; If you get a bus with tegra_i2c_get_bus(adap), it is "fdt initialized" If you not get a bus ... init it ... only once The problem before relocation could be solved with .data or later with the driver model ... > - adjust the ordering so that I2C normally happens post reloc except for > specific platforms with a CONFIG defined (Heiko, the difference as I > understand it is that with your patch CONFIG_HARD_I2C or CONFIG_SOFT_I2C > are now defined, and so I2C happens prior to reloc now) Hmm.. BTW: CONFIG_SOFT_I2C is no longer in the code :-) And there are a lot of boards that use i2c before relocation, that is not a new feature. The problem pop up, because in arch/arm/lib/board.c at init_func_i2c() the new code calls i2c_init_all() instead i2c_init() ... This i2c_init_all() was introduced to init fix the SPD EEPROM bus and call i2c_init_board() before it... So we can call here again i2c_init() I think ... but, i2c_init() also calls i2c_init_bus() and this i2c_init from the tegra driver, which sets speed and return 0 ... what is not really clean, it just works, because tegra_i2c_set_bus_speed() checks if the bus is valid, if not only returns ... but i2c_init_board is not called so early, which is the cause for our problem with the tegra driver and the new framework, as i2c_init_board() hysterically was introduced to unblock blocked i2c busses (Correct me, if I am wrong) My prefered way is still: - replace i2c_init_all() through i2c_init() in arch/arm/lib/board.c at init_func_i2c() to get tegra again working In the long term when all i2c drivers use the new framework init_func_i2c() will no longer necessary :-) As the goal is to change the i2c api, so all i2c functions pass "bus" as a parameter, and i2c_set_bus_num() get a static function in drivers/i2c/i2c_core.c no need longer for storing the old bus, setting the new bus, doing i2c work, set the old bus, as this sequence is spread over the hole u-boot code. - decoding fdt node in tegra_i2c_init, as I think it is the right place for it. optional more or less, but to prevent in future again such "order" problems ... >> As Wolfgang said: >> "Agreed - actually we're entering an area hear that smells pretty >> strong like device model reorganization :-)" >> >> BTW: How is this problem solved with the device model approach? > > > More that we need to solve it, probably with a limited malloc() pre-reloc. bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany