| Message ID | 20220710212423.681301-18-daniel.lezcano@linexp.org (mailing list archive) |
|---|---|
| State | Awaiting Upstream |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | None | expand |
Hi Daniel, Thanks for your work. On 2022-07-10 23:24:07 +0200, Daniel Lezcano wrote: > The thermal OF code has a new API allowing to migrate the OF > initialization to a simpler approach. > > Use this new API. I tested this together with the series it depends on and while temperature monitoring seems to work fine it breaks the emul_temp interface (/sys/class/thermal/thermal_zone2/emul_temp). Before this change I can write a temperature to this file and have it trigger actions, in my test-case changing the cooling state, which I observe in /sys/class/thermal/cooling_device0/cur_state. Likewise before this change I could trip the critical trip-point that would power off the board using the emul_temp interface, this too no longer works, echo 120000 > /sys/class/thermal/thermal_zone2/emul_temp Is this an intention change of the new API? > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org> > --- > drivers/thermal/rcar_gen3_thermal.c | 16 ++++++++-------- > drivers/thermal/rcar_thermal.c | 13 +++---------- > 2 files changed, 11 insertions(+), 18 deletions(-) > > diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c > index 43eb25b167bc..29946114a8f9 100644 > --- a/drivers/thermal/rcar_gen3_thermal.c > +++ b/drivers/thermal/rcar_gen3_thermal.c > @@ -164,9 +164,9 @@ static int rcar_gen3_thermal_round(int temp) > return result * RCAR3_THERMAL_GRAN; > } > > -static int rcar_gen3_thermal_get_temp(void *devdata, int *temp) > +static int rcar_gen3_thermal_get_temp(struct thermal_zone_device *tz, int *temp) > { > - struct rcar_gen3_thermal_tsc *tsc = devdata; > + struct rcar_gen3_thermal_tsc *tsc = tz->devdata; > int mcelsius, val; > int reg; > > @@ -203,9 +203,9 @@ static int rcar_gen3_thermal_mcelsius_to_temp(struct rcar_gen3_thermal_tsc *tsc, > return INT_FIXPT(val); > } > > -static int rcar_gen3_thermal_set_trips(void *devdata, int low, int high) > +static int rcar_gen3_thermal_set_trips(struct thermal_zone_device *tz, int low, int high) > { > - struct rcar_gen3_thermal_tsc *tsc = devdata; > + struct rcar_gen3_thermal_tsc *tsc = tz->devdata; > u32 irqmsk = 0; > > if (low != -INT_MAX) { > @@ -225,7 +225,7 @@ static int rcar_gen3_thermal_set_trips(void *devdata, int low, int high) > return 0; > } > > -static struct thermal_zone_of_device_ops rcar_gen3_tz_of_ops = { > +static struct thermal_zone_device_ops rcar_gen3_tz_of_ops = { > .get_temp = rcar_gen3_thermal_get_temp, > .set_trips = rcar_gen3_thermal_set_trips, > }; > @@ -504,8 +504,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev) > for (i = 0; i < priv->num_tscs; i++) { > struct rcar_gen3_thermal_tsc *tsc = priv->tscs[i]; > > - zone = devm_thermal_zone_of_sensor_register(dev, i, tsc, > - &rcar_gen3_tz_of_ops); > + zone = devm_thermal_of_zone_register(dev, i, tsc, > + &rcar_gen3_tz_of_ops); > if (IS_ERR(zone)) { > dev_err(dev, "Can't register thermal zone\n"); > ret = PTR_ERR(zone); > @@ -556,7 +556,7 @@ static int __maybe_unused rcar_gen3_thermal_resume(struct device *dev) > > priv->thermal_init(tsc); > if (zone->ops->set_trips) > - rcar_gen3_thermal_set_trips(tsc, zone->prev_low_trip, > + rcar_gen3_thermal_set_trips(zone, zone->prev_low_trip, > zone->prev_high_trip); > } > > diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c > index 1d729ed4d685..4df42d70d867 100644 > --- a/drivers/thermal/rcar_thermal.c > +++ b/drivers/thermal/rcar_thermal.c > @@ -271,13 +271,6 @@ static int rcar_thermal_get_current_temp(struct rcar_thermal_priv *priv, > return 0; > } > > -static int rcar_thermal_of_get_temp(void *data, int *temp) > -{ > - struct rcar_thermal_priv *priv = data; > - > - return rcar_thermal_get_current_temp(priv, temp); > -} > - > static int rcar_thermal_get_temp(struct thermal_zone_device *zone, int *temp) > { > struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone); > @@ -323,8 +316,8 @@ static int rcar_thermal_get_trip_temp(struct thermal_zone_device *zone, > return 0; > } > > -static const struct thermal_zone_of_device_ops rcar_thermal_zone_of_ops = { > - .get_temp = rcar_thermal_of_get_temp, > +static struct thermal_zone_device_ops rcar_thermal_zone_of_ops = { > + .get_temp = rcar_thermal_get_temp, > }; > > static struct thermal_zone_device_ops rcar_thermal_zone_ops = { > @@ -534,7 +527,7 @@ static int rcar_thermal_probe(struct platform_device *pdev) > goto error_unregister; > > if (chip->use_of_thermal) { > - priv->zone = devm_thermal_zone_of_sensor_register( > + priv->zone = devm_thermal_of_zone_register( > dev, i, priv, > &rcar_thermal_zone_of_ops); > } else { > -- > 2.25.1 >
Hi Niklas, On 19/07/2022 11:10, Niklas Söderlund wrote: > Hi Daniel, > > Thanks for your work. > > On 2022-07-10 23:24:07 +0200, Daniel Lezcano wrote: >> The thermal OF code has a new API allowing to migrate the OF >> initialization to a simpler approach. >> >> Use this new API. > > I tested this together with the series it depends on and while > temperature monitoring seems to work fine it breaks the emul_temp > interface (/sys/class/thermal/thermal_zone2/emul_temp). > > Before this change I can write a temperature to this file and have it > trigger actions, in my test-case changing the cooling state, which I > observe in /sys/class/thermal/cooling_device0/cur_state. > > Likewise before this change I could trip the critical trip-point that > would power off the board using the emul_temp interface, this too no > longer works, > > echo 120000 > /sys/class/thermal/thermal_zone2/emul_temp > > Is this an intention change of the new API? Absolutely not :) Thanks for taking the time to test and report back the issue. I'll investigate that. -- D.
Hi Niklas, I tried to reproduce the issue but without success. What sensor are you using ? On 19/07/2022 11:10, Niklas Söderlund wrote: > Hi Daniel, > > Thanks for your work. > > On 2022-07-10 23:24:07 +0200, Daniel Lezcano wrote: >> The thermal OF code has a new API allowing to migrate the OF >> initialization to a simpler approach. >> >> Use this new API. > I tested this together with the series it depends on and while > temperature monitoring seems to work fine it breaks the emul_temp > interface (/sys/class/thermal/thermal_zone2/emul_temp). > > Before this change I can write a temperature to this file and have it > trigger actions, in my test-case changing the cooling state, which I > observe in /sys/class/thermal/cooling_device0/cur_state. > > Likewise before this change I could trip the critical trip-point that > would power off the board using the emul_temp interface, this too no > longer works, > > echo 120000 > /sys/class/thermal/thermal_zone2/emul_temp > > Is this an intention change of the new API?
Hi Daniel, On 2022-07-24 20:27:54 +0200, Daniel Lezcano wrote: > > Hi Niklas, > > I tried to reproduce the issue but without success. > > What sensor are you using ? I was using rcar_gen3_thermal. I did my tests starting on v5.19-rc7 and then picked '[PATCH v5 00/12] thermal OF rework' from [1] and finally applied this full series on-top of that. If you have a branch or some specific test you wish me to try I'm happy to so. 1. https://lore.kernel.org/lkml/20220710123512.1714714-1-daniel.lezcano@linexp.org/ > > > On 19/07/2022 11:10, Niklas Söderlund wrote: > > Hi Daniel, > > > > Thanks for your work. > > > > On 2022-07-10 23:24:07 +0200, Daniel Lezcano wrote: > > > The thermal OF code has a new API allowing to migrate the OF > > > initialization to a simpler approach. > > > > > > Use this new API. > > I tested this together with the series it depends on and while > > temperature monitoring seems to work fine it breaks the emul_temp > > interface (/sys/class/thermal/thermal_zone2/emul_temp). > > > > Before this change I can write a temperature to this file and have it > > trigger actions, in my test-case changing the cooling state, which I > > observe in /sys/class/thermal/cooling_device0/cur_state. > > > > Likewise before this change I could trip the critical trip-point that > > would power off the board using the emul_temp interface, this too no > > longer works, > > > > echo 120000 > /sys/class/thermal/thermal_zone2/emul_temp > > > > Is this an intention change of the new API? > > > >
Hi Niklas, I give another try but failed to reproduce the issue. Perhaps my board has a path different from yours. Thanks for proposing to test the series. I've uploaded the branch here: https://github.com/dlezcano/linux-thermal On 24/07/2022 21:00, Niklas Söderlund wrote: > Hi Daniel, > > On 2022-07-24 20:27:54 +0200, Daniel Lezcano wrote: >> Hi Niklas, >> >> I tried to reproduce the issue but without success. >> >> What sensor are you using ? > I was using rcar_gen3_thermal. > > I did my tests starting on v5.19-rc7 and then picked '[PATCH v5 00/12] > thermal OF rework' from [1] and finally applied this full series on-top > of that. If you have a branch or some specific test you wish me to try > I'm happy to so. > > 1. https://lore.kernel.org/lkml/20220710123512.1714714-1-daniel.lezcano@linexp.org/ > >> >> On 19/07/2022 11:10, Niklas Söderlund wrote: >>> Hi Daniel, >>> >>> Thanks for your work. >>> >>> On 2022-07-10 23:24:07 +0200, Daniel Lezcano wrote: >>>> The thermal OF code has a new API allowing to migrate the OF >>>> initialization to a simpler approach. >>>> >>>> Use this new API. >>> I tested this together with the series it depends on and while >>> temperature monitoring seems to work fine it breaks the emul_temp >>> interface (/sys/class/thermal/thermal_zone2/emul_temp). >>> >>> Before this change I can write a temperature to this file and have it >>> trigger actions, in my test-case changing the cooling state, which I >>> observe in /sys/class/thermal/cooling_device0/cur_state. >>> >>> Likewise before this change I could trip the critical trip-point that >>> would power off the board using the emul_temp interface, this too no >>> longer works, >>> >>> echo 120000 > /sys/class/thermal/thermal_zone2/emul_temp >>> >>> Is this an intention change of the new API? >> >> >>
Hi Daniel,
I tested your branch, unfortunately with the same result for
rcar_gen3_thermal. Manipulation of emul_temp file do not trigger
actions.
If I on-top of your branch revert:
409ca214f4c6bd5b ("thermal/of: Remove old OF code")
7b43f76d3428227e ("thermal/drivers/rcar: Switch to new of API")
I'm able to 'restore' the behavior where I can change the cooling state
and trigger the critical trip point using emul_temp to shutdown the
board.
As the change in question also effects the rcar_thermal sensor I gave
that a try too. It have no cooling on this system I have so my only
test-case is to write a temperature above the critical trip point to
emul_temp as see if that shutdown the system. And just as with
rcar_gen3_thermal with your series nothing happens while with the two
commits outline above reverted the system shuts down.
echo 110000 > /sys/devices/virtual/thermal/thermal_zone0/emul_temp
If it's any help writing to emul_temp have some effect as the emulated
temperature is read back from the temp sysfs while. For rcar_thermal
where the critical trip point is 95 degrees,
* With this series
# grep . /sys/devices/virtual/thermal/thermal_zone0/trip_point_0_*
/sys/devices/virtual/thermal/thermal_zone0/trip_point_0_hyst:0
/sys/devices/virtual/thermal/thermal_zone0/trip_point_0_temp:95000
/sys/devices/virtual/thermal/thermal_zone0/trip_point_0_type:critical
# cat /sys/devices/virtual/thermal/thermal_zone0/temp
35000
# echo 50000 > /sys/devices/virtual/thermal/thermal_zone0/emul_temp
# cat /sys/devices/virtual/thermal/thermal_zone0/temp
50000
# echo 110000 > /sys/devices/virtual/thermal/thermal_zone0/emul_temp
# cat /sys/devices/virtual/thermal/thermal_zone0/temp
110000
*** system alive ***
* With this series and the two patches reverted or plain v5.19-rc4
# grep . /sys/devices/virtual/thermal/thermal_zone0/trip_point_0_*
/sys/devices/virtual/thermal/thermal_zone0/trip_point_0_hyst:0
/sys/devices/virtual/thermal/thermal_zone0/trip_point_0_temp:95000
/sys/devices/virtual/thermal/thermal_zone0/trip_point_0_type:critical
# cat /sys/devices/virtual/thermal/thermal_zone0/temp
35000
# echo 50000 > /sys/devices/virtual/thermal/thermal_zone0/emul_temp
# cat /sys/devices/virtual/thermal/thermal_zone0/temp
50000
# echo 110000 > /sys/devices/virtual/thermal/thermal_zone0/emul_temp
[ 121.380054] thermal thermal_zone0: cpu-thermal: critical temperature reached, shutting down
[ 121.388482] reboot: HARDWARE PROTECTION shutdown (Temperature too high)
*** system shuts down ***
And to make it more problematic I don't think the lack of action is
limited to the emul_temp interface. With rcar_thermal I lowered the
critical trip point value to 45C and used the cpuburn application to
generate load and raise the temperature.
The result mirrors the findings above, with your branch the system do
not trigger the critical trip point. If I revert the two commits or run
plain v5.19-rc4, once the temperature reaches 45C the critical trip
point kicks in and shuts down the system.
I hope this helps, I'm sorry I can't find the real issue diging in the
core changes. I'm happy to help trying to find the root cause for this
and I think the idea behind the new API is good.
On 2022-07-24 23:11:47 +0200, Daniel Lezcano wrote:
>
> Hi Niklas,
>
> I give another try but failed to reproduce the issue. Perhaps my board has a
> path different from yours.
>
> Thanks for proposing to test the series. I've uploaded the branch here:
>
> https://github.com/dlezcano/linux-thermal
>
>
> On 24/07/2022 21:00, Niklas Söderlund wrote:
> > Hi Daniel,
> >
> > On 2022-07-24 20:27:54 +0200, Daniel Lezcano wrote:
> > > Hi Niklas,
> > >
> > > I tried to reproduce the issue but without success.
> > >
> > > What sensor are you using ?
> > I was using rcar_gen3_thermal.
> >
> > I did my tests starting on v5.19-rc7 and then picked '[PATCH v5 00/12]
> > thermal OF rework' from [1] and finally applied this full series on-top
> > of that. If you have a branch or some specific test you wish me to try
> > I'm happy to so.
> >
> > 1. https://lore.kernel.org/lkml/20220710123512.1714714-1-daniel.lezcano@linexp.org/
> >
> > >
> > > On 19/07/2022 11:10, Niklas Söderlund wrote:
> > > > Hi Daniel,
> > > >
> > > > Thanks for your work.
> > > >
> > > > On 2022-07-10 23:24:07 +0200, Daniel Lezcano wrote:
> > > > > The thermal OF code has a new API allowing to migrate the OF
> > > > > initialization to a simpler approach.
> > > > >
> > > > > Use this new API.
> > > > I tested this together with the series it depends on and while
> > > > temperature monitoring seems to work fine it breaks the emul_temp
> > > > interface (/sys/class/thermal/thermal_zone2/emul_temp).
> > > >
> > > > Before this change I can write a temperature to this file and have it
> > > > trigger actions, in my test-case changing the cooling state, which I
> > > > observe in /sys/class/thermal/cooling_device0/cur_state.
> > > >
> > > > Likewise before this change I could trip the critical trip-point that
> > > > would power off the board using the emul_temp interface, this too no
> > > > longer works,
> > > >
> > > > echo 120000 > /sys/class/thermal/thermal_zone2/emul_temp
> > > >
> > > > Is this an intention change of the new API?
> > >
> > >
> > >
>
Hi (again) Daniel, I figured it out, the thermal zone is disabled after this change. For both rcar sensors with the new API thermal_zone_device_enable() is never called. In the old API the zone is enabled in the call chain of devm_thermal_zone_of_sensor_register(). While after this change the zone is not enabled by the core when calling thermal_zone_device_enable(). If I add a call to thermal_zone_device_enable() together with the new API everything works as before. But I'm not sure if the correct solution is to add a call to thermal_zone_device_enable() in the sensor drivers or in the call chain of the new API? On 2022-07-25 00:39:10 +0200, Niklas Söderlund wrote: > Hi Daniel, > > I tested your branch, unfortunately with the same result for > rcar_gen3_thermal. Manipulation of emul_temp file do not trigger > actions. > > If I on-top of your branch revert: > > 409ca214f4c6bd5b ("thermal/of: Remove old OF code") > 7b43f76d3428227e ("thermal/drivers/rcar: Switch to new of API") > > I'm able to 'restore' the behavior where I can change the cooling state > and trigger the critical trip point using emul_temp to shutdown the > board. > > As the change in question also effects the rcar_thermal sensor I gave > that a try too. It have no cooling on this system I have so my only > test-case is to write a temperature above the critical trip point to > emul_temp as see if that shutdown the system. And just as with > rcar_gen3_thermal with your series nothing happens while with the two > commits outline above reverted the system shuts down. > > echo 110000 > /sys/devices/virtual/thermal/thermal_zone0/emul_temp > > If it's any help writing to emul_temp have some effect as the emulated > temperature is read back from the temp sysfs while. For rcar_thermal > where the critical trip point is 95 degrees, > > * With this series > # grep . /sys/devices/virtual/thermal/thermal_zone0/trip_point_0_* > /sys/devices/virtual/thermal/thermal_zone0/trip_point_0_hyst:0 > /sys/devices/virtual/thermal/thermal_zone0/trip_point_0_temp:95000 > /sys/devices/virtual/thermal/thermal_zone0/trip_point_0_type:critical > # cat /sys/devices/virtual/thermal/thermal_zone0/temp > 35000 > # echo 50000 > /sys/devices/virtual/thermal/thermal_zone0/emul_temp > # cat /sys/devices/virtual/thermal/thermal_zone0/temp > 50000 > # echo 110000 > /sys/devices/virtual/thermal/thermal_zone0/emul_temp > # cat /sys/devices/virtual/thermal/thermal_zone0/temp > 110000 > *** system alive *** > > * With this series and the two patches reverted or plain v5.19-rc4 > # grep . /sys/devices/virtual/thermal/thermal_zone0/trip_point_0_* > /sys/devices/virtual/thermal/thermal_zone0/trip_point_0_hyst:0 > /sys/devices/virtual/thermal/thermal_zone0/trip_point_0_temp:95000 > /sys/devices/virtual/thermal/thermal_zone0/trip_point_0_type:critical > # cat /sys/devices/virtual/thermal/thermal_zone0/temp > 35000 > # echo 50000 > /sys/devices/virtual/thermal/thermal_zone0/emul_temp > # cat /sys/devices/virtual/thermal/thermal_zone0/temp > 50000 > # echo 110000 > /sys/devices/virtual/thermal/thermal_zone0/emul_temp > [ 121.380054] thermal thermal_zone0: cpu-thermal: critical temperature reached, shutting down > [ 121.388482] reboot: HARDWARE PROTECTION shutdown (Temperature too high) > *** system shuts down *** > > And to make it more problematic I don't think the lack of action is > limited to the emul_temp interface. With rcar_thermal I lowered the > critical trip point value to 45C and used the cpuburn application to > generate load and raise the temperature. > > The result mirrors the findings above, with your branch the system do > not trigger the critical trip point. If I revert the two commits or run > plain v5.19-rc4, once the temperature reaches 45C the critical trip > point kicks in and shuts down the system. > > I hope this helps, I'm sorry I can't find the real issue diging in the > core changes. I'm happy to help trying to find the root cause for this > and I think the idea behind the new API is good. > > On 2022-07-24 23:11:47 +0200, Daniel Lezcano wrote: > > > > Hi Niklas, > > > > I give another try but failed to reproduce the issue. Perhaps my board has a > > path different from yours. > > > > Thanks for proposing to test the series. I've uploaded the branch here: > > > > https://github.com/dlezcano/linux-thermal > > > > > > On 24/07/2022 21:00, Niklas Söderlund wrote: > > > Hi Daniel, > > > > > > On 2022-07-24 20:27:54 +0200, Daniel Lezcano wrote: > > > > Hi Niklas, > > > > > > > > I tried to reproduce the issue but without success. > > > > > > > > What sensor are you using ? > > > I was using rcar_gen3_thermal. > > > > > > I did my tests starting on v5.19-rc7 and then picked '[PATCH v5 00/12] > > > thermal OF rework' from [1] and finally applied this full series on-top > > > of that. If you have a branch or some specific test you wish me to try > > > I'm happy to so. > > > > > > 1. https://lore.kernel.org/lkml/20220710123512.1714714-1-daniel.lezcano@linexp.org/ > > > > > > > > > > > On 19/07/2022 11:10, Niklas Söderlund wrote: > > > > > Hi Daniel, > > > > > > > > > > Thanks for your work. > > > > > > > > > > On 2022-07-10 23:24:07 +0200, Daniel Lezcano wrote: > > > > > > The thermal OF code has a new API allowing to migrate the OF > > > > > > initialization to a simpler approach. > > > > > > > > > > > > Use this new API. > > > > > I tested this together with the series it depends on and while > > > > > temperature monitoring seems to work fine it breaks the emul_temp > > > > > interface (/sys/class/thermal/thermal_zone2/emul_temp). > > > > > > > > > > Before this change I can write a temperature to this file and have it > > > > > trigger actions, in my test-case changing the cooling state, which I > > > > > observe in /sys/class/thermal/cooling_device0/cur_state. > > > > > > > > > > Likewise before this change I could trip the critical trip-point that > > > > > would power off the board using the emul_temp interface, this too no > > > > > longer works, > > > > > > > > > > echo 120000 > /sys/class/thermal/thermal_zone2/emul_temp > > > > > > > > > > Is this an intention change of the new API? > > > > > > > > > > > > > > > > -- > Kind Regards, > Niklas Söderlund
Hi Niklas, On 25/07/2022 01:28, Niklas Söderlund wrote: > Hi (again) Daniel, > > I figured it out, the thermal zone is disabled after this change. For > both rcar sensors with the new API thermal_zone_device_enable() is never > called. > > In the old API the zone is enabled in the call chain of > devm_thermal_zone_of_sensor_register(). While after this change the zone > is not enabled by the core when calling thermal_zone_device_enable(). > > If I add a call to thermal_zone_device_enable() together with the new > API everything works as before. But I'm not sure if the correct solution > is to add a call to thermal_zone_device_enable() in the sensor drivers > or in the call chain of the new API? > > On 2022-07-25 00:39:10 +0200, Niklas Söderlund wrote: >> Hi Daniel, >> >> I tested your branch, unfortunately with the same result for >> rcar_gen3_thermal. Manipulation of emul_temp file do not trigger >> actions. Thanks for investigating, I updated the branch. Does it fix the issue ?
Hi Daniel, On 2022-07-25 12:00:30 +0200, Daniel Lezcano wrote: > > Hi Niklas, > > On 25/07/2022 01:28, Niklas Söderlund wrote: > > Hi (again) Daniel, > > > > I figured it out, the thermal zone is disabled after this change. For > > both rcar sensors with the new API thermal_zone_device_enable() is never > > called. > > > > In the old API the zone is enabled in the call chain of > > devm_thermal_zone_of_sensor_register(). While after this change the zone > > is not enabled by the core when calling thermal_zone_device_enable(). > > > > If I add a call to thermal_zone_device_enable() together with the new > > API everything works as before. But I'm not sure if the correct solution > > is to add a call to thermal_zone_device_enable() in the sensor drivers > > or in the call chain of the new API? > > > > On 2022-07-25 00:39:10 +0200, Niklas Söderlund wrote: > > > Hi Daniel, > > > > > > I tested your branch, unfortunately with the same result for > > > rcar_gen3_thermal. Manipulation of emul_temp file do not trigger > > > actions. > > Thanks for investigating, I updated the branch. Does it fix the issue ? I tested the branch with the head [1] and it restores the expected operation for both rcar_gen3_thermal and rcar_thermal sensors. Thanks for the fix, with this change I'm happy with this new API. 1. commit e9b792a531c10756 ("thermal/of: Remove old OF code")
On 25/07/2022 12:38, Niklas Söderlund wrote: > Hi Daniel, > > On 2022-07-25 12:00:30 +0200, Daniel Lezcano wrote: >> >> Hi Niklas, >> >> On 25/07/2022 01:28, Niklas Söderlund wrote: >>> Hi (again) Daniel, >>> >>> I figured it out, the thermal zone is disabled after this change. For >>> both rcar sensors with the new API thermal_zone_device_enable() is never >>> called. >>> >>> In the old API the zone is enabled in the call chain of >>> devm_thermal_zone_of_sensor_register(). While after this change the zone >>> is not enabled by the core when calling thermal_zone_device_enable(). >>> >>> If I add a call to thermal_zone_device_enable() together with the new >>> API everything works as before. But I'm not sure if the correct solution >>> is to add a call to thermal_zone_device_enable() in the sensor drivers >>> or in the call chain of the new API? >>> >>> On 2022-07-25 00:39:10 +0200, Niklas Söderlund wrote: >>>> Hi Daniel, >>>> >>>> I tested your branch, unfortunately with the same result for >>>> rcar_gen3_thermal. Manipulation of emul_temp file do not trigger >>>> actions. >> >> Thanks for investigating, I updated the branch. Does it fix the issue ? > > I tested the branch with the head [1] and it restores the expected > operation for both rcar_gen3_thermal and rcar_thermal sensors. > > Thanks for the fix, with this change I'm happy with this new API. > > 1. commit e9b792a531c10756 ("thermal/of: Remove old OF code") Thanks !!
diff --git a/drivers/thermal/rcar_gen3_thermal.c b/drivers/thermal/rcar_gen3_thermal.c index 43eb25b167bc..29946114a8f9 100644 --- a/drivers/thermal/rcar_gen3_thermal.c +++ b/drivers/thermal/rcar_gen3_thermal.c @@ -164,9 +164,9 @@ static int rcar_gen3_thermal_round(int temp) return result * RCAR3_THERMAL_GRAN; } -static int rcar_gen3_thermal_get_temp(void *devdata, int *temp) +static int rcar_gen3_thermal_get_temp(struct thermal_zone_device *tz, int *temp) { - struct rcar_gen3_thermal_tsc *tsc = devdata; + struct rcar_gen3_thermal_tsc *tsc = tz->devdata; int mcelsius, val; int reg; @@ -203,9 +203,9 @@ static int rcar_gen3_thermal_mcelsius_to_temp(struct rcar_gen3_thermal_tsc *tsc, return INT_FIXPT(val); } -static int rcar_gen3_thermal_set_trips(void *devdata, int low, int high) +static int rcar_gen3_thermal_set_trips(struct thermal_zone_device *tz, int low, int high) { - struct rcar_gen3_thermal_tsc *tsc = devdata; + struct rcar_gen3_thermal_tsc *tsc = tz->devdata; u32 irqmsk = 0; if (low != -INT_MAX) { @@ -225,7 +225,7 @@ static int rcar_gen3_thermal_set_trips(void *devdata, int low, int high) return 0; } -static struct thermal_zone_of_device_ops rcar_gen3_tz_of_ops = { +static struct thermal_zone_device_ops rcar_gen3_tz_of_ops = { .get_temp = rcar_gen3_thermal_get_temp, .set_trips = rcar_gen3_thermal_set_trips, }; @@ -504,8 +504,8 @@ static int rcar_gen3_thermal_probe(struct platform_device *pdev) for (i = 0; i < priv->num_tscs; i++) { struct rcar_gen3_thermal_tsc *tsc = priv->tscs[i]; - zone = devm_thermal_zone_of_sensor_register(dev, i, tsc, - &rcar_gen3_tz_of_ops); + zone = devm_thermal_of_zone_register(dev, i, tsc, + &rcar_gen3_tz_of_ops); if (IS_ERR(zone)) { dev_err(dev, "Can't register thermal zone\n"); ret = PTR_ERR(zone); @@ -556,7 +556,7 @@ static int __maybe_unused rcar_gen3_thermal_resume(struct device *dev) priv->thermal_init(tsc); if (zone->ops->set_trips) - rcar_gen3_thermal_set_trips(tsc, zone->prev_low_trip, + rcar_gen3_thermal_set_trips(zone, zone->prev_low_trip, zone->prev_high_trip); } diff --git a/drivers/thermal/rcar_thermal.c b/drivers/thermal/rcar_thermal.c index 1d729ed4d685..4df42d70d867 100644 --- a/drivers/thermal/rcar_thermal.c +++ b/drivers/thermal/rcar_thermal.c @@ -271,13 +271,6 @@ static int rcar_thermal_get_current_temp(struct rcar_thermal_priv *priv, return 0; } -static int rcar_thermal_of_get_temp(void *data, int *temp) -{ - struct rcar_thermal_priv *priv = data; - - return rcar_thermal_get_current_temp(priv, temp); -} - static int rcar_thermal_get_temp(struct thermal_zone_device *zone, int *temp) { struct rcar_thermal_priv *priv = rcar_zone_to_priv(zone); @@ -323,8 +316,8 @@ static int rcar_thermal_get_trip_temp(struct thermal_zone_device *zone, return 0; } -static const struct thermal_zone_of_device_ops rcar_thermal_zone_of_ops = { - .get_temp = rcar_thermal_of_get_temp, +static struct thermal_zone_device_ops rcar_thermal_zone_of_ops = { + .get_temp = rcar_thermal_get_temp, }; static struct thermal_zone_device_ops rcar_thermal_zone_ops = { @@ -534,7 +527,7 @@ static int rcar_thermal_probe(struct platform_device *pdev) goto error_unregister; if (chip->use_of_thermal) { - priv->zone = devm_thermal_zone_of_sensor_register( + priv->zone = devm_thermal_of_zone_register( dev, i, priv, &rcar_thermal_zone_of_ops); } else {
The thermal OF code has a new API allowing to migrate the OF initialization to a simpler approach. Use this new API. Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org> --- drivers/thermal/rcar_gen3_thermal.c | 16 ++++++++-------- drivers/thermal/rcar_thermal.c | 13 +++---------- 2 files changed, 11 insertions(+), 18 deletions(-)