[i2c] Identifying I2C busses

[David Brownell]

On Tuesday 06 March 2007 6:53 am, Jean Delvare wrote:
> Hi David,
> 
> On Mon, 5 Mar 2007 19:22:36 -0800, David Brownell wrote:
> [Lots of things]
> 
> I'm skipping the context so that we can focus again on the important
> points. Quite a challenge when replying to a 28 kB message... So I'm

Yeah, that's why I split my response to your long message into chunks
focussed on different topics.  :)

I'd have been more brief, except you were still overlooking things I'd
said before ... which I took as an indication that you didn't understand
the ramifications of a few things.


> starting a new thread and I'll attempt to summarize the situation.
> 
> * You seem to consider as important that Linux numbers the I2C busses
> the exact same way as your board schematics do. I didn't realize that
> before. I thought that bus numbering was just a way for you to
> connect chip declarations to their respective busses, but now I seem to
> understand that the possibility to "request" an I2C bus number from
> i2c-core is a feature you are interested in per se. Right?

What's fundamentally important is having some stable identifier that
corresponds to those numbers in the board schematics.  You were pushing
back on the use of numbers, with what seemed to me to be clearly unfounded
obections.  So naturally I push back on the unfounded stuff.  :)

I'm not interested in that "claim this bus number" mechanism per se,
although it seems (in some ways) simpler to use than the mechanism you
had eventually suggested:  coupling things to the device node.

On the other hand, having such a "claim" mechanism isn't free either.
And there's not a lot of difference between the context of the bus
number being "i2c_adapter.nr" a.k.a. "platform_device.id" (or zero, when
id == -1) ... or "&platform_device.dev", or "platform_device.dev.bus_id.
Except that it's much easier to pass around a number.  So I'm still thinking
about using the device node; it's an interesting notion.


> * I have to admit that platform data associated with the I2C busses is
> not the natural place to store I2C chip definitions. I guess I was
> using the platform data as a kind of transport means, but it would
> probably be more of a hack than anything else. So my solution doesn't
> actually fly - or at least it wasn't as nice as I thought it was.

OK.


> * You mostly convinced me that your approach was correct. One thing
> which worries me though: is there any other subsystem in the whole
> Linux tree which works that way (mixing requested identification
> numbers with dynamically assigned ones)? What makes I2C different?

SPI does it that way.  The board_info was essentially a clone of the
mechanism used there ... although since it's so small, the SPI core
is always statically linked.  That's not the best model for I2C, where
the core is several times larger.  (And some folk even objected to
that restriction for SPI ... not enough to supply a patch though!)

At the time I did that SPI stuff I mentioned that I was looking for
a model that would also work for I2C.  I suspect you didn't take
that seriously at the time.  ;)

I2C is not different in that respect:  neither serial bus supports
a very useful level of autoconfiguration.  So both need a way to
declare devices, including both "early" and "late" modes, etc.


> * I'm very curious about how the stacked-board designs you mentioned
> don't have problems with I2C address collisions if they work the way
> you described.

They're designed to work together -- without collisions.  Stacking
provides a lot of design, manufacturing and packaging flexibility.

Many of those CPU card designs seem to be geared, as I implied, to
folk designing semicustom systems.  The skills involved in making a
cpu card (6-8 layer PCB design, SMT manufacturing, board testing)
are not the same as those involved in small runs of a custom I/O box
to surround such a card.

(Related:  how do DRAM cards avoid collisions on their I2C EEPROMS?
I'm guessing three pins in each slot get hooked up to the EEPROM
address select pins, and they're wired on the baseboard so each slot
has a unique number.  Similar approaches can work for other kinds
of board stack, when an EEPROM is used to identify each card and
maybe record factory-tested calibration data.)

 
> * You seem to agree that bus_num could be moved out of struct
> i2c_board_info. Good. The sole fact that its value is ignored in a
> number of cases is IMHO a good indication that it indeed doesn't
> belong there. Which means that we then need a way to associate such a
> structure with a bus number, for the early I2C device declaration case.

Which is why I said it could be viewed as a cleanup.  :)

Pending any change to using bus numbers directly as stable IDs,
I'll just pass that to i2c_register_board_info.


> On a related topic, I am not fond of your "list of arrays of variable
> size" approach to store the I2C device declarations. This seems to be
> more complex than needed. Can't we simply go with a list of individual
> records?

That could work.  I don't see much point to that change though...


> Here's what I have in mind (beware, I changed the struct 
> names):
> 
> struct i2c_device_info {
> 	char		driver[KOBJ_NAME_LEN];
> 	char		type[I2C_NAME_SIZE];
> 	unsigned short	dev_addr;
> 	void		*platform_data;
> 	int		irq;
> };
> 
> struct i2c_board_info {
> 	struct list_head	list;
> 	short			bus_num;
> 	struct			i2c_device_info;
> };
> 
> And __i2c_board_list would be a list of struct i2c_board_info. This
> should make your code somewhat more simple, at the price of additional
> calls to i2c_register_board_info() and maybe some more memory
> fragmentation, but I don't think this is a major problem. What do you
> think?

I guess I still don't see a real advantage.  It hits the malloc heap more
often at board init time; a handful of loop instructions move from scanning
that list into setting it up.  If it bothers you deeply, I can change
that to provide a list of singletons.


> A different approach is one list per bus number, that would scale
> better (no common pool), but that makes things a bit more complex
> again, and as you underlined, we don't expect thousands of devices to
> be declared anyway so scalability probably doesn't matter. I guess
> real-world scenarii have a max of 2 I2C busses?

I've seen chips with three:  two lowspeed ones, and a highspeed one.
Regardless of how they're factored, I personally have never happened
across a production board with even a dozen I2C devices.

 
> * I am not happy with the idea of having one function named
> i2c_add_adapter, and one named i2c_register_adapter (i.e. almost the
> same name) doing something similar but different, without this
> difference being visible in their name. We need to come up with
> something better than that. Either we can have a single function, or we
> need to rename i2c_register_adapter to something which expresses its
> difference to i2c_add_adapter. i2c_add_numbered_adapter or something.

Your notion of using the device handle as the (relatively) stable
bus ID would let there be only one function...


> * With regards to i2c-mainboard, wouldn't it be more simple to just
> make it mandatory to build i2c-core into the kernel when you need the
> early device declaration process? I seem to remember that this is a
> common practice in the embedded world anyway, because you need I2C
> early in the boot process.

Simpler ... yes.  It's common regardless of whether the board needs I2C
early:  modular drivers cost more space than non-modular ones, and in any
case one doesn't want the driver for a key system bus to be modular.
But common doesn't mean universally desirable.

Also, I'm not sure that'd be a reasonable restriction.  It'd be no skin
off my back, for sure, but I know some folk would object.  And how would
you force static linkage of the i2c-core module in those cases?  It'd be
tricky to guarantee "make allmodconfig" works on all the relevant boards...

So despite the minor mess, I don't see an easy way around that being
a separate file.  Moving it out of i2c-core.c was a cleanup.  :)

 
> * Your implementation doesn't seem to support I2C bus number
> reservation if there is no chip on that bus. Isn't it going to be
> confusing? Don't we want a function in i2c-core that lets the platform
> code explicitly bump __i2c_first_dynamic_bus_num, rather that deducing
> it from the i2c devices definitions?

Good point, but a simpler fix is just to let i2c_register_boardinfo()
do that bumping even when there are no devices to register.  That falls
out easily from making the bus number be a parameter to that function
rather than a member of the i2c_board_info structure.

- Dave



> 
> Thanks,
> -- 
> Jean Delvare
>