Re: [Qemu-devel] omap_i2c: distinction between OMAP2_INTR_REV and OMAP2_GC_REV?

[andrzej zaborowski]

Hi,

On 17 February 2012 19:21, Peter Maydell <address@hidden> wrote:
> I'm looking at cleaning up some more omap3 patches, and have
> been working on the omap_i2c related ones. At the moment in
> omap_i2c.c there are the following #defines for the I2C module
> revision (as exposed via the revision register):
>
> #define OMAP2_INTR_REV 0x34
> #define OMAP2_GC_REV   0x34
>
> (plus a hardcoded 0x11 used by omap_i2c_init for omap1.)
> and it then uses both #defines apparently interchangeably
> when doing tests against the s->revision field.
>
> 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.

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)

>
> 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.

Cheers