[U-Boot-Users] PATCH: Add command support for multiple I2C controllers

[Ben Warren]

Wolfgang,

Thanks for the feedback.  Comments below.

regards,
Ben

On Thu, 2006-05-18 at 00:52 +0200, Wolfgang Denk wrote:
> In message <1147893695.16780.239.camel at saruman.qstreams.net> you wrote:
> > 
> > Attached is a second stab at a patch to allow access to multiple I2C
> > controllers.  It enhances the command set by adding the following
> > command, which changes the 'current' I2C bus.  Further write, read,
> > probe etc. commands will deal with the newly selected bus:
> > 
> > ibus [bus_index] [speed in Hz]
> 
> No. Please read my posting again. I want to see this as compatible as
> possible with other commands that operate  on  devices  connected  to
> busses. We use "ide dev ...", so I want to see "i2c dev" here, too.
> 
> As mentioned before, in the long term all i2c related commands should
> become sub-commands to the new "i2c" command.
> 
I understand and agree with your reasoning for moving all I2C commands
to a separate tree.  On the other hand, I'm very focused on your goal of
keeping the interface small and ALWAYS maintaining backwards
compatibility.  If you'd like me to move all I2C commands to a separate
tree, it should be trivial, but makes for a bigger package.  Please
advise.  
> > In addition, the logic for ignoring I2C devices during a probe command
> > has been enhanced to work with multiple controllers.  The software has
> > been tested with new and old configurations (i.e. is backwards
> > compatible).
> > 
> > Old way of ignoring devices:
> > 
> > #define CFG_I2C_NOPROBES {0x11, 0x22}
> > 
> > New way:
> > 
> > #define CFG_I2C_MULTI_NOPROBES {0x11, 0x22, 0xff, 0x33, 0x44, 0xff}
> 
> Argh... This is pretty unreadable. Can';t we just  use  an  array  of
> lists?
> 
I agree that at first glance this is unreadable.  However, it is quite
efficient and works well with macros.  I played around with lots of
different data structures, but I've usually found multi-dimensional
arrays in C to be more trouble than they're worth, especially if you're
using macros and don't know the size ahead of time.  Keeping things as a
1-D array allows the use of sizeof() to determine the size of the list,
not just the size of the pointer.  Maybe you can teach me something here
- even though I've been using C for many years, there are always new
things to learn...  Here's a simpler approach that you may think is OK:
	
#define I2C_DELIM  /* or something like that */
#define CFG_I2C_MULTI_NOPROBES {0x11, 0x22, I2C_DELIM, 0x33, 0x44 ...}


> Best regards,
> 
> Wolfgang Denk
>