From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Tue, 01 Oct 2013 08:36:51 +0200 Subject: [U-Boot] [PATCH 0/3] i2c: improve s3c24x0 with High-speed and new SYS_I2C framework support In-Reply-To: References: <1363085897-19814-1-git-send-email-ch.naveen@samsung.com> <1380524290-9644-1-git-send-email-ch.naveen@samsung.com> <524930DB.3050703@denx.de> <524A660E.40908@denx.de> Message-ID: <524A6D83.2070909@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 Naveen, Am 01.10.2013 08:19, schrieb Naveen Krishna Ch: > Hello Heiko, > > On 1 October 2013 11:35, Heiko Schocher wrote: >> Hello Naveen, >> >> Am 30.09.2013 12:03, schrieb Naveen Krishna Ch: >> >>> Helo Heiko, >>> >>> Thanks for timely reply. >>> >>> On 30 September 2013 13:35, Heiko Schocher 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. [...] >>> 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 ?? >> >> >> Hmm.. you are right, seems that the compiler sorts them alphabetical ... >> So, two ways: >> - use another name, saying first a two digit for the channel ? >> - or, use CONFIG_SYS_NUM_I2C_BUSES, CONFIG_SYS_I2C_DIRECT_BUS >> and CONFIG_SYS_I2C_BUSES as described in the README (grep >> for CONFIG_SYS_I2C_BUSES in include/configs and you will find >> some examples ...) and you can define, which adapter is on which >> i2c_bus number ... > Will try and implement it this way. Ok. >>>>> 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() >> >> >> Ah, ok! >> >> >>> IMHO, i2c_reset_port_fdt() is the only function to be discussed >> >> >> And why is i2c_init() not good? Why must we have here a new function? > That's right, even if there is a need for i2c_reset_port_fdt(). > it must be a i2c-core function instead of being in a driver. >> >> >>>> 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. >> >> >> just the U-Boot README ... The above was just a fast idea, how it >> is possible to add i2c buses from the info in the fdt ... >> >> Here some theoretical code, how it could look like: [...] > Thanks for this explanation. ! It is just theoretical ... you must try it ;-) >> And maybe here and there some adaptions for getting this running... >> Thinking of i2c_set_bus_num(), there must be now a check for >> i2c_fix_bus I think ... >> >> Adapt the README ... >> >> And then, from the place where you interpret the fdt, call if >> you have the information for one i2c bus, i2c_add_one_bus() ... >> >> Not sure, if I overlooked here something ... > > Will try and do this. > Mean while can we get the other 3 patches reviewed. > Patch 1: http://patchwork.ozlabs.org/patch/278933/ This patch seems good to me, added to u-boot-i2c.git as it seems it is a bugfix. > Patch 2: http://patchwork.ozlabs.org/patch/278931/ > Patch 3: http://patchwork.ozlabs.org/patch/278930/ Think, they are OK, but they go after the next release to mainline, as we are close to the next release ... > They were tested without the change to the new i2c framework. Thanks for your work! bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany