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

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

Hi Joonyoung,

> 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

Could you check if after booting you can read temperature from thermal
zones?

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



-- 
Best regards,

Lukasz Majewski

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