Messages in this thread | | | From | "Rafael J. Wysocki" <> | Date | Mon, 23 Jan 2023 21:19:02 +0100 | Subject | Re: [PATCH 3/3] thermal/drivers/intel: Use generic trip points for intel_soc_dts_iosf |
| |
On Mon, Jan 23, 2023 at 7:09 PM Daniel Lezcano <daniel.lezcano@linaro.org> wrote: > > > Hi Srinivas, > > On 19/01/2023 21:04, Rafael J. Wysocki wrote: > > On Wed, Jan 18, 2023 at 7:16 PM Daniel Lezcano > > <daniel.lezcano@linaro.org> wrote: > >> > >> From: Daniel Lezcano <daniel.lezcano@kernel.org> > >> > >> 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> > >> --- > > [ ... ] > > >> > >> @@ -320,7 +304,8 @@ static int add_dts_thermal_zone(int id, struct intel_soc_dts_sensor_entry *dts, > >> dts->trip_mask = trip_mask; > >> dts->trip_count = trip_count; > >> snprintf(name, sizeof(name), "soc_dts%d", id); > >> - dts->tzone = thermal_zone_device_register(name, > >> + dts->tzone = thermal_zone_device_register_with_trips(name, > >> + dts->trips, > >> trip_count, > >> trip_mask, > >> dts, &tzone_ops, > >> @@ -430,27 +415,28 @@ struct intel_soc_dts_sensors *intel_soc_dts_iosf_init( > >> notification = false; > >> else > >> notification = true; > >> - for (i = 0; i < SOC_MAX_DTS_SENSORS; ++i) { > >> - sensors->soc_dts[i].sensors = sensors; > >> - ret = add_dts_thermal_zone(i, &sensors->soc_dts[i], > >> - notification, trip_count, > >> - read_only_trip_count); > >> - if (ret) > >> - goto err_free; > >> - } > > > > How is this change related to the rest of the patch? > > We want to register the thermal zone with the trip points. > > thermal_zone_device_register() becomes > > thermal_zone_device_register_with_trips() > > But in the current code, the trip points are updated after the thermal > zones are created (and strictly speaking it is wrong as get_trip_temp > can be invoked before the trip points are updated). > > So the change inverts the initialization where we update the trip points > and then we register the thermal zones.
It would be nice to write this in the changelog too.
> >> > >> for (i = 0; i < SOC_MAX_DTS_SENSORS; ++i) { > >> ret = update_trip_temp(&sensors->soc_dts[i], 0, 0, > >> THERMAL_TRIP_PASSIVE); > >> if (ret) > >> - goto err_remove_zone; > >> + goto err_free; > >> > >> ret = update_trip_temp(&sensors->soc_dts[i], 1, 0, > >> THERMAL_TRIP_PASSIVE); > >> if (ret) > >> - goto err_remove_zone; > >> + goto err_free; > >> } > >> > >> + for (i = 0; i < SOC_MAX_DTS_SENSORS; ++i) { > >> + sensors->soc_dts[i].sensors = sensors; > >> + ret = add_dts_thermal_zone(i, &sensors->soc_dts[i], > >> + notification, trip_count, > >> + read_only_trip_count); > >> + if (ret) > >> + goto err_remove_zone; > >> + } > >> + > > --
| |