Helo Heiko, Thanks for timely reply.
On 30 September 2013 13:35, Heiko Schocher <h...@denx.de> wrote: > Hello Naveen, > > Am 30.09.2013 08:58, schrieb Naveen Krishna Chatradhi: > >> This patchset fixes few bugs in the existing s3c24x0.c code (standard i2c) >> and add support for High-speed i2c bus controller available on Exynos5 >> SoCs >> from Samsung. >> >> Exynos5250 channels [0 ~ 3] can configured for I2C contoller or >> new HSI2C controller >> channels [3 ~ 7] are standard I2C controller channels >> Exynos5420 channels [0 ~ 3] are standard I2C controller channels and >> channels [4 ~ 10] are High-speed controller channels >> >> Patchset: >> 1. exynos: i2c: Fix i2c driver to handle NACKs properly >> Improvements and fixes from Vadim Bendebury for standard i2c calls >> >> 2. exynos: i2c: Change FDT bus setup code to enumerate ports correctly >> FDT bus setup code from Simon Glass >> >> 3. PATCH v5: i2c: s3c24xx: add hsi2c controller support >> High-speed controller register description and defines new i2c >> read/write, >> probe and set_bus calls. >> >> This is been reviewed earlier at >> http://lists.denx.de/pipermail/u-boot/2013-May/153245.html >> Thanks for review and improvements from Vadim Bendebury. >> >> Question: >> 4. RFC: samsung: i2c: Enable new CONFIG_SYS_I2C framework >> I've tried to implement the new I2C multi bus framework in u-boot tree, >> taking tegra-i2c.c as reference. >> >> a). We have different number of channels on Exynos5250 and Exynos5420 >> (as explained above). How are we supposed to define the >> U_BOOT_I2C_ADAP_COMPLETE >> in such cases. Can i use #ifdef EXYNOS5420 or EXYNOS5250 > > > From my side, Yes. Ok, Will do accordingly. But, I would prefer a way of integrating it to FDT or passing as platform data (board files) instead of keeping it in the driver. > > >> b). When i define 11 buses as in the case of Exynos5420, the "i2c bus" >> lists them >> SMDK5420 # i2c bus >> Bus 0: s3c0 >> Bus 1: s3c1 >> Bus 2: s3c10 >> Bus 3: s3c2 >> Bus 4: s3c3 >> Bus 5: s3c4 >> Bus 6: s3c5 >> Bus 7: s3c6 >> Bus 8: s3c7 >> Bus 9: s3c8 >> Bus 10: s3c9 >> or (If i change the name to hsi2c) >> SMDK5420 # i2c bus >> Bus 0: hsi2c10 >> Bus 1: hsi2c4 >> Bus 2: hsi2c5 >> Bus 3: hsi2c6 >> Bus 4: hsi2c7 >> Bus 5: hsi2c8 >> Bus 6: hsi2c9 >> Bus 7: s3c0 >> Bus 8: s3c1 >> Bus 9: s3c2 >> Bus 10: s3c3 >> >> Whats the expected behaviour. If the above result is correct, I need to >> changei >> the strings to get them in the correct order. > > > What, if you use two digits: > > Bus 0: hsi2c01 > Bus 1: hsi2c02 > [...] > Bus 7: s3c00 > Bus 8: s3c01 > [...] > > ? > > Or with one digit: > > Bus 0: hsi2c1 > Bus 1: hsi2c2 > [...] > > Bus 7: s3c0 > Bus 8: s3c1 > [...] In the Exynos5420 hardware channels are as below channel: --0----1----2----3------4--------5---------6-------7--------8-------9-------10 controller: i2c, i2c, i2c, i2c, hsi2c, hsi2c, hsi2c, hsi2c, hsi2c, hsi2c, hsi2c. But the "i2c bus" command is showing the bus number according to the "name" string comparison. Which seems wrong. Isn't it ?? > > >> c). What's the alternative for the >> board_i2c_init(), i2c_get_bus_num_fdt(), i2c_reset_port_fdt(). >> "Functions to get the I2C bus number and reset I2C bus using FDT node" >> >> I think, these functions are still needed. > > > Hmm.. that needs a general discussion, how to use fdt and i2c I think. > > I would prefer a way (not really nowing, if it is possible for all > configurations) where, if using fdt, the DT gets parsed and the > availiable i2c buses gets created ... After that, "normal" i2c access > with i2c_set_bus_num(), i2c_read/write should be possible ... this is > currently not possible with the i2c framework, but should not be so hard > to do. Except the restriction, that it would not work in SPL case, or > before relocation for i2c buses announced through dt once i2c_init_board() is done board_i2c_init() is not quite needed using i2c_init_board we can avoid the relocation problem aswell. by the definition of i2c_get_bus_num() in drivers/i2c/i2c_core.c unsigned int i2c_get_bus_num(void) { return gd->cur_i2c_bus; } we don't need a special function i2c_get_bus_num_fdt() IMHO, i2c_reset_port_fdt() is the only function to be discussed > > Define CONFIG_SYS_I2C_MAX_HOPS -> CONFIG_SYS_I2C_DIRECT_BUS is not defined > so i2c_bus[] is used in drivers/i2c/i2c_core.c. Define the fix i2c buses in > the board config file with CONFIG_SYS_I2C_BUSES (if you have no fix buses, > let this empty) and add a function in drivers/i2c/i2c_core.c, which adds > new i2c buses to i2c_bus[] after the fix buses and call this new function, > from where you interpret the fdt ... I din't quite understood this. Can you point me to some readme or Doc or discussion Please. > > int i2c_get_bus_num_fdt(int node) should be integrated to > drivers/i2c/i2c_core.c > > For what purpose is i2c_reset_port_fdt() ? > > bye, > Heiko > -- > DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Thanks, -- Shine bright, (: Nav :) _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot