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

Wolfgang Denk wd at denx.de
Tue Dec 16 21:49:54 CET 2008


Dear Timur,

In message <4947F8B4.8070804 at freescale.com> you 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!

Hm... what exactly is broken with the concept of  having  a  "current
device"  or  a  "current  bus"?  We  use it elasewhere, too (like for
selection IDE or S-ATA devices and such), and so far I am  not  aware
of fundamental issues because of that.

You sound as if we were designing a system where tens of  independend
users  could  log in simultaneously and concurrently access different
devices on different busses.

But this  is  NOT  the  case.  We're  strictly  single  user,  single
threaded,  and  when one command is runnign we are certain that there
cannot be any interference from other I2C accesses.

> > 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.

Not really. It is really sufficient to set the bus for each  command.
There  is  no  need  to  save and restore any previous state, because
accesses cannot be nested because of the way how U-Boot is designed.

> > 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.

The design has been discussed before, and I see little reason why we
should not throw all this away and come up with something new, more
complicated.

> 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.

Really? Where exactly does such a thing happen?

I tend to call this a bug that needs to be fixed, if there is ssuch
code.

> 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.

This statement is probably correct: I don't share your point of  view
here, either.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Be wiser than other people if you can, but do not tell them so.
                                       -- Philip Earl of Chesterfield


More information about the U-Boot mailing list