[U-Boot] [PATCH v2 3/4] imx: mx6: add display of CPU temperature grade in print_cpuinfo()

[Tim Harvey]

On Fri, May 15, 2015 at 6:24 AM, Stefano Babic <sbabic at denx.de> wrote:
> Hi Tim,
>
> On 15/05/2015 15:16, Tim Harvey wrote:
>> On Fri, May 15, 2015 at 1:14 AM, Christian Gmeiner
>> <christian.gmeiner at gmail.com> wrote:
>>> 2015-05-14 7:11 GMT+02:00 Tim Harvey <tharvey at gateworks.com>:
>>>> When CONFIG_IMX6_THERMAL is defined print the CPU temperature grade info
>>>> along with the current temperature.
>>>>
>>>> Before:
>>>>  CPU:   Temperature 42 C
>>>>
>>>> After:
>>>>  CPU:   Automotive temperature grade (-40C to 125C) at 42C
>>>>  CPU:   Industiral temperature grade (-40C to 105C) at 42C
>>>>  CPU:   Extended Commercial temperature grade (-20C to 105C) at 42C
>>>>
>>>> Signed-off-by: Tim Harvey <tharvey at gateworks.com>
>>>> ---
>>>> v2:
>>>>  - moved display of CPU temperature grade to own patch and combined with the
>>>>    current temperature from the thermal sensor driver
>>>>  - add example output to description
>>>>
>>>> Signed-off-by: Tim Harvey <tharvey at gateworks.com>
>>>> ---
>>>>  arch/arm/imx-common/cpu.c | 27 +++++++++++++++++++++++----
>>>>  1 file changed, 23 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/arch/arm/imx-common/cpu.c b/arch/arm/imx-common/cpu.c
>>>> index 6b20482..f1b49bb 100644
>>>> --- a/arch/arm/imx-common/cpu.c
>>>> +++ b/arch/arm/imx-common/cpu.c
>>>> @@ -16,6 +16,7 @@
>>>>  #include <asm/arch/clock.h>
>>>>  #include <asm/arch/sys_proto.h>
>>>>  #include <asm/arch/crm_regs.h>
>>>> +#include <imx_thermal.h>
>>>>  #include <ipu_pixfmt.h>
>>>>  #include <thermal.h>
>>>>  #include <sata.h>
>>>> @@ -146,7 +147,7 @@ int print_cpuinfo(void)
>>>>
>>>>  #if defined(CONFIG_MX6) && defined(CONFIG_IMX6_THERMAL)
>>>>         struct udevice *thermal_dev;
>>>> -       int cpu_tmp, ret;
>>>> +       int cpu_tmp, minc, maxc, ret;
>>>>  #endif
>>>>
>>>>         cpurev = get_cpu_rev();
>>>> @@ -172,16 +173,34 @@ int print_cpuinfo(void)
>>>>  #endif
>>>>
>>>>  #if defined(CONFIG_MX6) && defined(CONFIG_IMX6_THERMAL)
>>>> +       puts("CPU:   ");
>>>> +       if (!is_cpu_type(MXC_CPU_MX6SX)) {
>>>> +               switch (get_cpu_temp_grade(&minc, &maxc)) {
>>>> +               case TEMP_AUTOMOTIVE:
>>>> +                       puts("Automotive temperature grade ");
>>>> +                       break;
>>>> +               case TEMP_INDUSTRIAL:
>>>> +                       puts("Industrial temperature grade ");
>>>> +                       break;
>>>> +               case TEMP_EXTCOMMERCIAL:
>>>> +                       puts("Extended Commercial temperature grade ");
>>>> +                       break;
>>>> +               default:
>>>> +                       puts("Commercial temperature grade ");
>>>> +                       break;
>>>> +               }
>>>> +               printf("(%dC to %dC)", minc, maxc);
>>>> +       }
>>>
>>> For the additions above we do not need CONFIG_IMX6_THERMAL.
>>
>> Hi Christian,
>>
>> Thanks for the review.
>>
>> Its true that CONFIG_IMX6_THERMAL is not needed for cpu temperature
>> grade but I felt that there would be resistance in general to adding
>> additional lines in print_cpuinfo without allowing them to be easily
>> disabled.
>
> Agree on this point.
>
>> I felt that temperature grade and current temperature made
>> sense to go together in this fasion.
>>
>> Looking over this though did remind me that now that Freescale has
>> told me that the IMX6SX has the same OTP registers/definitions for
>> temperature grade programmed I can remove the skip for MX6SX. I would
>> however sure like to have someone with an actual MX6SX enable that
>> code and give us some testing feedback.
>>
>
> Right. My proposal is to merge the currrent patchset, enabling for
> IMX6SX again when someone will test it.
>
> Best regards,
> Stefano Babic
>

Stefano,

Yes, that sounds like the best approach. What were your thoughts on
enabling the second CPU: line just to display the temperature grade if
CONFIG_IMX6_THERMAL was not enabled?

I'm surprised someone on this list hasn't already tested this on IMX6SX.

Fabio - certainly you must have a reference board with an IMX6SX and
could enable/test that the CPU markings agree with what is
detected/displayed via OTP?

Tim