i2c-omap.c:omap_i2c_idle() writes to nonexistent register on OMAP3630

Moving to qemu-linaro, should investigate and move as appropriate

Still happens, although the register is 0xe8 now. This was waiting for there to be a hwpack with a kernel for which we hadn't lost the ddeb, as it's basically undebuggable without debugsyms for the kernel.

It is only a beaglexm issue, incidentally.

This happens because the Linux omap_i2c driver omap_i2c_idle() routine thinks we have a register we don't have:
 if (dev->rev >= OMAP_I2C_REV_ON_4430)
  omap_i2c_write_reg(dev, OMAP_I2C_IRQENABLE_CLR, 1);
 else
  omap_i2c_write_reg(dev, OMAP_I2C_IE_REG, 0);

...because we're claiming a REVISION of 0x40 which Linux thinks means 4430. This in turn happens because qemu's beagle model initialises the omap3 code to be "omap3630", which is wrong according to the Beagle XM docs which say it has an omap3530. The qemu omap code doesn't model an omap3530.

I don't know if qemu is right about the I2C revision being 0x40 on the 3630 because for some reason TI think that the revision number is "TI internal data" despite it being freely available in the register if you have the hardware...

> the Beagle XM docs which say it has an omap3530

I was looking at the wrong doc; the Beagle is 3530 (apparently more or less identical to 3430 for marketing purposes); the XM is 3730/3630 (ditto).

> I don't know if qemu is right about the I2C revision being 0x40

It is; from real XM hardware:

[ 0.045898] i2c_omap i2c_omap.1: bus 1 rev4.0 at 2600 kHz

...so this is a bug in the kernel i2c-omap.c, and qemu is legitimately complaining about it.

Since this is a kernel issue I'm moving it over to "linux-linaro"; please let me know if I should have filed it somewhere else.

The specific kernel I've been looking at is:
 [ 0.000000] Linux version 2.6.37-1002-linaro-omap (buildd@actinidiaceae) (gcc version 4.5.2 (Ubuntu/Linaro 4.5.2-1ubuntu6) ) #5-Ubuntu SMP Thu Jan 20 02:19:36 UTC 2011 (Ubuntu 2.6.37-1002.5-linaro-omap 2.6.37)

from the 20110126 linaro omap3 snapshot+hwpack.

The underlying problem seems to be that TI extended the size of their revision register on the OMAP4430 version of the i2c peripheral unit, without maintaining the position of the revision LSB.

So, prior to OMAP4430, there was one 16-bit word at offset +0 defining the revision. On OMAP4430, there are two words and it seems the word with the same logical meaning as the one at +0 before, is now at +4.

This leads to a situation where the code can't probe just from the peripheral unit ID, because then it won't know beforehand which register(s) it is in.

In the existing driver, they go outside the peripheral unit's own ID and just in that one place to decide the register map, they use cpu_is_omap44xx() to decide if they will choose "old style" register map or "new style". The fact is, this DOES make the correct choice, because otherwise 3530 I2C would be broken since the OMAP4430 register layout is totally different.

That seems to lead to the conclusion that you can find 0x40 in the +0 register on OMAP3530 AND 0x40 in the +4 register on OMAP4430, the idea being that the extra 16 bits of revision numbering in the +0 register then lets you tell the difference.

The bug comes from an inconsistent test to use the new registers or not, by CPU type to select the register map and then by reported peripheral unit revision to decide whether to use them or not.

The attached patchset solves this and has been sent upstream.