Re: [PATCH 1/3] thermal/drivers/intel: Use generic trip points for quark_dts

From: Daniel Lezcano
Date: Tue Jan 31 2023 - 11:41:24 EST


On 26/01/2023 15:15, Rafael J. Wysocki wrote:
On Wed, Jan 18, 2023 at 7:16 PM Daniel Lezcano
<daniel.lezcano@xxxxxxxxxx> 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@xxxxxxxxxx>
---

[ ... ]

- 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