Re: [PATCH v2 08/11] OMAP4: Adding dev atrributes to OMAP4 smartreflex hwmod data

["Cousson, Benoit" <b-cousson@ti.com>]

Hi Kevin,

Sorry for that late reply, I missed that email a little bit.

On 10/14/2010 8:56 PM, Kevin Hilman wrote:
> On Sat, 2010-09-25 at 18:21 +0530, Thara Gopinath wrote:
>> diff --git a/arch/arm/plat-omap/include/plat/control.h
>> b/arch/arm/plat-omap/include/plat/control.h
>> index 042eb6e..1e8f6ec 100644
>> --- a/arch/arm/plat-omap/include/plat/control.h
>> +++ b/arch/arm/plat-omap/include/plat/control.h
>> @@ -181,6 +181,18 @@
>>   #define OMAP3630_CONTROL_FUSE_OPP50_VDD2
>> (OMAP2_CONTROL_GENERAL + 0x0128)
>>   #define OMAP3630_CONTROL_FUSE_OPP100_VDD2
>> (OMAP2_CONTROL_GENERAL + 0x012C)
>>
>> +/* 44xx control efuse offsets */
>> +#define OMAP44XX_CONTROL_FUSE_IVA_OPP50                0x22C
>> +#define OMAP44XX_CONTROL_FUSE_IVA_OPP100       0x22F
>> +#define OMAP44XX_CONTROL_FUSE_IVA_OPPTURBO     0x232
>> +#define OMAP44XX_CONTROL_FUSE_IVA_OPPNITRO     0x235
>> +#define OMAP44XX_CONTROL_FUSE_MPU_OPP50                0x240
>> +#define OMAP44XX_CONTROL_FUSE_MPU_OPP100       0x243
>> +#define OMAP44XX_CONTROL_FUSE_MPU_OPPTURBO     0x246
>> +#define OMAP44XX_CONTROL_FUSE_MPU_OPPNITRO     0x249
>> +#define OMAP44XX_CONTROL_FUSE_CORE_OPP50       0x254
>> +#define OMAP44XX_CONTROL_FUSE_CORE_OPP100      0x257
>> +
>
> These names are not consistent with the TRM (ES2.x NDA TRM vL).  e.g.
> rather than MPU_OPP50, OPP1, the TRM has MPU_0, MPU_1.  However, the
> TRM is confusing here too since the register names are MPU_0,
> MPU_1, etc. but the field names in the register descriptions use
> OPP50, OPP100, etc.  What a mess.  We need a consistent naming for these.
>
> Benoit?

It is indeed a mess... these names are the latest ones used in the HW 
spec. But we are still wondering how to name that in the future.
OPP_LOW, OPP_MID, OPP_HIGH, OPP_TURBO and OPP_NITRO are now considered 
but I'm not sure it is really better.

Honestly I do prefer the OPP50, OPP100, OPP130, OPP166 that are 
providing the percentage wrt nominal frequency and that can scale 
easily. If we do introduce a OPP between OPP_MID and OPP_HIGH, I'm not 
sure anybody will be able to find a proper name whereas OPP_80 is 
straightforward.

In anycase, that will never match the HW spec or the TRM :-(

Benoit

>
> Also, based on the TRM, the offsets here don't look quite right.
> e.g. there is no MPU OPP at 0x240, so the OMAP4 the code reading these is
> using reserved/undocumented bits for the first OPP.
>
> Also, the single byte offsets here could use a better explanation, not
> necessarily here but in the omap4 code that reads the nvalues.  I expected that these
> would be single byte aligned to compact the address space and remove some unused
> space, but no, they seem to be arbitrarily shifted around.
>
> Kevin
>
>