Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function

[Joonyoung Shim <jy0922.shim@samsung.com>]

Hi Lukasz,

On 01/30/2015 05:14 PM, Lukasz Majewski wrote:
> Hi Eduardo, Abhilash,
> 
>> On Thu, Jan 22, 2015 at 06:02:07PM +0530, Abhilash Kesavan wrote:
>>> Hi Lukasz,
>>>
>>> On Thu, Jan 22, 2015 at 2:31 PM, Lukasz Majewski
>>> <l.majewski@samsung.com> wrote:
>>>> Hi Abhilash,
>>>>
>>>>> Hi Lukasz,
>>>>>
>>>>> On Mon, Jan 19, 2015 at 5:14 PM, Lukasz Majewski
>>>>> <l.majewski@samsung.com> wrote:
>>>>>> The exynos_map_dt_data() function must be called before
>>>>>> thermal_zone_of_sensor_register(), and hence provide tmu_read()
>>>>>> function, before it is needed.
>>>>>>
>>>>>> This change is driven by adding support for enabling
>>>>>> thermal_zoneX when it is properly initialized.
>>>>>>
>>>>>> One can read the mode of operation
>>>>>> at /sys/class/thermal/thermal_zone0/mode Such functionality was
>>>>>> missing in the of-thermal.c code.
>>>>>>
>>>>>> Reported-by: Abhilash Kesavan <a.kesavan@samsung.com>
>>>>>> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
>>>>>> ---
>>>>>>  drivers/thermal/samsung/exynos_tmu.c | 7 ++++---
>>>>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/thermal/samsung/exynos_tmu.c
>>>>>> b/drivers/thermal/samsung/exynos_tmu.c index 9d2d685..5d946ab
>>>>>> 100644 --- a/drivers/thermal/samsung/exynos_tmu.c
>>>>>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>>>>>> @@ -975,15 +975,16 @@ static int exynos_tmu_probe(struct
>>>>>> platform_device *pdev) platform_set_drvdata(pdev, data);
>>>>>>         mutex_init(&data->lock);
>>>>>>
>>>>>> +       ret = exynos_map_dt_data(pdev);
>>>>>> +       if (ret)
>>>>>> +               goto err_sensor;

It's enough to just return ret.

One more, i think to need to take out regulator enable codes from
exynos_map_dt_data. If not, can't restore about regulator when error
occurs.

>>>>>> +
>>>>>>         data->tzd =
>>>>>> thermal_zone_of_sensor_register(&pdev->dev, 0, data,
>>>>>> &exynos_sensor_ops); if (IS_ERR(data->tzd)) {
>>>>>>                 pr_err("thermal: tz: %p ERROR\n", data->tzd);
>>>>>>                 return PTR_ERR(data->tzd);
>>>>>>         }
>>>>>> -       ret = exynos_map_dt_data(pdev);
>>>>>> -       if (ret)
>>>>>> -               goto err_sensor;
>>>>>>
>>>>>>         pdata = data->pdata;
>>>>>
>>>>> I have been testing this along with your v5 patch set and am
>>>>> seeing incorrect temperature being reported at boot-up on
>>>>> exynos7.
>>>>
>>>> Does it show a maximal temperature value (0x1FF)?
>>>
>>> I did not print the current temperature register, but I remember the
>>> message showing ~105C. Will give you the register value when I test
>>> with more debug prints tomorrow.
>>>
>>>>
>>>>>  It looks
>>>>> like exynos_tmu_read gets called from
>>>>> thermal_zone_of_device_update during boot-up, now that we have
>>>>> it populated early. However, as the tmu initialization function
>>>>> has not been called yet it returns a wrong value. Does that
>>>>> sound correct ?
>>>>
>>>> No, this is a mistake. However, I'm wondering why on Exynos4/5
>>>> this error didn't show up...
>>>
>>> I have been lowering the software trip point temperature in the
>>> exynos7 dts file (to 55C) for testing purposes. Hence, when the
>>> temperature is read incorrectly as ~105C the board trips at boot-up
> 
> 					^^^^ this is a very unusual
> 					value - I had problems with
> 					reading 0xFF values with
> 					similar symptom (but this was
> 					caused by lack of vtmu).
> 
>>> itself. Maybe for exynos4/5 the incorrect value read during boot-up
>>> is in the non-tripping range and once the tmu is initialized later
>>> it continues to function properly thereafter ?
>>>
>>>>
>>>> The reordering is needed to be able to call set_mode callback at
>>>> of-thermal.c to set the mode.
>>>>
>>>> If this change causes problems, then another solution (probably
>>>> not so neat) must be found.
>>>
>>> Please let me know if you need any further details.
> 
> Abhilash, could you provide more details (like relevant output from
> dmesg) and point me a list of patches which shall I apply to test this
> issue on Exynos4/5?
> 
>>
>> What is the status of this patch? Is it still required?
> 
> It is strange, since on Exynos4/5 this works and some problems show up
> when run on Exynos7.
> 

I'm also wondering the status of this patch.

I get below errors when boots odroidxu3 board without this patch.

[    4.831980] thermal thermal_zone0: failed to read out thermal zone (-22)
[    4.838096] thermal thermal_zone1: failed to read out thermal zone (-22)
[    4.844894] thermal thermal_zone2: failed to read out thermal zone (-22)
[    4.851470] thermal thermal_zone3: failed to read out thermal zone (-22)
[    4.858186] thermal thermal_zone4: failed to read out thermal zone (-22)

Thanks.