From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Wed, 31 Jul 2013 07:03:32 +0200 Subject: [U-Boot] [PATCH v3 8/9] tegra: i2c: Enable new CONFIG_SYS_I2C framework In-Reply-To: <51F81B51.6010305@wwwdotorg.org> 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> Message-ID: <51F89AA4.20401@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 Stephen, Am 30.07.2013 22:00, schrieb Stephen Warren: > On 07/30/2013 01:22 PM, Stephen Warren wrote: >> On 07/29/2013 10:28 PM, Heiko Schocher wrote: >>> Hello Stephen, >>> >>> Am 29.07.2013 18:12, schrieb Stephen Warren: >>>> On 05/04/2013 06:01 AM, Heiko Schocher wrote: >>>>> From: Simon Glass >>>>> >>>>> This enables CONFIG_SYS_I2C on Tegra, updating existing boards and >>>>> the Tegra >>>>> i2c driver to support this. >>>> >>>> Heiko, the latest U-Boot tree hangs during boot on Tegra, and "git >>>> bisect" points at this patch. Olof reported the issue to me. >>>> Can you take a look at the code and see what might be wrong? Thanks. >>>> >>>> I suspect some kind of initialization ordering issue, since the boot >>>> messages are: >>>> >>>> ----- >>>> U-Boot SPL 2013.07-rc3-00038-g880540d (Jul 29 2013 - 10:04:37) >>>> U-Boot 2013.07-rc3-00038-g880540d (Jul 29 2013 - 10:04:37) >>>> >>>> TEGRA30 >>>> Board: NVIDIA Beaver >>>> I2C: Caller requested bad clock: periph=-49, parent=2 >>>> ----- >>>> >>>> ... and that "bad clock" message implies to me that the I2C driver is >>>> initializing before it has parsed the correct clock ID out of device >>>> tree. >>> >>> Hmm... looking in the patch ... I can see nothing which changes >>> some initializing order ... >> >> Yes, there's some initialization order issue; before this patch, I see >> the I2C controller initialization, followed by some usage of it: >> >> ---------- >> U-Boot SPL 2013.07-rc3-00036-gd84eb85-dirty (Jul 30 2013 - 13:04:47) >> U-Boot 2013.07-rc3-00036-gd84eb85-dirty (Jul 30 2013 - 13:04:47) >> >> TEGRA30 >> Board: NVIDIA Beaver >> DRAM: 2 GiB >> i2c: controller bus 0 at 7000d000, periph_id 47, speed 100000: ok >> i2c: controller bus 1 at 7000c000, periph_id 12, speed 100000: ok >> i2c: controller bus 2 at 7000c400, periph_id 54, speed 100000: ok >> i2c: controller bus 3 at 7000c500, periph_id 67, speed 100000: ok >> i2c: controller bus 4 at 7000c700, periph_id 103, speed 100000: ok >> MMC: i2c_write: chip=0x2d, addr=0x32, len=0x1 >> ---------- >> >> However with this patch applied, something starts using the controller >> immediately, without it having been "probed" from device-tree: >> >> ---------- >> U-Boot SPL 2013.07-rc3-00037-g1f2ba72-dirty (Jul 30 2013 - 13:08:28) >> U-Boot 2013.07-rc3-00037-g1f2ba72-dirty (Jul 30 2013 - 13:08:28) >> >> TEGRA30 >> Board: NVIDIA Beaver >> I2C: i2c_init(speed=100000, slaveaddr=0xfe) >> ---------- >> >> i2c_init touches HW, but since process_nodes() hasn't run, none of the >> parameters like register address or clock ID are yet known. >> >> I think this call comes from init_sequence_f[] -> init_func_i2c() -> >> i2c_init() -> i2c_init() == __i2c_init() -> i2c_init_bus() -> >> I2C_ADAP->init(), although I didn't validate that in the running code, >> just by code inspection. > > Oh, with the options Tegra has enabled, perhaps the call sequence is: > > board_init_f() (which uses init_sequence_f[]) -> init_func_i2c() -> > i2c_init_all(), which then calls: > > * i2c_init_board(), which is supposed to parse DT > * i2c_set_bus_num(), which will call I2C_ADAP->init > > However, according to the comments near the top of arch/arm/lib/crt0.S, > board_init_f() is called in an environment where variable data (.data, > .bss) is not available, hence i2c_init_board() cannot possibly operate > correctly since its whole purpose is to fill in variable data structures > from DT. Yes, you are right, this would not work on tegra. > I think the only way to solve this is not to use DT to instantiate > devices, or to move the I2C initialization after relocation etc., > although the latter won't work on boards that need I2C up in order to > initialize DRAM. Yes. > It seems like much of U-Boot's initialization architecture simply wasn't > designed to accommodate dynamically initializing devices from DT. Yes. bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany