Messages in this thread | | | Date | Tue, 31 Jan 2023 17:41:10 +0100 | Subject | Re: [PATCH 1/3] thermal/drivers/intel: Use generic trip points for quark_dts | From | Daniel Lezcano <> |
| |
On 26/01/2023 15:15, Rafael J. Wysocki wrote: > On Wed, Jan 18, 2023 at 7:16 PM Daniel Lezcano > <daniel.lezcano@linaro.org> wrote: >> >> The thermal framework gives the possibility to register the trip >> points with the thermal zone. When that is done, no get_trip_* ops are >> needed and they can be removed. >> >> Convert ops content logic into generic trip points and register them with the >> thermal zone. >> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linaro.org> >> ---
[ ... ]
>> - aux_entry->tzone = thermal_zone_device_register("quark_dts", >> - QRK_MAX_DTS_TRIPS, >> - wr_mask, >> - aux_entry, &tzone_ops, NULL, 0, polling_delay); >> + err = get_trip_temp(QRK_DTS_ID_TP_CRITICAL, &temperature); >> + if (err) >> + goto err_ret; >> + >> + aux_entry->trips[QRK_DTS_ID_TP_CRITICAL].temperature = temperature; >> + aux_entry->trips[QRK_DTS_ID_TP_CRITICAL].type = THERMAL_TRIP_CRITICAL; >> + >> + err = get_trip_temp(QRK_DTS_ID_TP_HOT, &temperature); >> + if (err) >> + goto err_ret; > > If I'm not mistaken, this won't even try to register the thermal zone > if at least one trip cannot be initialized, but previously it was > registered in that case, but the trips that failed to respond were > disabled. > > This is a change in behavior that would at least need to be documented > in the changelog, but it isn't. > > I'm not sure if it is safe to make even, however.
Thanks for catching this.
Two solutions:
1. Set the temperature to THERMAL_TEMP_INVALID and change get_thermal_trip() to return -EINVAL or -ERANGE if the temperature is THERMAL_TEMP_INVALID
2. Register only the valid trip points.
What would be the preferable way ?
-- <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook | <http://twitter.com/#!/linaroorg> Twitter | <http://www.linaro.org/linaro-blog/> Blog
| |