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

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

Hi Lukasz,

On 04/15/2015 08:05 PM, Lukasz Majewski wrote:
> Hi Joonyoung,
> 
>> 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.
> 
> This patch has been dropped.
> 
>>
>> I get below errors when boots odroidxu3 board without this patch.
> 
> Could you share the exact SHA1 and branch which you use in your setup?
> 

I tested with of odroidxu3 patchset for pwm-fan control of Anand Moon on
20150414 -next.

http://www.spinics.net/lists/arm-kernel/msg412031.html

> For a reference please check following branch at github (this is the
> code which should be merged to v4.1-rc1)
> 
> git@github.com:lmajewski/linux-samsung-thermal.git
> 
> branch: next [1]
> 
> This branch includes exynos7 support done by Chanwoo.
> 

I got following errors from branch [1] without patchset of Anand Moon,

[    4.576442] thermal thermal_zone0: failed to read out thermal zone 0
[    4.581685] 10060000.tmu supply vtmu not found, using dummy regulator
[    4.588223] thermal thermal_zone1: failed to read out thermal zone 1
[    4.594110] 10064000.tmu supply vtmu not found, using dummy regulator
[    4.600849] thermal thermal_zone2: failed to read out thermal zone 2
[    4.606847] 10068000.tmu supply vtmu not found, using dummy regulator
[    4.613640] thermal thermal_zone3: failed to read out thermal zone 3
[    4.619584] 1006c000.tmu supply vtmu not found, using dummy regulator
[    4.626370] thermal thermal_zone4: failed to read out thermal zone 4
[    4.632324] 100a0000.tmu supply vtmu not found, using dummy regulator

>>
>> [    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)
>>
> 
> This issue has been fixed by following patch:
> "thermal: exynos: fix: Check if data->tmu_read callback is present
> before read"
> 
> SHA1: 4531fa1684bb883ee01f1a182900b1e15d461b3
> 

I already have this commit on my test branch.

> Please check [1] if it solves your problems.
> 
>> Thanks.
> 

Thanks.