| some thoughts having read this:
|
| more should be said about how how 'smart' controllers should implement
| certain types of commands.  (are you assuming that the command type
| will be stuffed in the data, and that smart controller commands will
| grovel it from there?  That seems like it might be more painful than
| it needs to be.)

I don't know what you're talking about here.  The I2C bus specification
defines the bus cycles and how you send out a 7-bit or 10-bit address,
the read/write bit, and how data bytes are sent on the wire, but that's
it.  Any protocols to communicate with the destination device are specific
to that particular device.  Some require you send an address first then
read or write a single data byte.  Some have an implicit data pointer which
can be manipulated and transfer data in arbitrary sized blocks.  Some only
transfer one byte at a time.  Some require start between each byte transferred,
while some devices require a start only at the beginning of the operation.
I have seen many different combinations.

As for interrupt driven controllers:  you load the address, load the data to
be transferred, kick the controller to start the transfer and wait for an
interrupt.  When the fifo underflows/overflows or the dma count reaches
zero, you reload the device and kick it again.

| polling vs. interrupt driven operation: what are the assumptions about
| each?

Non-polled operation will block waiting for an interrupt.  Polled operation
will not block.  I suppose this should be made explicit in the manpage.

| What about reentrancy?  i.e. command currently being processed by the
| bus in interrupt-driven fashion, and some process sleeping for it, and
| some other code needing to execute an op from an interrupt context?

The driver should queue commands and execute them atomically, the
same way SCSI commands are handled.  Except I2C does not support the
equivalent of disconnect/reselect.

| This describes part of the interface used to issue commands, info
| about the internal interfaces would be useful.  (i.e., for 'dumb'
| drivers that need to hook into the generic framework.  for people
| writing drivers in general there's not enough there, even if you know
| about i2c...)

I was thinking about that, but since that bit was already in existence
I didn't really want to spend too much time documenting it.  I suppose
another manpage for the i2c_bus stuff would be a good idea.

| It makes sense to have a higher level interface, in addition to this
| 'drive these bytes' interface.  I.e., there are a bunch of standard
| operations used by many/most devices, and it's silly to have to recode
| them in each driver that needs them.

There are no such standards.  If there were my life would be much easier
and I would have spent much less time debugging individual i2c device
drivers.

| Read vs. write, and the array of i2o commands: How do you do a simple
| addressed read command from a device?  It's not at all clear what
| combination of operations you'd use for this, what the 'nops' are for,
| why you would chain operations together.

The `Motorola syntax' for EEPROM access is that you send an address byte
to the device then transfer a single data byte.  In that case you would do:

int
at76c651match(parent, cf, aux)
	struct device *parent;
	struct cfdata *cf;
	void *aux;
{
	struct i2c_attach_args *i2a = aux;
	struct i2c_op op[2];
	int8_t regaddr = AT_CHIPID; /* Address of CHIPID register */
	int16_t id;

	/* Check the address */
	if (i2a->ia_addr < AT_ADDR_BASE || i2a->ia_addr > AT_ADDR_MAX)
		return (0);

	/* 
	 * Check the version register:
	 *
	 * 1) address the register
	 */
	op[0].io_data = &regaddr;
	op[0].io_len = sizeof(regaddr);
	op[0].io_flags = I2CO_WRITE;

	/*
	 * 2) read 2 bytes of data
	 */
	op[1].io_data = &id;
	op[1].io_len = 2;
	op[1].io_flags = I2CO_READ;

	if (i2c_cmd(i2a->ia_ctlr, i2a->ia_addr, op, 2, I2C_POLL|I2C_REPSTART) 
		!= 0) {
#ifdef DEBUG
		printf("at76c651match: chipid read failure\n");
#endif
		return (0);
	}

	if (id != AT76C651_CHIPID) {
#ifdef DEBUG
		printf("at76c651match: id %x != %x mismatch\n",
			id, AT76C651_CHIPID);
#endif
		return (0);
	}
	/* We should also match a specific bus... */
	return (1);
}

This particular device requires a START between each byte transfer, hence
the I2C_REPSTART (which forces the driver for the IBM IIC controller to 
transfer bytes one at a time because the hardware cannot be set to insert
unecessary START phases).  If you are interested I have several other
drivers that show completely different communications protocols.

| If the goal is to indicate read vs write parts of a single command w/
| multiple op structures, then why do you need all of those flags for
| each?  (the flags seem like they're 'per-command'.)

The 8-bit and 10-bit addresses overlap but are disjoint, so you need to
be able to distinguish one from the other.  The only other flags are POLL,
like the SCSI POLL flag, and the I2C_REPSTART flag to specify a slight
variation in the communications protocol.  You can get the same effect as
I2C_REPSTART by having a separate operation for each byte, but that's more
difficult to code than just adding a flag.

| Have you read the SMBus spec?

This is I2C bus.  What does SMBus have to do with this?

| Alas, i'm going to be travelling most of this week, so i won't have
| time to read much more you might write on this...  but from where i
| sit, what you've presented here is fairly incoherent and doesn't
| obviously correspond to my undstanding of i2c and smbus devices...

Then I think you should read the I2C spec.

Eduardo