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

Lukasz Majewski
2015-04-15T11:06:05Z

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?

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.

>
> [ 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

Please check [1] if it solves your problems.

> Thanks.

--
Best regards,

Lukasz Majewski

Samsung R&D Institute Poland (SRPOL) | Linux Platform Group