public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [U-Boot] i2c: rework multibus/multiadapter functionality
@ 2009-03-24  8:10 Heiko Schocher
  2009-07-17 22:09 ` Wolfgang Denk
  0 siblings, 1 reply; 9+ messages in thread
From: Heiko Schocher @ 2009-03-24  8:10 UTC (permalink / raw)
  To: u-boot

Hello,

I want now, because the merge window is open again, restart the
the multibus/multiadapter discussion.

To have a base for this discusion, I made in the i2c git tree
(git://git.denx.de/u-boot-i2c.git) the following new 2 branches:

"multibus":

  Patches from Sergey Kubushyn <ksi@koi8.net> posted in the last merge window.
  see, http://lists.denx.de/pipermail/u-boot/2009-February/047465.html

They were discussed, but not accepted ... also some CodingStyle
comments, changes in patch hierarchic are not done ...
I only synced them to actuall code to have a base for the discussion.

"multibus_v2":
(I couldn;t post the patches because one of them is bigger than
 100k :-( and I cannot split it in 2 patches, because this would
 break git bisection compatibility. But the patches can be found
 in git://git.denx.de/u-boot-i2c.git):

Based on the i2c_core from Sergeys patches, added
the following suggestions from Wolfgang and me:

- v2 is bisection compatible
- commits are in (hopefully) logical blocks

- "cur_adap" added in gd_t:
  This points allways to the actuall used i2c adapter:

  - because gd_t is writeable when running in flash,
    complete multiadapter/multibus functionality is
    usable, when running in flash
  - using a pointer brings faster accesses to the i2c_adapter_t
    struct and saves some bytes here and there (see later).

- init from a i2c controller:
  In the "multibus" branch (also in actual code) all i2c controllers,
  as a precaution, getting initialized. In the "multibus_v2"
  branch, a i2c controller gets only initialized if it is
  used. This is done in i2c_set_bus_num().

  Actually, I let the i2c_init_all() in code, but just call
  in this function i2c_set_bus_num(CONFIG_SYS_SPD_BUS_NUM).
  This can be dropped in a second step completely.

  Also, with this approach, we can easy add in a second step,
  a i2c_deinit() function in i2c_set_bus_num(), so we can
  deactivate a no longer used controller.

- added "hw_adapnr" in i2c_adapter_t struct:
  when for example a CPU has more then one i2c controller
  we can use this variable to differentiate which
  controller the actual i2c adapter uses. This results in
  lower codesize and lower sourcecode changes.
  Example:

  fsl_i2c driver:
  (MPC8548CDS only with fsl driver)
    multibus:

    [hs at pollux u-boot-i2c]$ ./MAKEALL MPC8548CDS
    Configuring for MPC8548CDS board...
    fsl_i2c.c: In function '__i2c_init':
    fsl_i2c.c:173: warning: assignment discards qualifiers from pointer target type
       text    data     bss     dec     hex filename
     222168   17344   27256  266768   41210 ./u-boot

    multibus_v2:

    [hs at pollux u-boot-i2c]$ ./MAKEALL MPC8548CDS
    Configuring for MPC8548CDS board...
       text    data     bss     dec     hex filename
     222080   17344   27256  266680   411b8 ./u-boot

  also the bitbang driver gets better maintainable:

  Sourcecode changes:

    multibus:

    drivers/i2c/soft_i2c.c              |  704 ++++++++++++++++++++---------------

    multibus_v2:

    drivers/i2c/soft_i2c.c                         |  155 +++++---

  Codesize bitbang driver:
  (MPC8548CDS with the following bus topology:
   * Busses:
   *
   *      0:      Direct off of FSL_I2C[0]
   *      1:      FSL_I2C[1]->PCA9542(0)->PCA9542(0)
   *      2:      FSL_I2C[1]->PCA9542(0)->PCA9542(1)
   *      3:      FSL_I2C[1]->PCA9542(1)
   *      4:      Direct off of SOFT_I2C[0]
   *      5:      Direct off of SOFT_I2C[1]
   )

    multibus:

    [hs at pollux u-boot-i2c]$ ./MAKEALL MPC8548CDS
    Configuring for MPC8548CDS board...
    fsl_i2c.c: In function '__i2c_init':
    fsl_i2c.c:173: warning: assignment discards qualifiers from pointer target type
       text    data     bss     dec     hex filename
     227092   17864   27256  272212   42754 ./u-boot
    [hs at pollux u-boot-i2c]$

    multibus_v2:

    [hs at pollux u-boot-i2c]$ ./MAKEALL MPC8548CDS
    Configuring for MPC8548CDS board...
       text    data     bss     dec     hex filename
     225048   17868   27256  270172   41f5c ./u-boot
    [hs at pollux u-boot-i2c]$

  For this I call for all I2C_SDA, I2C_SCL, ... defines,
  which holds the board specific code, now functions (This
  is not a must, but I did this as an example see MPC8548CDS).
  In this functions I use "hw_adapnr" to switch to do the
  adapter specific gpio settings ...

  If we look at the "multibus" approach, there, this is not needed
  but Sourcecode size and Codesize grows with each bitbang
  adapter ... and if I think for a board with n bitbang adapter
  (with n > 5) this results in nearly unmaintable source code ...

  I just write this here because this has to be discussed (and
  this was a point, which drove the last discussion in chaos).
  We can now easy compare such a board ... see MPC8548CDS) and
  discuss on facts, which version we want to have in mainline.

- adding a base_addr to i2c_adap_t struct (Did this not yet,
  but would also a good thing to have, because when I ported
  for example the ppc_4xx i2c hardware adapter, I saw, that
  some CPUs have more than one controller, and they just differ
  in the base addr, so this variable would be a "good to have").
  I solved this actually in adding a function in this driver,
  which returns the base addr depending on hw_adapnr, which
  is a suboptimal way ...

  see for an example new ppc_4xx i2c driver:

  in drivers/i2c/ppc4xx_i2c.c:  ppc4xx_get_base ()

Codesize:

for a board with one soft_i2c driver:

[hs at pollux u-boot-i2c]$ git checkout master
Switched to branch "master"
Your branch is ahead of 'origin/master' by 143 commits.
[hs at pollux u-boot-i2c]$ ./MAKEALL CPU86
Configuring for CPU86 board...
... booting from 64-bit flash
   text    data     bss     dec     hex filename
 146572   22068   24144  192784   2f110 ./u-boot
[hs at pollux u-boot-i2c]$ git checkout multibus
Switched to branch "multibus"
[hs at pollux u-boot-i2c]$ ./MAKEALL CPU86
Configuring for CPU86 board...
... booting from 64-bit flash
   text    data     bss     dec     hex filename
 147816   22124   24144  194084   2f624 ./u-boot
[hs at pollux u-boot-i2c]$ git checkout multibus_v2
Switched to branch "multibus_v2"
[hs at pollux u-boot-i2c]$ ./MAKEALL CPU86
Configuring for CPU86 board...
... booting from 64-bit flash
   text    data     bss     dec     hex filename
 147688   22128   24144  193960   2f5a8 ./u-boot
[hs at pollux u-boot-i2c]$


for a board with the fsl driver:

[hs at pollux u-boot-i2c]$ git checkout master
./MA    Switched to branch "master"
Your branch is ahead of 'origin/master' by 143 commits.
[hs at pollux u-boot-i2c]$ ./MAKEALL MPC8360EMDS
Configuring for MPC8360EMDS board...
   text    data     bss     dec     hex filename
 203504   11588   26900  241992   3b148 ./u-boot
[hs at pollux u-boot-i2c]$ git checkout multibus
Switched to branch "multibus"
[hs at pollux u-boot-i2c]$ ./MAKEALL MPC8360EMDS
Configuring for MPC8360EMDS board...
fsl_i2c.c: In function '__i2c_init':
fsl_i2c.c:173: warning: assignment discards qualifiers from pointer target type
   text    data     bss     dec     hex filename
 204288   11628   26900  242816   3b480 ./u-boot
[hs at pollux u-boot-i2c]$ git checkout multibus_v2
Switched to branch "multibus_v2"
[hs at pollux u-boot-i2c]$ ./MAKEALL MPC8360EMDS
Configuring for MPC8360EMDS board...
   text    data     bss     dec     hex filename
 204256   11632   26900  242788   3b464 ./u-boot
[hs at pollux u-boot-i2c]$


at last here comes a TODO list what should be done:

- add README for the multibus functionality

- list of drivers to port:

  drivbers/i2c:
	bfin-twi_i2c.c
	mxc_i2c.c
	omap1510_i2c.c
	omap24xx_i2c.c
	tsi108_i2c.c

  grep -lr HARD_I2C cpu/
	cpu/arm920t/at91rm9200/i2c.c
	cpu/arm920t/s3c24x0/i2c.c
	cpu/mpc512x/i2c.c
	cpu/mpc5xxx/i2c.c
	cpu/mpc8220/i2c.c
	cpu/mpc824x/drivers/i2c/i2c.c
	cpu/mpc8xx/i2c.c
	cpu/pxa/i2c.c

  Hope didn;t forget some more ...

- test, test, test ...

- i2c devices must ported to new multibus functionality.
  Ideas here are welcome ;-)
  (They must "know", on which bus they are ...)

- In actuall code it is possible to add new i2c busses
  with the "i2c bus" command (and from code with i2c_mux_ident_muxstring())
  This actual is not included in the new code, but that could
  be done the following way (for the multibus_v2 version,
  for the approach in "multibus" branch I have no idea
  how to make this):

  When running in flash, we just can use the i2c busses
  defined in CONFIG_SYS_I2C_ADAPTERS (should be okay in the
  first step. When relocating code, the i2c busses in
  CONFIG_SYS_I2C_ADAPTERS are converted in a dynamcial
  list, so we can then easy add new busses to this list.

  Also we define a "i2c_bus" environment variable, which
  contains i2c busses, that gets added when relocating.
  This is needed for the keymile boards. There are a lot
  of board versions which differ only in the i2c bus
  topology. With this approach, we can use one u-boot
  binary for all board versions, just defining the i2c
  bus topology in the environment ...

  BTW:
  This is also a point for using "cur_adap", because the
  rest of the multibus functionality doesn;t have to be
  changed ...

So, I hope I didn;t forget something ... lets start with
the discussion ...

bye
heiko
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2009-07-18 10:18 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-03-24  8:10 [U-Boot] i2c: rework multibus/multiadapter functionality Heiko Schocher
2009-07-17 22:09 ` Wolfgang Denk
2009-07-17 22:26   ` ksi at koi8.net
2009-07-17 22:39     ` Wolfgang Denk
2009-07-17 23:16       ` ksi at koi8.net
2009-07-18  8:17         ` Heiko Schocher
2009-07-18  8:11       ` Heiko Schocher
2009-07-18  8:07   ` Heiko Schocher
2009-07-18 10:18     ` Wolfgang Denk

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox