[U-Boot] [PATCH 0/3] i2c: improve s3c24x0 with High-speed and new SYS_I2C framework support
Naveen Krishna Ch
naveenkrishna.ch at gmail.com
Tue Oct 1 08:19:08 CEST 2013
Hello Heiko,
On 1 October 2013 11:35, Heiko Schocher <hs at denx.de> 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<hs at 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.
>
> [...]
>
>>>> 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 ??
>
>
> 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.
>
>
>>>> 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:
>
> in the board config file add:
>
> #define CONFIG_SYS_I2C_DIRECT_BUS 1
> #define CONFIG_SYS_I2C_MAX_HOPS 0
> #define CONFIG_SYS_NUM_I2C_BUSES 10 /* maximum possible i2c buses
> used on the hw */
> #define CONFIG_SYS_FIX_I2C_BUSES 1 /* just as an example, here with
> one fix i2c bus */
> #define CONFIG_SYS_I2C_BUSES { {0}, /* adapter 0 is fix on i2c_bus
> number 0 */
> }
>
> in drivers/i2c/i2c_core.c:
>
> static int i2c_fix_bus = CONFIG_SYS_FIX_I2C_BUSES;
>
> /* add one i2c bus without muxes ... ToDo: how to add i2c buses with muxes
> */
> static int i2c_add_one_bus(char *adapter_name)
> {
> struct i2c_bus_hose *tmp;
>
> if (i2c_fix_bus >= CONFIG_SYS_NUM_I2C_BUSES)
> return -ENOMEM;
>
> /* search adapter with name */
> adap = i2c_search_adapter_name(name); /* Code this function */
> if (adap < 0)
> return -ENODEV;
>
> tmp = kalloc(sizeof(struct i2c_bus_hose));
> tmp->adapter = adap;
> /* if found add it into i2c_bus */
> memcpy(&i2c_bus[i2c_fix_bus], tmp, sizeof(struct i2c_bus_hose));
> i2c_fix_bus++;
> }
Thanks for this explanation.
>
> 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/
Patch 2: http://patchwork.ozlabs.org/patch/278931/
Patch 3: http://patchwork.ozlabs.org/patch/278930/
They were tested without the change to the new i2c framework.
>
>
> bye,
> Heiko
> --
> DENX Software Engineering GmbH, MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
--
Shine bright,
(: Nav :)
More information about the U-Boot
mailing list