Messages in this thread | | | Date | Wed, 1 Feb 2023 00:55:57 +0100 | Subject | Re: [PATCH 1/3] thermal/drivers/intel: Use generic trip points for quark_dts | From | Daniel Lezcano <> |
| |
On 31/01/2023 20:11, Rafael J. Wysocki wrote: > On Tue, Jan 31, 2023 at 5:41 PM Daniel Lezcano > <daniel.lezcano@linaro.org> wrote: >> >> 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 ? > > I think that the trip points that are registered currently need to > still be registered after the change. > > Does registering a trip point with the temperature set to > THERMAL_TEMP_INVALID cause it to be effectively disabled?
No but if we have thermal_zone_get_trip() returning -EINVAL if THERMAL_TEMP_INVALID is set for the specified trip id. Then the registering will set the disabled flag.
https://git.kernel.org/pub/scm/linux/kernel/git/thermal/linux.git/tree/drivers/thermal/thermal_core.c?h=thermal/bleeding-edge#n1395
-- <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
| |