From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heiko Schocher Date: Fri, 06 Dec 2013 07:16:18 +0100 Subject: [U-Boot] i2c: samsung: register i2c busses for Exynso5420 and Exynos5250 In-Reply-To: References: <1385378884-14033-1-git-send-email-ch.naveen@samsung.com> <52A062DB.3010006@denx.de> <52A075EE.3010809@denx.de> Message-ID: <52A16BB2.8020402@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 06.12.2013 06:47, schrieb Naveen Krishna Ch: > Hello Heiko, > > On 5 December 2013 18:17, Heiko Schocher wrote: >> Hello Naveen, >> >> Am 05.12.2013 13:21, schrieb Naveen Krishna Ch: >> >>> Hello Heiko, >>> >>> On 5 December 2013 16:56, Heiko Schocher wrote: >>>> >>>> Hello Naveen, >>>> >>>> Sorry for the late reply ... >>>> >>>> Am 25.11.2013 12:28, schrieb Naveen Krishna Ch: >>>> >>>>> This patch adds the U_BOOT_I2C_ADAP_COMPLETE defines for channels >>>>> on Exynos5420 and Exynos5250 and also adds support for init function >>>>> for hsi2c channels >>>>> >>>>> Signed-off-by: Naveen Krishna Chatradhi >>>>> >>>>> --- >>>>> README | 6 ++ >>>>> drivers/i2c/s3c24x0_i2c.c | 222 >>>>> +++++++++++++++++++++++++++++++++++---------- >>>>> 2 files changed, 180 insertions(+), 48 deletions(-) >>>>> >>>>> diff --git a/README b/README >>>>> index c97ff0a..c04e352 100644 >>>>> --- a/README >>>>> +++ b/README >>>>> @@ -2076,6 +2076,12 @@ CBFS (Coreboot Filesystem) support >>>>> - set CONFIG_SYS_I2C_ZYNQ_SPEED for speed setting >>>>> - set CONFIG_SYS_I2C_ZYNQ_SLAVE for slave addr >>>>> >>>>> + - drivers/i2c/s3c24x0_i2c.c: >>>>> + - activate this driver with CONFIG_SYS_I2C_S3C24X0 >>>>> + - This driver adds i2c buses (11 for Exynos5250, >>>>> Exynos5420 >>>>> + 9 i2c buses for Exynos4 and 1 for S3C24X0 SoCs from >>>>> Samsung) >>>>> + with a fix speed from 100000 and the slave addr 0! >>>>> + >>>>> additional defines: >>>>> >>>>> CONFIG_SYS_NUM_I2C_BUSES >>>>> diff --git a/drivers/i2c/s3c24x0_i2c.c b/drivers/i2c/s3c24x0_i2c.c >>>>> index 1e9dba0..6e719ec 100644 >>>>> --- a/drivers/i2c/s3c24x0_i2c.c >>>>> +++ b/drivers/i2c/s3c24x0_i2c.c >>>>> @@ -721,6 +721,15 @@ static unsigned int >>>>> s3c24x0_i2c_set_bus_speed(struct >>>>> i2c_adapter *adap, >>>>> return 0; >>>>> } >>>>> >>>>> +static void exynos_i2c_init(struct i2c_adapter *adap, int speed, int >>>>> slaveaddr) >>>>> +{ >>>>> + /* This will override the speed selected in the fdt for that >>>>> port >>>>> */ >>>>> + debug("i2c_init(speed=%u, slaveaddr=0x%x)\n", speed, slaveaddr); >>>>> + if (i2c_set_bus_speed(speed)) >>>>> + printf("i2c_init: failed to init bus %d for speed = >>>>> %d\n", >>>>> + adap->hwadapnr, speed); >>>>> +} >>> >>> This init is not used for Exynos4 and lower series SoCs >>>>> >>>>> + >>>>> /* >>>>> * cmd_type is 0 for write, 1 for read. >>>>> * >>>>> @@ -1071,51 +1080,168 @@ int i2c_reset_port_fdt(const void *blob, int >>>>> node) >>>>> /* >>>>> * Register s3c24x0 i2c adapters >>>>> */ >>>>> -U_BOOT_I2C_ADAP_COMPLETE(s3c24x0_0, s3c24x0_i2c_init, >>>>> s3c24x0_i2c_probe, [...] >>>>> +U_BOOT_I2C_ADAP_COMPLETE(i2c08, s3c24x0_i2c_init, s3c24x0_i2c_probe, >>>>> + s3c24x0_i2c_read, s3c24x0_i2c_write, >>>>> + s3c24x0_i2c_set_bus_speed, >>>>> + CONFIG_SYS_I2C_S3C24X0_SPEED, >>>>> + CONFIG_SYS_I2C_S3C24X0_SLAVE, 8 > Is missing the closing braces. My local changes has it so i couldnt > see the problem. Ups? >>>> leads in a compile error for trats and trats2, could you please fix? >>> >>> I ran "MAKEALL -v samsung" >>> s3c24x0_i2c.c:724:13: warning: 'exynos_i2c_init' defined but not used >>> [-Wunused-function] >>> is the warning seen for trats and trats2 boards >>> If you are talking about this one, will fix it in the next versio. >> >> >> No, there is a missing ")" at the end! >> >> pollux:u-boot hs [master] $ ./MAKEALL trats >> Configuring for trats board... >> s3c24x0_i2c.c:1247:0: error: unterminated argument list invoking macro >> "U_BOOT_I2C_ADAP_COMPLETE" >> s3c24x0_i2c.c:1236:1: error: expected '=', ',', ';', 'asm' or >> '__attribute__' at end of input >> arm-linux-gnueabi-size: './u-boot': No such file >> s3c24x0_i2c.c:1247:0: error: unterminated argument list invoking macro >> "U_BOOT_I2C_ADAP_COMPLETE" >> s3c24x0_i2c.c:1236:1: error: expected '=', ',', ';', 'asm' or >> '__attribute__' at end of input >> >> s3c24x0_i2c.c:724:13: warning: 'exynos_i2c_init' defined but not used >> [-Wunused-function] >> make[1]: *** [s3c24x0_i2c.o] Fehler 1 >> make[1]: *** Warte auf noch nicht beendete Prozesse... >> make: *** [drivers/i2c/built-in.o] Fehler 2 >> make: *** Warte auf noch nicht beendete Prozesse... >> >> --------------------- SUMMARY ---------------------------- >> Boards compiled: 1 >> Boards with errors: 1 ( trats ) >> ---------------------------------------------------------- > #pwclient git-am 292705 > #pwclient git-am 293889 > #pwclient git-am 292706 > #./MAKEALL trats > Shows me this errors. Thanks for the catch. Puuh... >> pollux:u-boot hs [master] $ >> >> pollux:u-boot hs [(kein Zweig, Neuaufbau begonnen bei master)] $ git bisect >> log >> git bisect start >> # bad: [f5f40408ce3b00637c652fdabfb6f3c4e0d09635] arm: omap: i2c: don't zero >> cnt in i2c_write >> git bisect bad f5f40408ce3b00637c652fdabfb6f3c4e0d09635 >> # good: [f44483b57c49282299da0e5c10073b909cdad979] Merge branch 'serial' of >> git://git.denx.de/u-boot-microblaze >> git bisect good f44483b57c49282299da0e5c10073b909cdad979 >> # bad: [4d06a7c5707f813f4bc7fd7a9d700606c63fa4de] i2c: fti2c010: cosmetic: >> coding style cleanup >> git bisect bad 4d06a7c5707f813f4bc7fd7a9d700606c63fa4de >> # good: [a6756bbdac43474193472b43309626e2c28d8100] driver:i2c:s3c24x0: fix >> clock init for hsi2c >> git bisect good a6756bbdac43474193472b43309626e2c28d8100 >> # bad: [c3b6bba20862ff6476c99aa6d3168f7097a5b4b2] i2c: samsung: register i2c >> busses for Exynso5420 and Exynos5250 >> git bisect bad c3b6bba20862ff6476c99aa6d3168f7097a5b4b2 >> # first bad commit: [c3b6bba20862ff6476c99aa6d3168f7097a5b4b2] i2c: samsung: >> register i2c busses for Exynso5420 and Exynos5250 >> pollux:u-boot hs [(kein Zweig, Neuaufbau begonnen bei master)] $ >> >> >>>> >>>> Hmm... could you reformat this very long list in something like this: >>>> >>>> U_BOOT_I2C_ADAP_COMPLETE(s3c0, s3c24x0_i2c_init, s3c24x0_i2c_probe, >>>> if defined(CONFIG_EXYNOS4) || defined(CONFIG_EXYNOS5420) || >>>> defined(CONFIG_EXYNOS5420) >>>> U_BOOT_I2C_ADAP_COMPLETE(s3c1, s3c24x0_i2c_init, s3c24x0_i2c_probe, >>>> [...] >>>> U_BOOT_I2C_ADAP_COMPLETE(s3c8, s3c24x0_i2c_init, s3c24x0_i2c_probe, >>>> #endif >>>> if defined(CONFIG_EXYNOS5420) || defined(CONFIG_EXYNOS5420) >>>> U_BOOT_I2C_ADAP_COMPLETE(s3c9, s3c24x0_i2c_init, s3c24x0_i2c_probe, >>>> U_BOOT_I2C_ADAP_COMPLETE(s3c10, s3c24x0_i2c_init, s3c24x0_i2c_probe, >>>> #endif > Even if we implement a common init function and reduce this list. The > speeds would > be fixed and the user need to set the speeds by calling i2c speed or > the corresponding > function. Ok, I see... so please just sent the compileerrorfree patch, thanks! >>> Basically, s3c24x0_i2c.c now support both HighSpeed I2C modules and >>> Standard I2C >>> Modules. They have different init functions. >>> Exynos5250/5260 has 4 hsi2c channels hsi2c 0 ~ 3 and 7 i2c channels 4 ~ 10 >>> Exynos5420 has 4 i2c channels i2c 0 ~ 3 and 7 hsi2c channels 4 ~ 10 >>> Exynos4 has 9 i2c channels >>> Hence, the long list. It would be helpful if anyone can suggest a way >>> to reduce this list. >> >> >> Ah, now I see it, thanks! >> >> Maybe we can define one s3c24x0_i2c_init function, and call from there >> the SoC / adapater specific init func? So we can prevent this long list? > I can think of something like this > +static void samsung_i2c_init(struct i2c_adapter *adap, int speed, int slaveadd) > +{ > + > +#ifdef CONFIG_EXYNOS5 > +#ifdef CONFIG_EXYNOS5420 > + if(adap->hwadapnr> 3) > + s3c24x0_i2c_init(adap, speed, slaveadd); > + else > + exynos_i2c_init(adap, speed, slaveadd); > +#elif defined(CONFIG_EXYNOS5250) || defined(CONFIG_EXYNOS5260) > + if(adap->hwadapnr> 3) > + exynos_i2c_init(adap, speed, slaveadd); > + else > + s3c24x0_i2c_init(adap, speed, slaveadd); > +#endif > +#else > + s3c24x0_i2c_init(adap, speed, slaveadd); > +#endif > +} > > But, as mentioned earlier the speeds needs to be set from a seperate function > for each board. >> >> >>> Is U_BOOT_I2C_ADAP_COMPLETE is the only (best) way to register to new i2c >>> framework ? > >> >> >> Yes. > Is there any work going on to simplyfy U_BOOT_I2C_ADAP_COMPLETE ? No, what do you thinking of? Simplify patches are always welcome! bye, Heiko -- DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany