[U-Boot] [PATCH v3] i2c: merge all i2c_reg_read() and i2c_reg_write() into inline functions

[ksi at koi8.net]

On Tue, 16 Dec 2008, Timur Tabi wrote:

> ksi at koi8.net wrote:
>
>> That looks similar. But why do you want to remove i2c_set_bus_num()? I
> think
>> it would be less work to keep it.
>
> Perhaps, but it would be even better to get rid of it.  IMHO, it's a
> kludge.  It
> was a hack added to allow existing I2C routines to function while adding
> minimal
> support for multiple buses on those platforms that needed it.

I have nothing against changing parameters list for i2c_read/write(), it's
just more work and less elegant.

>> This way you can leave 90% or so of
>> existing I2C code unchanged by setting bus number to 0 at init.
>
> I only intend on exporting the multiple-bus versions of the I2C function
> if
> CONFIG_I2C_MULTI_BUS is defined.

That looks messy... Why would we use two different versions if we can make
everything uniform?

>> My idea is to have global bus number variable in a single place and a
> single
>> i2c_set_bus_num() that can be excluded for most boards with a single
> bus
>> with #ifdef...
>
> We already have something like that.  A global variable is inconvenient
> because
> every time you want to access the bus, you need to do something like
> this:
>
> 	bus = i2c_get_bus_num();
> 	i2c_set_bus_num(x);
> 	i2c_write(...)
> 	i2c_set_bus_num(bus);
>
> We need to save/restore the current bus number, because the I2C
> command-line has
> the concept of a

Eh, you can just set bus number every time you're gonna do i2c read/write...
That i2c_get_bus_num() doesn't make any sence at all. Just set bus number
every time you access i2c device. U-boot is single-task so there is no other
process that can change it from under you and you do not save anything with
checking that bus number. I can't see any practical use for
i2c_get_bus_num() except for showing it with an interactive command. That
is, in turn, is also redundant -- one would not save any keystrokes by
typing something like "i2c bus" and not typing "i2c bus X" if that X is what
he wants. Just type the second command every time when in doubt and you
won't have to type the first one :)

Also, for all boards with a single I2C bus we can assume bus=0 so there is
no need for bus-related functions/commands at all...

>> Then, we could use some kind of array of I2C structures each
> containing
>> pointers to appropriate i2c-{read,write,probe,init}() functions with
> generic
>> i2c functions just calling those pointers using bus number as index
> into
>> that array.
>
> Sounds complicated.

What is complicated? It is something like 3-4 lines of code per I2C bus.
Look how e.g. NAND subsystem is initialized...

And, BTW, this initialization can even go into include/configs/$(BOARD).h.

>> That would allow for unlimited number of different adapters for any
> board.
>
> Ah, now this is something else entirely.  I don't think U-boot supports
> this at
> all.  I think you're being too ambitious.  It's a noble idea, and I
> think U-boot
> should support it, but I think we need to simplify the support for
> multiple
> buses first.

It ain't rocket science :) The reason why I'm doing this is exactly what you
said -- U-boot does NOT support it at all. And I do really need this and I
think I'm not the only one.

>> Initial code for initializing such an array would have to go into each
> and
>> every $(BOARD).c board specific file.
>
> Ugh.

Ah, that's not biggie. And I volunteer to do this and come up with patches.

---
******************************************************************
*  KSI at home    KOI8 Net  < >  The impossible we do immediately.  *
*  Las Vegas   NV, USA   < >  Miracles require 24-hour notice.   *
******************************************************************