[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 messy... Why would we use two different versions if we can
> make
>> everything uniform?
>
> Because we already have something that makes it uniform, and it's
> broken.  The
> idea of having a "current i2c bus" that needs to be set before
> read/write
> operations can be performed is the broken part!

Eh, we don't have any uniformity. That "uniformity" we do have is only for a
trivial case of a single I2C bus. Everything else is a bunch of SoC specific
hacks made differently fo different platforms. The fsl-i2c.c e.g. uses local
bus number variable.

>> Eh, you can just set bus number every time you're gonna do i2c
> read/write...
>
> Not with the current i2c command line.  We would need another global
> variable in
> the i2c command line code to store what IT thinks is the current bus.

That can be taken care of.

>> That i2c_get_bus_num() doesn't make any sence at all. Just set bus
> number
>> every time you access i2c device.
>
> That's risky.  Sooner or later, you will want to know what the current
> bus
> number is, at least for debugging or status purposes.

So what? Just use i2c_get_bus() for this... We can even add a string to
i2c_func structure for textual representation.

>> 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.
>
> Sounds to me like you haven't really looked at the U-Boot code.  There
> are
> plenty of places where one function does I2C operations, then calls
> another
> function that does its own.

So what? Does it make it multitasking? Can that another function preempt the
first one and change bus number from under it?

> I think all this boils down to one core disagreement we have: I think
> the idea
> of a "current" i2c bus is a bad one.

I can see nothing wrong with that but I also have nothing against changing
all i2c_{read,write,probe}() functions to take bus number as an argument.

I offered 4 possible scenarios and additional parameter to i2c functions was
one of them. Wolfgang said that current bus approach looks better than
others and I agree with him. But it is not rocket science to use an
additional parameter either. The only thing is it MUST be consistent, i.e.
we should NOT have 2 different sets of functions based on CONFIG_MULTIBUS or
whatever. If we are to make this change ALL boards must be multibus with
majority of them having bus count of 1.

Does anybody else have something to say on this? I'm going to write code so
let's make some decision. I don't want this to end up as a company hack and
making it properly affects a lot of U-boot...

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