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<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.
[...]
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 ...

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?

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++;
}

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 ...

bye,
Heiko
--
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to