Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Tue, 31 Jan 2023 20:11:05 +0100 | Subject | Re: [PATCH 1/3] thermal/drivers/intel: Use generic trip points for quark_dts |
| |
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?
| |