| Message ID | 1421667844-13627-2-git-send-email-l.majewski@samsung.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Eduardo Valentin |
| Headers | show |
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 > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 > >
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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
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 -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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. -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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.
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. > > > -- To unsubscribe from this list: send the line "unsubscribe linux-pm" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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. > > > > > > >
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;
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(-)