[U-Boot] [PATCH v2 1/2] imx: mx6: add get_cpu_speed_grade_hz func to return MHz speed grade from OTP

[Christian Gmeiner]

2015-05-15 15:20 GMT+02:00 Stefano Babic <sbabic at denx.de>:
> Hi Christian,
>
> On 15/05/2015 09:53, Christian Gmeiner wrote:
>> Hi
>>
>> 2015-05-14 7:08 GMT+02:00 Tim Harvey <tharvey at gateworks.com>:
>>> The IMX6 has four different speed grades determined by eFUSE SPEED_GRADING
>>> indicated by OCOTP_CFG3[17:16] which is at 0x440 in the Fusemap Description
>>> Table. Return this frequency so that it can be used elsewhere.
>>>
>>> Note that the IMX6SDLRM and the IMX6SXRM do not indicate this in the
>>> their Fusemap Description Table however Freescale has confirmed that these
>>> eFUSE bits match the description within the IMX6DQRM and that they will
>>> be added to the next revision of the respective reference manuals.
>>>
>>> These have been tested with IMX6 Quad/Solo/Dual-light 800Mhz and 1GHz grades.
>>>
>>> Signed-off-by: Tim Harvey <tharvey at gateworks.com>
>>> ---
>>>  arch/arm/cpu/armv7/mx6/soc.c              | 41 +++++++++++++++++++++++++++++++
>>>  arch/arm/include/asm/arch-mx6/sys_proto.h |  1 +
>>>  2 files changed, 42 insertions(+)
>>>
>>> diff --git a/arch/arm/cpu/armv7/mx6/soc.c b/arch/arm/cpu/armv7/mx6/soc.c
>>> index dd34138..71fa1fb 100644
>>> --- a/arch/arm/cpu/armv7/mx6/soc.c
>>> +++ b/arch/arm/cpu/armv7/mx6/soc.c
>>> @@ -83,6 +83,47 @@ u32 get_cpu_rev(void)
>>>         return (type << 12) | (reg + 0x10);
>>>  }
>>>
>>> +/*
>>> + * OCOTP_CFG3[17:16] (see Fusemap Description Table offset 0x440)
>>> + * defines a 2-bit SPEED_GRADING
>>> + */
>>> +#define OCOTP_CFG3_SPEED_SHIFT 16
>>> +#define OCOTP_CFG3_SPEED_800MHZ        0
>>> +#define OCOTP_CFG3_SPEED_850MHZ        1
>>> +#define OCOTP_CFG3_SPEED_1GHZ  2
>>> +#define OCOTP_CFG3_SPEED_1P2GHZ        3
>>
>> Note: 0x3 is defined as reserved (IMX6DQRM Rev 2, 06/2014).
>>
>
> That means that the manual is buggy, if the SOC returns exactly this
> value ;-)

yep :)

>
>>> +
>>> +u32 get_cpu_speed_grade_hz(void)
>>> +{
>>> +       struct ocotp_regs *ocotp = (struct ocotp_regs *)OCOTP_BASE_ADDR;
>>> +       struct fuse_bank *bank = &ocotp->bank[0];
>>> +       struct fuse_bank0_regs *fuse =
>>> +               (struct fuse_bank0_regs *)bank->fuse_regs;
>>> +       uint32_t val;
>>> +
>>> +       val = readl(&fuse->cfg3);
>>> +       val >>= OCOTP_CFG3_SPEED_SHIFT;
>>> +       val &= 0x3;
>>> +
>>> +       switch (val) {
>>> +       /* Valid for IMX6DQ */
>>> +       case OCOTP_CFG3_SPEED_1P2GHZ:
>>> +               if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D))
>>> +                       return 1200000000;
>>> +       /* Valid for IMX6SX/IMX6SDL/IMX6DQ */
>>> +       case OCOTP_CFG3_SPEED_1GHZ:
>>> +               return 996000000;
>>> +       /* Valid for IMX6DQ */
>>> +       case OCOTP_CFG3_SPEED_850MHZ:
>>> +               if (is_cpu_type(MXC_CPU_MX6Q) || is_cpu_type(MXC_CPU_MX6D))
>>> +                       return 852000000;
>>> +       /* Valid for IMX6SX/IMX6SDL/IMX6DQ */
>>> +       case OCOTP_CFG3_SPEED_800MHZ:
>>> +               return 792000000;
>>> +       }
>>> +       return 0;
>>
>> Do we really need the ifs inside the cases? The speed grading fuse
>> value is read only and
>> we must simply life with the value we read back. So I would drop the
>> is_cpu_type(..) thing.
>
> Function does not returns the value of the fuse, else it should returns
> val. The value is parsed for consistency, and if it is not, 0 means a
> failure. IMHO this interpretation is correct - I would agree with you
> only if the return value would be the read value, but it is not.
>

If all mx6 variants use the same otp register/values, then the
ifs are _NOT_ needed. I hope that later testings/doc updates/etc
proofs it and we can get rid of the superfluous ifs.

If they are different then I can life with it.

greets
--
Christian Gmeiner, MSc

https://soundcloud.com/christian-gmeiner