All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] thermal: Set default thermal_zoneX mode to "enabled"
@ 2015-01-19 11:44 Lukasz Majewski
  2015-01-19 11:44 ` [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function Lukasz Majewski
  2015-01-19 11:44 ` [PATCH 2/2] thermal: of: Enable thermal_zoneX when sensor is correctly added Lukasz Majewski
  0 siblings, 2 replies; 19+ messages in thread
From: Lukasz Majewski @ 2015-01-19 11:44 UTC (permalink / raw)
  To: Eduardo Valentin, Zhang Rui, Kukjin Kim, Kukjin Kim
  Cc: Linux PM list, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
	Lukasz Majewski, Amit Daniel Kachhap, Abhilash Kesavan,
	Abhilash Kesavan, Kyungmin Park, Chanwoo Choi, Lukasz Majewski

After Exynos TMU rework the default value of 
/sys/class/thermal/thermal_zone0/mode was set to "disabled"

Those two patches enable it by default.

The problem was reported by: Abhilash Kesavan <a.kesavan@samsung.com>

Dependencies:
linux-soc-thermal/next
SHA1: 1813d80874699145f04af6b05ebab0a6419001fb
And applied following patch series:
[PATCH v5 00/18] thermal: exynos: Thermal code rework to use device tree

Lukasz Majewski (2):
  thermal: exynos: Reorder exynos_map_dt_data() function
  thermal: of: Enable thermal_zoneX when sensor is correctly added

 drivers/thermal/of-thermal.c         | 3 +++
 drivers/thermal/samsung/exynos_tmu.c | 7 ++++---
 2 files changed, 7 insertions(+), 3 deletions(-)

-- 
2.0.0.rc2


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-01-19 11:44 [PATCH 0/2] thermal: Set default thermal_zoneX mode to "enabled" Lukasz Majewski
@ 2015-01-19 11:44 ` Lukasz Majewski
  2015-01-22  8:29   ` Abhilash Kesavan
  2015-01-19 11:44 ` [PATCH 2/2] thermal: of: Enable thermal_zoneX when sensor is correctly added Lukasz Majewski
  1 sibling, 1 reply; 19+ messages in thread
From: Lukasz Majewski @ 2015-01-19 11:44 UTC (permalink / raw)
  To: Eduardo Valentin, Zhang Rui, Kukjin Kim, Kukjin Kim
  Cc: Linux PM list, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
	Lukasz Majewski, Amit Daniel Kachhap, Abhilash Kesavan,
	Abhilash Kesavan, Kyungmin Park, Chanwoo Choi, Lukasz Majewski

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;
+
 	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;
 
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/2] thermal: of: Enable thermal_zoneX when sensor is correctly added
  2015-01-19 11:44 [PATCH 0/2] thermal: Set default thermal_zoneX mode to "enabled" Lukasz Majewski
  2015-01-19 11:44 ` [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function Lukasz Majewski
@ 2015-01-19 11:44 ` Lukasz Majewski
  2015-01-20 18:26   ` Javi Merino
  1 sibling, 1 reply; 19+ messages in thread
From: Lukasz Majewski @ 2015-01-19 11:44 UTC (permalink / raw)
  To: Eduardo Valentin, Zhang Rui, Kukjin Kim, Kukjin Kim
  Cc: Linux PM list, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
	Lukasz Majewski, Amit Daniel Kachhap, Abhilash Kesavan,
	Abhilash Kesavan, Kyungmin Park, Chanwoo Choi, Lukasz Majewski

Up till now the thermal_zone mode was by default "disabled". With this
patch the default behavior was changed to "enable".

One can read the mode at:
/sys/class/thermal/thermal_zone0/mode

Reported-by: Abhilash Kesavan <a.kesavan@samsung.com>
Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>
---
 drivers/thermal/of-thermal.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
index d717f3d..668fb1b 100644
--- a/drivers/thermal/of-thermal.c
+++ b/drivers/thermal/of-thermal.c
@@ -497,6 +497,9 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
 		if (sensor_specs.np == sensor_np && id == sensor_id) {
 			tzd = thermal_zone_of_add_sensor(child, sensor_np,
 							 data, ops);
+			if (!IS_ERR(tzd))
+				tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
+
 			of_node_put(sensor_specs.np);
 			of_node_put(child);
 			goto exit;
-- 
2.0.0.rc2

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 2/2] thermal: of: Enable thermal_zoneX when sensor is correctly added
  2015-01-19 11:44 ` [PATCH 2/2] thermal: of: Enable thermal_zoneX when sensor is correctly added Lukasz Majewski
@ 2015-01-20 18:26   ` Javi Merino
  0 siblings, 0 replies; 19+ messages in thread
From: Javi Merino @ 2015-01-20 18:26 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Eduardo Valentin, Zhang Rui, Kukjin Kim, Kukjin Kim,
	Linux PM list, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
	Lukasz Majewski, Amit Daniel Kachhap, Abhilash Kesavan,
	Abhilash Kesavan, Kyungmin Park, Chanwoo Choi

On Mon, Jan 19, 2015 at 11:44:04AM +0000, Lukasz Majewski wrote:
> Up till now the thermal_zone mode was by default "disabled". With this
> patch the default behavior was changed to "enable".
> 
> One can read the mode at:
> /sys/class/thermal/thermal_zone0/mode
> 
> Reported-by: Abhilash Kesavan <a.kesavan@samsung.com>
> Signed-off-by: Lukasz Majewski <l.majewski@samsung.com>

We are facing the same issue with Juno.  When registering thermal
zones from device tree they were coming up disabled.  I've tested it
on my Juno board and it fixes it.

Tested-by: Javi Merino <javi.merino@arm.com>

> ---
>  drivers/thermal/of-thermal.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/thermal/of-thermal.c b/drivers/thermal/of-thermal.c
> index d717f3d..668fb1b 100644
> --- a/drivers/thermal/of-thermal.c
> +++ b/drivers/thermal/of-thermal.c
> @@ -497,6 +497,9 @@ thermal_zone_of_sensor_register(struct device *dev, int sensor_id, void *data,
>  		if (sensor_specs.np == sensor_np && id == sensor_id) {
>  			tzd = thermal_zone_of_add_sensor(child, sensor_np,
>  							 data, ops);
> +			if (!IS_ERR(tzd))
> +				tzd->ops->set_mode(tzd, THERMAL_DEVICE_ENABLED);
> +
>  			of_node_put(sensor_specs.np);
>  			of_node_put(child);
>  			goto exit;

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-01-19 11:44 ` [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function Lukasz Majewski
@ 2015-01-22  8:29   ` Abhilash Kesavan
  2015-01-22  9:01     ` Lukasz Majewski
  0 siblings, 1 reply; 19+ messages in thread
From: Abhilash Kesavan @ 2015-01-22  8:29 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Eduardo Valentin, Zhang Rui, Kukjin Kim, Kukjin Kim,
	Linux PM list, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
	Lukasz Majewski, Amit Daniel Kachhap, Kyungmin Park,
	Chanwoo Choi

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;
> +
>         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. 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 ?

Regards,
Abhilash
>
> --
> 2.0.0.rc2
>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-01-22  8:29   ` Abhilash Kesavan
@ 2015-01-22  9:01     ` Lukasz Majewski
  2015-01-22 12:32       ` Abhilash Kesavan
  0 siblings, 1 reply; 19+ messages in thread
From: Lukasz Majewski @ 2015-01-22  9:01 UTC (permalink / raw)
  To: Abhilash Kesavan
  Cc: Eduardo Valentin, Zhang Rui, Kukjin Kim, Kukjin Kim,
	Linux PM list, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
	Lukasz Majewski, Amit Daniel Kachhap, Kyungmin Park,
	Chanwoo Choi

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;
> > +
> >         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)?

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

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.

> 
> Regards,
> Abhilash
> >
> > --
> > 2.0.0.rc2
> >



-- 
Best regards,

Lukasz Majewski

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-01-22  9:01     ` Lukasz Majewski
@ 2015-01-22 12:32       ` Abhilash Kesavan
  2015-01-29 23:06         ` Eduardo Valentin
  0 siblings, 1 reply; 19+ messages in thread
From: Abhilash Kesavan @ 2015-01-22 12:32 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Eduardo Valentin, Zhang Rui, Kukjin Kim, Kukjin Kim,
	Linux PM list, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
	Lukasz Majewski, Amit Daniel Kachhap, Kyungmin Park,
	Chanwoo Choi

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;
>> > +
>> >         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
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.

Thanks,
Abhilash

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-01-22 12:32       ` Abhilash Kesavan
@ 2015-01-29 23:06         ` Eduardo Valentin
  2015-01-30  8:14           ` Lukasz Majewski
  0 siblings, 1 reply; 19+ messages in thread
From: Eduardo Valentin @ 2015-01-29 23:06 UTC (permalink / raw)
  To: Abhilash Kesavan
  Cc: Lukasz Majewski, Zhang Rui, Kukjin Kim, Kukjin Kim,
	Linux PM list, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
	Lukasz Majewski, Amit Daniel Kachhap, Kyungmin Park,
	Chanwoo Choi

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;
> >> > +
> >> >         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
> 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.

What is the status of this patch? Is it still required?

Cheers,

> 
> Thanks,
> Abhilash

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-01-29 23:06         ` Eduardo Valentin
@ 2015-01-30  8:14           ` Lukasz Majewski
  2015-01-30 15:06             ` Abhilash Kesavan
  2015-04-15  6:45             ` Joonyoung Shim
  0 siblings, 2 replies; 19+ messages in thread
From: Lukasz Majewski @ 2015-01-30  8:14 UTC (permalink / raw)
  To: Eduardo Valentin, Abhilash Kesavan
  Cc: Zhang Rui, Kukjin Kim, Kukjin Kim, Linux PM list,
	linux-samsung-soc, Bartlomiej Zolnierkiewicz, Lukasz Majewski,
	Amit Daniel Kachhap, Kyungmin Park, Chanwoo Choi

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;
> > >> > +
> > >> >         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.

> 
> Cheers,
> 
> > 
> > Thanks,
> > Abhilash



-- 
Best regards,

Lukasz Majewski

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-01-30  8:14           ` Lukasz Majewski
@ 2015-01-30 15:06             ` Abhilash Kesavan
  2015-02-02  5:36               ` Abhilash Kesavan
  2015-04-15  6:45             ` Joonyoung Shim
  1 sibling, 1 reply; 19+ messages in thread
From: Abhilash Kesavan @ 2015-01-30 15:06 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Eduardo Valentin, Zhang Rui, Kukjin Kim, Kukjin Kim,
	Linux PM list, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
	Lukasz Majewski, Amit Daniel Kachhap, Kyungmin Park,
	Chanwoo Choi

Hi Lukasz,

On Fri, Jan 30, 2015 at 1:44 PM, Lukasz Majewski <l.majewski@samsung.com> 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;
>> > >> > +
>> > >> >         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?
Sorry, I have not had the time to re-check this and provide you with
the current temperature register value. I will definitely do so on
Monday and also provide you the dmesg output. I applied the following
patches on linux-next:

bbf872d thermal: exynos: Add TMU support for Exynos7 SoC
b8190ac dts: Documentation: Add documentation for Exynos7 SoC thermal bindings
9330ec1 thermal: exynos: Reorder exynos_map_dt_data() function
4dd41c4 thermal: exynos: dts: Provide device tree bindings identical
to the one in exynos_tmu_data.c
a7b80b9 thermal: dts: exynos: Trip points and sensor configuration
data for Exynos5440
77d072e thermal: exynos: dts: Define default thermal-zones for Exynos4
964dd36 thermal: dts: Default trip points definition for Exynos5420 SoCs
c1d2f97 thermal: exynos: dts: Add default definition of the TMU sensor parameter
02a4496 arm: dts: Adding CPU cooling binding for Exynos SoCs
bfadff0 arm: dts: odroid: Enable TMU at Exynos4412 based Odroid U3 device
862764c arm: dts: odroid: Add LDO10 regulator node necessary for TMU on Odroid
c064731 arm: dts: trats: Enable TMU on the Exynos4210 trats device

Along with the above list I have a patch which adds the dt changes
required for exynos7 on similar lines as done for exynos4/exynos5. In
the exyno7 trip point dts file I have modified the cpu-crit-0
temperature to a low value of 55C for testing purposes.

>
>>
>> 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 would have expected the issue to show up on Exynos4/5 too. I will
test this on the 5420 based board I have on Monday.

Regards,
Abhilash

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-01-30 15:06             ` Abhilash Kesavan
@ 2015-02-02  5:36               ` Abhilash Kesavan
  2015-02-04 10:36                 ` Lukasz Majewski
  0 siblings, 1 reply; 19+ messages in thread
From: Abhilash Kesavan @ 2015-02-02  5:36 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Eduardo Valentin, Zhang Rui, Kukjin Kim, Kukjin Kim,
	Linux PM list, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
	Lukasz Majewski, Amit Daniel Kachhap, Kyungmin Park,
	Chanwoo Choi

Hi Lukasz,

On Fri, Jan 30, 2015 at 8:36 PM, Abhilash Kesavan
<kesavan.abhilash@gmail.com> wrote:
> Hi Lukasz,
>
> On Fri, Jan 30, 2015 at 1:44 PM, Lukasz Majewski <l.majewski@samsung.com> 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;
>>> > >> > +
>>> > >> >         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?
> Sorry, I have not had the time to re-check this and provide you with
> the current temperature register value. I will definitely do so on
> Monday and also provide you the dmesg output. I applied the following
> patches on linux-next:
>
> bbf872d thermal: exynos: Add TMU support for Exynos7 SoC
> b8190ac dts: Documentation: Add documentation for Exynos7 SoC thermal bindings
> 9330ec1 thermal: exynos: Reorder exynos_map_dt_data() function
> 4dd41c4 thermal: exynos: dts: Provide device tree bindings identical
> to the one in exynos_tmu_data.c
> a7b80b9 thermal: dts: exynos: Trip points and sensor configuration
> data for Exynos5440
> 77d072e thermal: exynos: dts: Define default thermal-zones for Exynos4
> 964dd36 thermal: dts: Default trip points definition for Exynos5420 SoCs
> c1d2f97 thermal: exynos: dts: Add default definition of the TMU sensor parameter
> 02a4496 arm: dts: Adding CPU cooling binding for Exynos SoCs
> bfadff0 arm: dts: odroid: Enable TMU at Exynos4412 based Odroid U3 device
> 862764c arm: dts: odroid: Add LDO10 regulator node necessary for TMU on Odroid
> c064731 arm: dts: trats: Enable TMU on the Exynos4210 trats device
>
> Along with the above list I have a patch which adds the dt changes
> required for exynos7 on similar lines as done for exynos4/exynos5. In
> the exyno7 trip point dts file I have modified the cpu-crit-0
> temperature to a low value of 55C for testing purposes.
>
>>
>>>
>>> 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 would have expected the issue to show up on Exynos4/5 too. I will
> test this on the 5420 based board I have on Monday.

I tested this on a 5420 based chromebook that I have (Peach Pit) and
observed similar results as that on Exynos7. Following are the patches
applied on next-20150130:

5b1194d thermal: exynos: Reorder exynos_map_dt_data() function
30c6165 thermal: exynos: dts: Provide device tree bindings identical
to the one in exynos_tmu_data.c
d94c248 thermal: dts: exynos: Trip points and sensor configuration
data for Exynos5440
aac8b3a thermal: exynos: dts: Define default thermal-zones for Exynos4
5e8cf52 thermal: dts: Default trip points definition for Exynos5420 SoCs
17ec9c2 thermal: exynos: dts: Add default definition of the TMU sensor parameter
36e247b arm: dts: Adding CPU cooling binding for Exynos SoCs
7aa1bb1 arm: dts: odroid: Enable TMU at Exynos4412 based Odroid U3 device
8884a76 arm: dts: odroid: Add LDO10 regulator node necessary for TMU on Odroid
aae2e51 arm: dts: trats: Enable TMU on the Exynos4210 trats device
827e3bd Add linux-next specific files for 20150130

On top of these patches, I have the following diff for
debugging/testing pruposes:

diff --git a/arch/arm/boot/dts/exynos5420-trip-points.dtsi
b/arch/arm/boot/dts/exynos5420-trip-points.dtsi
index 09d6c56..ac8b6a0 100644
--- a/arch/arm/boot/dts/exynos5420-trip-points.dtsi
+++ b/arch/arm/boot/dts/exynos5420-trip-points.dtsi
@@ -13,22 +13,22 @@ polling-delay-passive = <0>;
 polling-delay = <0>;
 trips {
        cpu-alert-0 {
-               temperature = <85000>; /* millicelsius */
+               temperature = <55000>; /* millicelsius */
                hysteresis = <10000>; /* millicelsius */
                type = "active";
        };
        cpu-alert-1 {
-               temperature = <103000>; /* millicelsius */
+               temperature = <63000>; /* millicelsius */
                hysteresis = <10000>; /* millicelsius */
                type = "active";
        };
        cpu-alert-2 {
-               temperature = <110000>; /* millicelsius */
+               temperature = <70000>; /* millicelsius */
                hysteresis = <10000>; /* millicelsius */
                type = "active";
        };
        cpu-crit-0 {
-               temperature = <1200000>; /* millicelsius */
+               temperature = <75000>; /* millicelsius */
                hysteresis = <0>; /* millicelsius */
                type = "critical";
        };
diff --git a/drivers/thermal/samsung/exynos_tmu.c
b/drivers/thermal/samsung/exynos_tmu.c
index 852e622..7281581 100644
--- a/drivers/thermal/samsung/exynos_tmu.c
+++ b/drivers/thermal/samsung/exynos_tmu.c
@@ -237,6 +237,7 @@ static int code_to_temp(struct exynos_tmu_data
*data, u8 temp_code)
                break;
        case TYPE_ONE_POINT_TRIMMING:
                temp = temp_code - data->temp_error1 + pdata->first_point_trim;
+               printk("temp_code is %d, err1 is %d, trim is %d, temp
is %d\n", temp_code, data->temp_error1, pdata->first_point_trim,
temp);
                break;
        default:
                temp = temp_code - pdata->default_temp_offset;
@@ -585,6 +586,7 @@ static int exynos_get_temp(void *p, long *temp)

        *temp = code_to_temp(data, data->tmu_read(data)) * MCELSIUS;

+       printk("temperature is %ld\n", *temp);
        clk_disable(data->clk);
        mutex_unlock(&data->lock);

@@ -675,6 +677,7 @@ static int exynos4210_tmu_read(struct exynos_tmu_data *data)

 static int exynos4412_tmu_read(struct exynos_tmu_data *data)
 {
+       printk("\n%s: Reading the current temperature register value:
0x%x\n\n", __func__, readb(data->base + EXYNOS_TMU_REG_CURRENT_TEMP));
        return readb(data->base + EXYNOS_TMU_REG_CURRENT_TEMP);
 }

diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 48491d1..981fc99 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -469,7 +469,7 @@ static void update_temperature(struct
thermal_zone_device *tz)
        mutex_unlock(&tz->lock);

        trace_thermal_temperature(tz);
-       dev_dbg(&tz->device, "last_temperature=%d, current_temperature=%d\n",
+       dev_err(&tz->device, "last_temperature=%d, current_temperature=%d\n",
                                tz->last_temperature, tz->temperature);
 }

Also, I noticed  a typo (an extra zero) in cpu-crit-0 temperature above.

Following is the relevant boot-up log:

[    3.160126] TPS65090_RAILSLDO2: supplied by vbat-supply
[    3.343421] thermal thermal_zone5: last_temperature=0,
current_temperature=25900
[    3.349365] sbs-battery 20-000b: sbs-battery: battery gas gauge
device registered
[    3.374280] 10060000.tmu supply vtmu not found, using dummy regulator
[    3.379408]
[    3.379408] exynos4412_tmu_read: Reading the current temperature
register value: 0x36
[    3.379408]
[    3.390026] temp_code is 54, err1 is 0, trim is 25, temp is 79
[    3.395827] temperature is 79000
[    3.399053] thermal thermal_zone0: last_temperature=0,
current_temperature=79000
[    3.406429] thermal thermal_zone0: critical temperature reached(79
C),shutting down
[    3.414241] reboot: Failed to start orderly shutdown: forcing the issue
[    3.420690] usb 5-1: new high-speed USB device number 2 using exynos-ehci
[    3.427819] 10064000.tmu supply vtmu not found, using dummy regulator
[    3.434011] Emergency Sync complete
[    3.434355]
[    3.434355] exynos4412_tmu_read: Reading the current temperature
register value: 0x3
[    3.434355]
[    3.434367] temp_code is 3, err1 is 0, trim is 25, temp is 28
[    3.434373] temperature is 28000
[    3.434393] thermal thermal_zone1: last_temperature=0,
current_temperature=28000
[    3.434744] 10068000.tmu supply vtmu not found, using dummy regulator
[    3.435069]
[    3.435069] exynos4412_tmu_read: Reading the current temperature
register value: 0x2
[    3.435069]
[    3.435080] temp_code is 2, err1 is 0, trim is 25, temp is 27
[    3.435086] temperature is 27000
[    3.435103] thermal thermal_zone2: last_temperature=0,
current_temperature=27000
[    3.435427] 1006c000.tmu supply vtmu not found, using dummy regulator
[    3.435762]
[    3.435762] exynos4412_tmu_read: Reading the current temperature
register value: 0x2
[    3.435762]
[    3.435772] temp_code is 2, err1 is 0, trim is 25, temp is 27
[    3.435778] temperature is 27000
[    3.435794] thermal thermal_zone3: last_temperature=0,
current_temperature=27000
[    3.436112] 100a0000.tmu supply vtmu not found, using dummy regulator
[    3.436464]
[    3.436464] exynos4412_tmu_read: Reading the current temperature
register value: 0x33
[    3.436464]
[    3.436475] temp_code is 51, err1 is 0, trim is 25, temp is 76
[    3.436480] temperature is 76000
[    3.436496] thermal thermal_zone4: last_temperature=0,
current_temperature=76000
[    3.436527] thermal thermal_zone4: critical temperature reached(76
C),shutting down

>From the log, thermal_zone0 and thermal_zone4 show incorrect
temperatures. It looks like the first point trim error1 would need to
be intialized before we do a read ?
Please let me know if you need any more details.

Regards,
Abhilash

^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-02-02  5:36               ` Abhilash Kesavan
@ 2015-02-04 10:36                 ` Lukasz Majewski
  2015-02-04 11:50                   ` Abhilash Kesavan
  0 siblings, 1 reply; 19+ messages in thread
From: Lukasz Majewski @ 2015-02-04 10:36 UTC (permalink / raw)
  To: Abhilash Kesavan
  Cc: Eduardo Valentin, Zhang Rui, Kukjin Kim, Kukjin Kim,
	Linux PM list, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
	Lukasz Majewski, Amit Daniel Kachhap, Kyungmin Park,
	Chanwoo Choi

Hi Abhilash,

> Hi Lukasz,
> 
> On Fri, Jan 30, 2015 at 8:36 PM, Abhilash Kesavan
> <kesavan.abhilash@gmail.com> wrote:
> > Hi Lukasz,
> >
> > On Fri, Jan 30, 2015 at 1:44 PM, Lukasz Majewski
> > <l.majewski@samsung.com> 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;
> >>> > >> > +
> >>> > >> >         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?
> > Sorry, I have not had the time to re-check this and provide you with
> > the current temperature register value. I will definitely do so on
> > Monday and also provide you the dmesg output. I applied the
> > following patches on linux-next:
> >
> > bbf872d thermal: exynos: Add TMU support for Exynos7 SoC
> > b8190ac dts: Documentation: Add documentation for Exynos7 SoC
> > thermal bindings 9330ec1 thermal: exynos: Reorder
> > exynos_map_dt_data() function 4dd41c4 thermal: exynos: dts: Provide
> > device tree bindings identical to the one in exynos_tmu_data.c
> > a7b80b9 thermal: dts: exynos: Trip points and sensor configuration
> > data for Exynos5440
> > 77d072e thermal: exynos: dts: Define default thermal-zones for
> > Exynos4 964dd36 thermal: dts: Default trip points definition for
> > Exynos5420 SoCs c1d2f97 thermal: exynos: dts: Add default
> > definition of the TMU sensor parameter 02a4496 arm: dts: Adding CPU
> > cooling binding for Exynos SoCs bfadff0 arm: dts: odroid: Enable
> > TMU at Exynos4412 based Odroid U3 device 862764c arm: dts: odroid:
> > Add LDO10 regulator node necessary for TMU on Odroid c064731 arm:
> > dts: trats: Enable TMU on the Exynos4210 trats device
> >
> > Along with the above list I have a patch which adds the dt changes
> > required for exynos7 on similar lines as done for exynos4/exynos5.
> > In the exyno7 trip point dts file I have modified the cpu-crit-0
> > temperature to a low value of 55C for testing purposes.
> >
> >>
> >>>
> >>> 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 would have expected the issue to show up on Exynos4/5 too. I will
> > test this on the 5420 based board I have on Monday.
> 
> I tested this on a 5420 based chromebook that I have (Peach Pit) and
> observed similar results as that on Exynos7. Following are the patches
> applied on next-20150130:
> 
> 5b1194d thermal: exynos: Reorder exynos_map_dt_data() function
> 30c6165 thermal: exynos: dts: Provide device tree bindings identical
> to the one in exynos_tmu_data.c
> d94c248 thermal: dts: exynos: Trip points and sensor configuration
> data for Exynos5440
> aac8b3a thermal: exynos: dts: Define default thermal-zones for Exynos4
> 5e8cf52 thermal: dts: Default trip points definition for Exynos5420
> SoCs 17ec9c2 thermal: exynos: dts: Add default definition of the TMU
> sensor parameter 36e247b arm: dts: Adding CPU cooling binding for
> Exynos SoCs 7aa1bb1 arm: dts: odroid: Enable TMU at Exynos4412 based
> Odroid U3 device 8884a76 arm: dts: odroid: Add LDO10 regulator node
> necessary for TMU on Odroid aae2e51 arm: dts: trats: Enable TMU on
> the Exynos4210 trats device 827e3bd Add linux-next specific files for
> 20150130
> 
> On top of these patches, I have the following diff for
> debugging/testing pruposes:
> 
> diff --git a/arch/arm/boot/dts/exynos5420-trip-points.dtsi
> b/arch/arm/boot/dts/exynos5420-trip-points.dtsi
> index 09d6c56..ac8b6a0 100644
> --- a/arch/arm/boot/dts/exynos5420-trip-points.dtsi
> +++ b/arch/arm/boot/dts/exynos5420-trip-points.dtsi
> @@ -13,22 +13,22 @@ polling-delay-passive = <0>;
>  polling-delay = <0>;
>  trips {
>         cpu-alert-0 {
> -               temperature = <85000>; /* millicelsius */
> +               temperature = <55000>; /* millicelsius */
>                 hysteresis = <10000>; /* millicelsius */
>                 type = "active";
>         };
>         cpu-alert-1 {
> -               temperature = <103000>; /* millicelsius */
> +               temperature = <63000>; /* millicelsius */
>                 hysteresis = <10000>; /* millicelsius */
>                 type = "active";
>         };
>         cpu-alert-2 {
> -               temperature = <110000>; /* millicelsius */
> +               temperature = <70000>; /* millicelsius */
>                 hysteresis = <10000>; /* millicelsius */
>                 type = "active";
>         };
>         cpu-crit-0 {
> -               temperature = <1200000>; /* millicelsius */
> +               temperature = <75000>; /* millicelsius */
>                 hysteresis = <0>; /* millicelsius */
>                 type = "critical";
>         };
> diff --git a/drivers/thermal/samsung/exynos_tmu.c
> b/drivers/thermal/samsung/exynos_tmu.c
> index 852e622..7281581 100644
> --- a/drivers/thermal/samsung/exynos_tmu.c
> +++ b/drivers/thermal/samsung/exynos_tmu.c
> @@ -237,6 +237,7 @@ static int code_to_temp(struct exynos_tmu_data
> *data, u8 temp_code)
>                 break;
>         case TYPE_ONE_POINT_TRIMMING:
>                 temp = temp_code - data->temp_error1 +
> pdata->first_point_trim;
> +               printk("temp_code is %d, err1 is %d, trim is %d, temp
> is %d\n", temp_code, data->temp_error1, pdata->first_point_trim,
> temp);
>                 break;
>         default:
>                 temp = temp_code - pdata->default_temp_offset;
> @@ -585,6 +586,7 @@ static int exynos_get_temp(void *p, long *temp)
> 
>         *temp = code_to_temp(data, data->tmu_read(data)) * MCELSIUS;
> 
> +       printk("temperature is %ld\n", *temp);
>         clk_disable(data->clk);
>         mutex_unlock(&data->lock);
> 
> @@ -675,6 +677,7 @@ static int exynos4210_tmu_read(struct
> exynos_tmu_data *data)
> 
>  static int exynos4412_tmu_read(struct exynos_tmu_data *data)
>  {
> +       printk("\n%s: Reading the current temperature register value:
> 0x%x\n\n", __func__, readb(data->base + EXYNOS_TMU_REG_CURRENT_TEMP));
>         return readb(data->base + EXYNOS_TMU_REG_CURRENT_TEMP);
>  }
> 
> diff --git a/drivers/thermal/thermal_core.c
> b/drivers/thermal/thermal_core.c index 48491d1..981fc99 100644
> --- a/drivers/thermal/thermal_core.c
> +++ b/drivers/thermal/thermal_core.c
> @@ -469,7 +469,7 @@ static void update_temperature(struct
> thermal_zone_device *tz)
>         mutex_unlock(&tz->lock);
> 
>         trace_thermal_temperature(tz);
> -       dev_dbg(&tz->device, "last_temperature=%d,
> current_temperature=%d\n",
> +       dev_err(&tz->device, "last_temperature=%d,
> current_temperature=%d\n", tz->last_temperature, tz->temperature);
>  }
> 
> Also, I noticed  a typo (an extra zero) in cpu-crit-0 temperature
> above.
> 
> Following is the relevant boot-up log:
> 
> [    3.160126] TPS65090_RAILSLDO2: supplied by vbat-supply
> [    3.343421] thermal thermal_zone5: last_temperature=0,
> current_temperature=25900
> [    3.349365] sbs-battery 20-000b: sbs-battery: battery gas gauge
> device registered
> [    3.374280] 10060000.tmu supply vtmu not found, using dummy
> regulator [    3.379408]
> [    3.379408] exynos4412_tmu_read: Reading the current temperature
> register value: 0x36
> [    3.379408]
> [    3.390026] temp_code is 54, err1 is 0, trim is 25, temp is 79
> [    3.395827] temperature is 79000
> [    3.399053] thermal thermal_zone0: last_temperature=0,
> current_temperature=79000
> [    3.406429] thermal thermal_zone0: critical temperature reached(79
> C),shutting down
> [    3.414241] reboot: Failed to start orderly shutdown: forcing the
> issue [    3.420690] usb 5-1: new high-speed USB device number 2
> using exynos-ehci [    3.427819] 10064000.tmu supply vtmu not found,
> using dummy regulator [    3.434011] Emergency Sync complete
> [    3.434355]
> [    3.434355] exynos4412_tmu_read: Reading the current temperature
> register value: 0x3
> [    3.434355]
> [    3.434367] temp_code is 3, err1 is 0, trim is 25, temp is 28
> [    3.434373] temperature is 28000
> [    3.434393] thermal thermal_zone1: last_temperature=0,
> current_temperature=28000
> [    3.434744] 10068000.tmu supply vtmu not found, using dummy
> regulator [    3.435069]
> [    3.435069] exynos4412_tmu_read: Reading the current temperature
> register value: 0x2
> [    3.435069]
> [    3.435080] temp_code is 2, err1 is 0, trim is 25, temp is 27
> [    3.435086] temperature is 27000
> [    3.435103] thermal thermal_zone2: last_temperature=0,
> current_temperature=27000
> [    3.435427] 1006c000.tmu supply vtmu not found, using dummy
> regulator [    3.435762]
> [    3.435762] exynos4412_tmu_read: Reading the current temperature
> register value: 0x2
> [    3.435762]
> [    3.435772] temp_code is 2, err1 is 0, trim is 25, temp is 27
> [    3.435778] temperature is 27000
> [    3.435794] thermal thermal_zone3: last_temperature=0,
> current_temperature=27000
> [    3.436112] 100a0000.tmu supply vtmu not found, using dummy
> regulator [    3.436464]
> [    3.436464] exynos4412_tmu_read: Reading the current temperature
> register value: 0x33
> [    3.436464]
> [    3.436475] temp_code is 51, err1 is 0, trim is 25, temp is 76
> [    3.436480] temperature is 76000
> [    3.436496] thermal thermal_zone4: last_temperature=0,
> current_temperature=76000
> [    3.436527] thermal thermal_zone4: critical temperature reached(76
> C),shutting down
> 
> From the log, thermal_zone0 and thermal_zone4 show incorrect
> temperatures. It looks like the first point trim error1 would need to
> be intialized before we do a read ?

Could you check if this error happens when you have default temp
threshold values?

It is strange, that only this exceeded temp happens for thermal zone 0
and 4. Other three show correct temp.

Off topic - where are placed those two misbehaving zones? Isn't thermal
zone 4 the one for GPU? 


> Please let me know if you need any more details.
> 
> Regards,
> Abhilash



-- 
Best regards,

Lukasz Majewski

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-02-04 10:36                 ` Lukasz Majewski
@ 2015-02-04 11:50                   ` Abhilash Kesavan
  0 siblings, 0 replies; 19+ messages in thread
From: Abhilash Kesavan @ 2015-02-04 11:50 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Eduardo Valentin, Zhang Rui, Kukjin Kim, Kukjin Kim,
	Linux PM list, linux-samsung-soc, Bartlomiej Zolnierkiewicz,
	Lukasz Majewski, Amit Daniel Kachhap, Kyungmin Park,
	Chanwoo Choi

Hi Lukasz,

On Wed, Feb 4, 2015 at 4:06 PM, Lukasz Majewski <l.majewski@samsung.com> wrote:
> Hi Abhilash,
>
>> Hi Lukasz,
>>
>> On Fri, Jan 30, 2015 at 8:36 PM, Abhilash Kesavan
>> <kesavan.abhilash@gmail.com> wrote:
>> > Hi Lukasz,
>> >
>> > On Fri, Jan 30, 2015 at 1:44 PM, Lukasz Majewski
>> > <l.majewski@samsung.com> 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;
>> >>> > >> > +
>> >>> > >> >         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?
>> > Sorry, I have not had the time to re-check this and provide you with
>> > the current temperature register value. I will definitely do so on
>> > Monday and also provide you the dmesg output. I applied the
>> > following patches on linux-next:
>> >
>> > bbf872d thermal: exynos: Add TMU support for Exynos7 SoC
>> > b8190ac dts: Documentation: Add documentation for Exynos7 SoC
>> > thermal bindings 9330ec1 thermal: exynos: Reorder
>> > exynos_map_dt_data() function 4dd41c4 thermal: exynos: dts: Provide
>> > device tree bindings identical to the one in exynos_tmu_data.c
>> > a7b80b9 thermal: dts: exynos: Trip points and sensor configuration
>> > data for Exynos5440
>> > 77d072e thermal: exynos: dts: Define default thermal-zones for
>> > Exynos4 964dd36 thermal: dts: Default trip points definition for
>> > Exynos5420 SoCs c1d2f97 thermal: exynos: dts: Add default
>> > definition of the TMU sensor parameter 02a4496 arm: dts: Adding CPU
>> > cooling binding for Exynos SoCs bfadff0 arm: dts: odroid: Enable
>> > TMU at Exynos4412 based Odroid U3 device 862764c arm: dts: odroid:
>> > Add LDO10 regulator node necessary for TMU on Odroid c064731 arm:
>> > dts: trats: Enable TMU on the Exynos4210 trats device
>> >
>> > Along with the above list I have a patch which adds the dt changes
>> > required for exynos7 on similar lines as done for exynos4/exynos5.
>> > In the exyno7 trip point dts file I have modified the cpu-crit-0
>> > temperature to a low value of 55C for testing purposes.
>> >
>> >>
>> >>>
>> >>> 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 would have expected the issue to show up on Exynos4/5 too. I will
>> > test this on the 5420 based board I have on Monday.
>>
>> I tested this on a 5420 based chromebook that I have (Peach Pit) and
>> observed similar results as that on Exynos7. Following are the patches
>> applied on next-20150130:
>>
>> 5b1194d thermal: exynos: Reorder exynos_map_dt_data() function
>> 30c6165 thermal: exynos: dts: Provide device tree bindings identical
>> to the one in exynos_tmu_data.c
>> d94c248 thermal: dts: exynos: Trip points and sensor configuration
>> data for Exynos5440
>> aac8b3a thermal: exynos: dts: Define default thermal-zones for Exynos4
>> 5e8cf52 thermal: dts: Default trip points definition for Exynos5420
>> SoCs 17ec9c2 thermal: exynos: dts: Add default definition of the TMU
>> sensor parameter 36e247b arm: dts: Adding CPU cooling binding for
>> Exynos SoCs 7aa1bb1 arm: dts: odroid: Enable TMU at Exynos4412 based
>> Odroid U3 device 8884a76 arm: dts: odroid: Add LDO10 regulator node
>> necessary for TMU on Odroid aae2e51 arm: dts: trats: Enable TMU on
>> the Exynos4210 trats device 827e3bd Add linux-next specific files for
>> 20150130
>>
>> On top of these patches, I have the following diff for
>> debugging/testing pruposes:
>>
>> diff --git a/arch/arm/boot/dts/exynos5420-trip-points.dtsi
>> b/arch/arm/boot/dts/exynos5420-trip-points.dtsi
>> index 09d6c56..ac8b6a0 100644
>> --- a/arch/arm/boot/dts/exynos5420-trip-points.dtsi
>> +++ b/arch/arm/boot/dts/exynos5420-trip-points.dtsi
>> @@ -13,22 +13,22 @@ polling-delay-passive = <0>;
>>  polling-delay = <0>;
>>  trips {
>>         cpu-alert-0 {
>> -               temperature = <85000>; /* millicelsius */
>> +               temperature = <55000>; /* millicelsius */
>>                 hysteresis = <10000>; /* millicelsius */
>>                 type = "active";
>>         };
>>         cpu-alert-1 {
>> -               temperature = <103000>; /* millicelsius */
>> +               temperature = <63000>; /* millicelsius */
>>                 hysteresis = <10000>; /* millicelsius */
>>                 type = "active";
>>         };
>>         cpu-alert-2 {
>> -               temperature = <110000>; /* millicelsius */
>> +               temperature = <70000>; /* millicelsius */
>>                 hysteresis = <10000>; /* millicelsius */
>>                 type = "active";
>>         };
>>         cpu-crit-0 {
>> -               temperature = <1200000>; /* millicelsius */
>> +               temperature = <75000>; /* millicelsius */
>>                 hysteresis = <0>; /* millicelsius */
>>                 type = "critical";
>>         };
>> diff --git a/drivers/thermal/samsung/exynos_tmu.c
>> b/drivers/thermal/samsung/exynos_tmu.c
>> index 852e622..7281581 100644
>> --- a/drivers/thermal/samsung/exynos_tmu.c
>> +++ b/drivers/thermal/samsung/exynos_tmu.c
>> @@ -237,6 +237,7 @@ static int code_to_temp(struct exynos_tmu_data
>> *data, u8 temp_code)
>>                 break;
>>         case TYPE_ONE_POINT_TRIMMING:
>>                 temp = temp_code - data->temp_error1 +
>> pdata->first_point_trim;
>> +               printk("temp_code is %d, err1 is %d, trim is %d, temp
>> is %d\n", temp_code, data->temp_error1, pdata->first_point_trim,
>> temp);
>>                 break;
>>         default:
>>                 temp = temp_code - pdata->default_temp_offset;
>> @@ -585,6 +586,7 @@ static int exynos_get_temp(void *p, long *temp)
>>
>>         *temp = code_to_temp(data, data->tmu_read(data)) * MCELSIUS;
>>
>> +       printk("temperature is %ld\n", *temp);
>>         clk_disable(data->clk);
>>         mutex_unlock(&data->lock);
>>
>> @@ -675,6 +677,7 @@ static int exynos4210_tmu_read(struct
>> exynos_tmu_data *data)
>>
>>  static int exynos4412_tmu_read(struct exynos_tmu_data *data)
>>  {
>> +       printk("\n%s: Reading the current temperature register value:
>> 0x%x\n\n", __func__, readb(data->base + EXYNOS_TMU_REG_CURRENT_TEMP));
>>         return readb(data->base + EXYNOS_TMU_REG_CURRENT_TEMP);
>>  }
>>
>> diff --git a/drivers/thermal/thermal_core.c
>> b/drivers/thermal/thermal_core.c index 48491d1..981fc99 100644
>> --- a/drivers/thermal/thermal_core.c
>> +++ b/drivers/thermal/thermal_core.c
>> @@ -469,7 +469,7 @@ static void update_temperature(struct
>> thermal_zone_device *tz)
>>         mutex_unlock(&tz->lock);
>>
>>         trace_thermal_temperature(tz);
>> -       dev_dbg(&tz->device, "last_temperature=%d,
>> current_temperature=%d\n",
>> +       dev_err(&tz->device, "last_temperature=%d,
>> current_temperature=%d\n", tz->last_temperature, tz->temperature);
>>  }
>>
>> Also, I noticed  a typo (an extra zero) in cpu-crit-0 temperature
>> above.
>>
>> Following is the relevant boot-up log:
>>
>> [    3.160126] TPS65090_RAILSLDO2: supplied by vbat-supply
>> [    3.343421] thermal thermal_zone5: last_temperature=0,
>> current_temperature=25900
>> [    3.349365] sbs-battery 20-000b: sbs-battery: battery gas gauge
>> device registered
>> [    3.374280] 10060000.tmu supply vtmu not found, using dummy
>> regulator [    3.379408]
>> [    3.379408] exynos4412_tmu_read: Reading the current temperature
>> register value: 0x36
>> [    3.379408]
>> [    3.390026] temp_code is 54, err1 is 0, trim is 25, temp is 79
>> [    3.395827] temperature is 79000
>> [    3.399053] thermal thermal_zone0: last_temperature=0,
>> current_temperature=79000
>> [    3.406429] thermal thermal_zone0: critical temperature reached(79
>> C),shutting down
>> [    3.414241] reboot: Failed to start orderly shutdown: forcing the
>> issue [    3.420690] usb 5-1: new high-speed USB device number 2
>> using exynos-ehci [    3.427819] 10064000.tmu supply vtmu not found,
>> using dummy regulator [    3.434011] Emergency Sync complete
>> [    3.434355]
>> [    3.434355] exynos4412_tmu_read: Reading the current temperature
>> register value: 0x3
>> [    3.434355]
>> [    3.434367] temp_code is 3, err1 is 0, trim is 25, temp is 28
>> [    3.434373] temperature is 28000
>> [    3.434393] thermal thermal_zone1: last_temperature=0,
>> current_temperature=28000
>> [    3.434744] 10068000.tmu supply vtmu not found, using dummy
>> regulator [    3.435069]
>> [    3.435069] exynos4412_tmu_read: Reading the current temperature
>> register value: 0x2
>> [    3.435069]
>> [    3.435080] temp_code is 2, err1 is 0, trim is 25, temp is 27
>> [    3.435086] temperature is 27000
>> [    3.435103] thermal thermal_zone2: last_temperature=0,
>> current_temperature=27000
>> [    3.435427] 1006c000.tmu supply vtmu not found, using dummy
>> regulator [    3.435762]
>> [    3.435762] exynos4412_tmu_read: Reading the current temperature
>> register value: 0x2
>> [    3.435762]
>> [    3.435772] temp_code is 2, err1 is 0, trim is 25, temp is 27
>> [    3.435778] temperature is 27000
>> [    3.435794] thermal thermal_zone3: last_temperature=0,
>> current_temperature=27000
>> [    3.436112] 100a0000.tmu supply vtmu not found, using dummy
>> regulator [    3.436464]
>> [    3.436464] exynos4412_tmu_read: Reading the current temperature
>> register value: 0x33
>> [    3.436464]
>> [    3.436475] temp_code is 51, err1 is 0, trim is 25, temp is 76
>> [    3.436480] temperature is 76000
>> [    3.436496] thermal thermal_zone4: last_temperature=0,
>> current_temperature=76000
>> [    3.436527] thermal thermal_zone4: critical temperature reached(76
>> C),shutting down
>>
>> From the log, thermal_zone0 and thermal_zone4 show incorrect
>> temperatures. It looks like the first point trim error1 would need to
>> be intialized before we do a read ?
>
> Could you check if this error happens when you have default temp
> threshold values?

No, it does not. The default thresholds on 5420 have the alerts
starting at 85C. The incorrect temperature read is in the range of
74-78C for 5420 and so with the default thresholds no trip occurs.
Once the tmu is properly initialized in the probe function, the error
will anyway not occur.

>
> It is strange, that only this exceeded temp happens for thermal zone 0
> and 4. Other three show correct temp.
>
> Off topic - where are placed those two misbehaving zones? Isn't thermal
> zone 4 the one for GPU?

Yes, zone4 is for the GPU.

On the arndale octa, if you reduce the thresholds do you see the same
behavior I observe ? or  can you just add the debug prints that I have
been using and see what temperature is being reported initially on the
arndale octa board you have.

Thanks,
Abhilash

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-01-30  8:14           ` Lukasz Majewski
  2015-01-30 15:06             ` Abhilash Kesavan
@ 2015-04-15  6:45             ` Joonyoung Shim
  2015-04-15 11:05               ` Lukasz Majewski
  1 sibling, 1 reply; 19+ messages in thread
From: Joonyoung Shim @ 2015-04-15  6:45 UTC (permalink / raw)
  To: Lukasz Majewski, Eduardo Valentin, Abhilash Kesavan
  Cc: Zhang Rui, Kukjin Kim, Kukjin Kim, Linux PM list,
	linux-samsung-soc, Bartlomiej Zolnierkiewicz, Lukasz Majewski,
	Amit Daniel Kachhap, Kyungmin Park, Chanwoo Choi

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-04-15  6:45             ` Joonyoung Shim
@ 2015-04-15 11:05               ` Lukasz Majewski
  2015-04-16  1:01                 ` Joonyoung Shim
  0 siblings, 1 reply; 19+ messages in thread
From: Lukasz Majewski @ 2015-04-15 11:05 UTC (permalink / raw)
  To: Joonyoung Shim
  Cc: Eduardo Valentin, Abhilash Kesavan, Zhang Rui, Kukjin Kim,
	Kukjin Kim, Linux PM list, linux-samsung-soc,
	Bartlomiej Zolnierkiewicz, Lukasz Majewski, Amit Daniel Kachhap,
	Kyungmin Park, Chanwoo Choi

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-04-15 11:05               ` Lukasz Majewski
@ 2015-04-16  1:01                 ` Joonyoung Shim
  2015-04-17 12:39                   ` Lukasz Majewski
  0 siblings, 1 reply; 19+ messages in thread
From: Joonyoung Shim @ 2015-04-16  1:01 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Eduardo Valentin, Abhilash Kesavan, Zhang Rui, Kukjin Kim,
	Kukjin Kim, Linux PM list, linux-samsung-soc,
	Bartlomiej Zolnierkiewicz, Lukasz Majewski, Amit Daniel Kachhap,
	Kyungmin Park, Chanwoo Choi

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.

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-04-16  1:01                 ` Joonyoung Shim
@ 2015-04-17 12:39                   ` Lukasz Majewski
  2015-04-22  1:34                     ` Joonyoung Shim
  0 siblings, 1 reply; 19+ messages in thread
From: Lukasz Majewski @ 2015-04-17 12:39 UTC (permalink / raw)
  To: Joonyoung Shim
  Cc: Eduardo Valentin, Abhilash Kesavan, Zhang Rui, Kukjin Kim,
	Kukjin Kim, Linux PM list, linux-samsung-soc,
	Bartlomiej Zolnierkiewicz, Lukasz Majewski, Amit Daniel Kachhap,
	Kyungmin Park, Chanwoo Choi

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-04-17 12:39                   ` Lukasz Majewski
@ 2015-04-22  1:34                     ` Joonyoung Shim
  2015-04-22  8:49                       ` Lukasz Majewski
  0 siblings, 1 reply; 19+ messages in thread
From: Joonyoung Shim @ 2015-04-22  1:34 UTC (permalink / raw)
  To: Lukasz Majewski
  Cc: Eduardo Valentin, Abhilash Kesavan, Zhang Rui, Kukjin Kim,
	Kukjin Kim, Linux PM list, linux-samsung-soc,
	Bartlomiej Zolnierkiewicz, Lukasz Majewski, Amit Daniel Kachhap,
	Kyungmin Park, Chanwoo Choi

Hi Lukasz,

On 04/17/2015 09:39 PM, Lukasz Majewski wrote:
> 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?
> 

Yes, i can read temperature from
/sys/devices/virtual/thermal/thermal_zone[X]/temp after booting.

Thanks.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function
  2015-04-22  1:34                     ` Joonyoung Shim
@ 2015-04-22  8:49                       ` Lukasz Majewski
  0 siblings, 0 replies; 19+ messages in thread
From: Lukasz Majewski @ 2015-04-22  8:49 UTC (permalink / raw)
  To: Joonyoung Shim
  Cc: Eduardo Valentin, Abhilash Kesavan, Zhang Rui, Kukjin Kim,
	Kukjin Kim, Linux PM list, linux-samsung-soc,
	Bartlomiej Zolnierkiewicz, Lukasz Majewski, Amit Daniel Kachhap,
	Kyungmin Park, Chanwoo Choi

Hi Joonyoung,

> Hi Lukasz,
> 
> On 04/17/2015 09:39 PM, Lukasz Majewski wrote:
> > 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?
> > 
> 
> Yes, i can read temperature from
> /sys/devices/virtual/thermal/thermal_zone[X]/temp after booting.

I'm aware of "thermal thermal_zone4: failed to read out thermal
zone" messages which show up during booting.

It is to prevent crashing TMU when it is not yet properly configured.
The problem is related with dependency of of-thermal and exynos
specific code.

I'm planning to fix this issue.

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

^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2015-04-22  8:49 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-01-19 11:44 [PATCH 0/2] thermal: Set default thermal_zoneX mode to "enabled" Lukasz Majewski
2015-01-19 11:44 ` [PATCH 1/2] thermal: exynos: Reorder exynos_map_dt_data() function Lukasz Majewski
2015-01-22  8:29   ` Abhilash Kesavan
2015-01-22  9:01     ` Lukasz Majewski
2015-01-22 12:32       ` Abhilash Kesavan
2015-01-29 23:06         ` Eduardo Valentin
2015-01-30  8:14           ` Lukasz Majewski
2015-01-30 15:06             ` Abhilash Kesavan
2015-02-02  5:36               ` Abhilash Kesavan
2015-02-04 10:36                 ` Lukasz Majewski
2015-02-04 11:50                   ` Abhilash Kesavan
2015-04-15  6:45             ` Joonyoung Shim
2015-04-15 11:05               ` Lukasz Majewski
2015-04-16  1:01                 ` Joonyoung Shim
2015-04-17 12:39                   ` Lukasz Majewski
2015-04-22  1:34                     ` Joonyoung Shim
2015-04-22  8:49                       ` Lukasz Majewski
2015-01-19 11:44 ` [PATCH 2/2] thermal: of: Enable thermal_zoneX when sensor is correctly added Lukasz Majewski
2015-01-20 18:26   ` Javi Merino

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.