[U-Boot] i2c: rework multibus/multiadapter functionality
Heiko Schocher
hs at denx.de
Tue Mar 24 09:10:28 CET 2009
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 at 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
More information about the U-Boot
mailing list