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

Joonyoung Shim
2015-04-15T06:45:49Z

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.