Re: [Qemu-devel] omap_i2c: distinction between OMAP2_INTR_REV and OMAP2_GC_REV?
Hi, On 17 February 2012 19:21, Peter Maydell peter.mayd...@linaro.org 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
Re: [Qemu-devel] omap_i2c: distinction between OMAP2_INTR_REV and OMAP2_GC_REV?
On 20 February 2012 13:12, andrzej zaborowski balr...@gmail.com wrote: 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 Sorry, the revision is not given in the TRM, 3.1 is an example. Cheers
Re: [Qemu-devel] omap_i2c: distinction between OMAP2_INTR_REV and OMAP2_GC_REV?
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
[Qemu-devel] omap_i2c: distinction between OMAP2_INTR_REV and OMAP2_GC_REV?
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?) The patchset I'm trying to clean up goes for: #define OMAP1_INTR_REV0x11 #define OMAP2_INTR_REV0x34 #define OMAP3_INTR_REV0x3c #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... thanks -- PMM
