Hello ksi,
k...@koi8.net wrote:
> On Sat, 14 Feb 2009, Heiko Schocher wrote:
>
>
>> Hello ksi,
>>
>> k...@koi8.net wrote:
>>
>>> On Fri, 13 Feb 2009, Heiko Schocher wrote:
>>>
>>>> k...@koi8.net wrote:
>>>>
>>>>> Signed-off-by: Sergey Kubushyn <k...@koi8.net>
>>>>> ---
>>>>> diff -purN u-boot-i2c.orig/drivers/i2c/soft_i2c.c
>>>>> u-boot-i2c/drivers/i2c/soft_i2c.c
>>>>> --- u-boot-i2c.orig/drivers/i2c/soft_i2c.c 2009-02-12
>>>>> 10:43:41.000000000 -0800
>>>>> +++ u-boot-i2c/drivers/i2c/soft_i2c.c 2009-02-12 10:46:00.000000000
>>>>> -0800
>>>>> @@ -1,4 +1,8 @@
>>>>> /*
>>>>> + * Copyright (c) 2009 Sergey Kubushyn <k...@koi8.net>
>>>>> + *
>>>>> + * Changes for multibus/multiadapter I2C support.
>>>>> + *
>>>>> * (C) Copyright 2001, 2002
>>>>> * Wolfgang Denk, DENX Software Engineering, w...@denx.de.
>>>>>
>>>> [...]
>>>>
>>>> The following patch is based on your patches without 7/12 and
>>>> adds multibus support for the soft_i2c driver without doing such
>>>> a big change as you did. Maybe it is not yet perfect, because
>>>> it is just a fast try, but I think we should go this way. What
>>>> do you/others think?
>>>>
>>> The reason behind this patch is making SEVERAL different SOFT_I2C ADAPTERS
>>> available. Not BUSSES but separate PHYSICAL I2C ADAPTERS made of different
>>> pin pairs from different chips.
>>>
>> This you can also do with "my" suggestion ...
>>
>>
>>> OK, please explain how are you going to make different functions for
>>> different adapters? Let's say you want to use 2 on-SoC GPIO pins for
>>>
>> You can do now the following for example in your
>> include/configs/MPC8548CDS.h example:
>>
>> you only have to define
>>
>> #define I2C_SDA(bit) (printf("hwadap: %d sda1: %d", cur_adap_nr->hwadapnr,
>> bit))
>>
>> if this is a real driver you can make a function in your board code
>> say (just a fast thought):
>>
>> void i2c_soft_sda (int bit)
>> {
>> switch(cur_adap_nr->hwadapnr) {
>> case 0:
>> /* adapter specfic code 0 */
>> break;
>> case 1:
>> /* adapter specfic code 1 */
>> break;
>> [...]
>> }
>> }
>>
>> and define in config file
>>
>> #define I2C_SDA(bit) i2c_soft_sda (bit)
>>
>
> That means you have to make changes in two places instead of one -- config
> file AND $(BOARD).c. Also you use functions instead of macros and you can
>
Yes. But that was just a thought, it should be possible to do this also
in macros.
> NOT make them inline because they come from a separate object file. This
> essentially defeats the very purpose of that common soft_i2c.c driver. If
> you want to make functions for bitbanged I2C into the $(BOARD).c there is no
> reason to have them as a base for that driver. It is much more logical to do
>
Maybe more logical, but not needed.
> everything in reverse, i.e. instead of having soft_i2c.c as a bona fide
> drivers and those I2C_SDA and friends as its building blocks make those
> i2c_soft_sda() etc. in each and every $(BOARD).c into primary entities and
> build the actual driver in the $(BOARD).c itself. Just convert that
> soft_i2c.c into a header file with macros for real functions (soft_i2c_read
> etc.) and instantiate them in the $(BOARD).c.
>
> The only problem with that is it breaks uniformity and makes another mess.
>
Just, if we do this, but we don;t need to do it so.
> The whole idea was to bring _ALL_ I2C drivers to a single place and make
> them totally transparent and uniform. Something like e.g. Linux VFS.
>
> And remember, the devil is in details. How are you going to assign
> (initialize) that innocent looking "cur_adap_nr->hwadapnr"? How are you
> going to work on an adapter other that "current" in a situation when you can
> NOT change "current" adapter (e.g. perform all I2C layer initialization
> while still running from flash?) Remember, this is plain C and there is no
>
Yes, good point. But do we need more then one i2c adapter when running
from flash? I see only one reason to use i2c when running from flash:
accessing SPD EEprom ... and this "bus" could always be the first
hw adapter. All other accesses to i2c should be moved to run when
we are in ram.
> "this" pointer... And that is just a tip of an iceberg...
>
> And the million dollar question -- what is the potential gain?
>
I want to avoid such a big change in soft_i2c.c. Also if you have
4 bitbang instances with your version you have 3 times more code.
But if others are on your side, I have no problem with your approach.
>
>>> adapter #0, 2 GPIOs from a PCI-PCI bridge for adapter #1, and 2 pins from
>>> some chip sitting behind that bridge for adapter #2 if all those pin sets
>>> are accessed totally different. I won't even start about using pins from
>>> different chips for SDA and SCL (let's say you only have one GPIO available
>>> on your SoC and another one on PCI Bridge.)
>>>
>>> What your patch creates is just aliases to the SAME physical adapter.
>>>
>> No, it is not! I only use the same functions, but in the board
>> specific code it is possible to made a switch and access
>> the Pins where ever they are.
>>
>
> You are adding unnecessary complexity to the code. And you break uniformity.
>
not really.
> Those defines in config/soft_i2c.c make real inline functions at _COMPILE_
> time. Your approach shifts it to _LINK_ time. It also makes those drivers
> come from 3 places ($(BOARD).c, include/configs/$(BOARD).h, and soft_i2c.c)
>
Thats not really a problem.
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
