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

[Lukasz Majewski <l.majewski@samsung.com>]

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