On 20 February 2012 12:12, andrzej zaborowski balr...@gmail.com wrote:
On 17 February 2012 19:21, Peter Maydell peter.mayd...@linaro.org wrote:
Does anybody know what the distinction between these two constants
is supposed to be? They were introduced by commit 29885477
back in 2008... (Also, why INTR and GC?)
Apparently the intent was for the revision to be used to
enable/disable different features of the I2C module. OMAP2_GC_REV
should be the minimum revision where the omap2 Global Call interrupt
is available. INTR seems to be the feature that enables the interrupt
status to be read resetting some interrupt flags. There's an
assumption that every revision is backwards compatible and features
never go away.
Ah, I see. The difficulty with this is that we don't have separate
docs for the I2C module which indicate what all the new features
are and when they were introduced, so we only have the TRMs.
I think another assumption was that between the I2C module revision
1.1 and 3.4 other features were introduced gradually, so
omap1/omap2/omap3 resolution would not be enough.
I have neither of the TRMs at hand but I downloaded the TRM for
omap35x which features i2c module 3.1 and it looks like the current
code is not even handling the masking of all the interrupts of that
version correctly (there are 14 in 3.1)
There are a lot of omap3 changes in my patchset which I'm
trying to clean up; possibly that's one of them. Incidentally
I don't think the 35x does have 3.1 of this module, that's just
an example indicating the format of the revision field. If you
boot a beagle board (36xx but that's the same as 35xx) it will
say omap_i2c omap_i2c.1: bus 1 rev4.0 at 2600 kHz indicating
that this is a rev 4.0 I2C module.
(I have no idea why TI don't put the actual submodule revisions
in their TRMs, it's not like you can't find it out from the hardware.)
The patchset I'm trying to clean up goes for:
#define OMAP1_INTR_REV 0x11
#define OMAP2_INTR_REV 0x34
#define OMAP3_INTR_REV 0x3c
#define OMAP3630_INTR_REV 0x40
although I'm not sure whether 'INTR' makes much sense rather
than 'I2C' or something. I'll stick with these constants if nobody
has a better idea, but I was wondering if anybody could remember
the history behind the current constant names...
I guess it would be a change of semantics rather than clean-up, but it
may a good idea. On the other hand I don't see much benefit from
using those defines instead of hardcoding the revision values in the
init functions.
Well, the #defines mean that when the code which actually implements
the only in this rev or better is doing an if() it isn't using a
hardcoded constant.
FWIW the patch I have which makes this a sysbus device makes the
I2C module revision be a qdev property, so the omap2 initialisation
function creates two i2c devices with the revision property set to
0x34, and so on.
-- PMM