| Message ID | 22010294.EfDdHjke4D@kreacher (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | ACPI: thermal: Removal of redundant data and cleanup | expand |
On Tue, Sep 12, 2023 at 8:47 PM Rafael J. Wysocki <rjw@rjwysocki.net> wrote: > > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Separate the code needed to update active trips (in a response to a > notification from the platform firmware) as well as to initialize them > from the code that is only necessary for their initialization and > cleanly divide it into functions that each carry out a specific action. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- > drivers/acpi/thermal.c | 197 ++++++++++++++++++++++++------------------------- > 1 file changed, 100 insertions(+), 97 deletions(-) > > Index: linux-pm/drivers/acpi/thermal.c > =================================================================== > --- linux-pm.orig/drivers/acpi/thermal.c > +++ linux-pm/drivers/acpi/thermal.c > @@ -184,94 +184,6 @@ static int acpi_thermal_temp(struct acpi > tz->kelvin_offset); > } > > -static void __acpi_thermal_trips_update(struct acpi_thermal *tz, int flag) > -{ > - acpi_status status; > - unsigned long long tmp; > - struct acpi_handle_list devices; > - bool valid = false; > - int i; > - > - /* Active (optional) */ > - for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) { > - char name[5] = { '_', 'A', 'C', ('0' + i), '\0' }; > - valid = tz->trips.active[i].trip.valid; > - > - if (act == -1) > - break; /* disable all active trip points */ > - > - if (flag == ACPI_TRIPS_INIT || ((flag & ACPI_TRIPS_ACTIVE) && > - tz->trips.active[i].trip.valid)) { > - status = acpi_evaluate_integer(tz->device->handle, > - name, NULL, &tmp); > - if (ACPI_FAILURE(status)) { > - tz->trips.active[i].trip.valid = false; > - if (i == 0) > - break; > - > - if (act <= 0) > - break; > - > - if (i == 1) > - tz->trips.active[0].trip.temperature = > - celsius_to_deci_kelvin(act); > - else > - /* > - * Don't allow override higher than > - * the next higher trip point > - */ > - tz->trips.active[i-1].trip.temperature = > - min_t(unsigned long, > - tz->trips.active[i-2].trip.temperature, > - celsius_to_deci_kelvin(act)); > - > - break; > - } else { > - tz->trips.active[i].trip.temperature = tmp; > - tz->trips.active[i].trip.valid = true; > - } > - } > - > - name[2] = 'L'; > - if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.active[i].trip.valid) { > - memset(&devices, 0, sizeof(struct acpi_handle_list)); > - status = acpi_evaluate_reference(tz->device->handle, > - name, NULL, &devices); > - if (ACPI_FAILURE(status)) { > - acpi_handle_info(tz->device->handle, > - "Invalid active%d threshold\n", i); > - tz->trips.active[i].trip.valid = false; > - } else { > - tz->trips.active[i].trip.valid = true; > - } > - > - if (memcmp(&tz->trips.active[i].devices, &devices, > - sizeof(struct acpi_handle_list))) { > - memcpy(&tz->trips.active[i].devices, &devices, > - sizeof(struct acpi_handle_list)); > - ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device"); > - } > - } > - if ((flag & ACPI_TRIPS_ACTIVE) || (flag & ACPI_TRIPS_DEVICES)) > - if (valid != tz->trips.active[i].trip.valid) > - ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state"); > - > - if (!tz->trips.active[i].trip.valid) > - break; > - } > - > - if (flag & ACPI_TRIPS_DEVICES) { > - memset(&devices, 0, sizeof(devices)); > - status = acpi_evaluate_reference(tz->device->handle, "_TZD", > - NULL, &devices); > - if (ACPI_SUCCESS(status) && > - memcmp(&tz->devices, &devices, sizeof(devices))) { > - tz->devices = devices; > - ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device"); > - } > - } > -} > - > static void update_acpi_thermal_trip_temp(struct acpi_thermal_trip *acpi_trip, > int temp) > { > @@ -338,6 +250,78 @@ static void acpi_thermal_update_passive_ > ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_PASSIVE, tz, "state"); > } > > +static long get_active_temp(struct acpi_thermal *tz, int index) > +{ > + char method[] = { '_', 'A', 'C', '0' + index, '\0' }; > + unsigned long long tmp; > + acpi_status status; > + > + status = acpi_evaluate_integer(tz->device->handle, method, NULL, &tmp); > + if (ACPI_FAILURE(status)) > + return THERMAL_TEMP_INVALID; > + > + /* > + * If an override has been provided, apply it so there are no active > + * trips with thresholds greater than the override. > + */ > + if (act > 0) { > + unsigned long long override = celsius_to_deci_kelvin(act); > + > + if (tmp > override) > + tmp = override; > + } > + return tmp; > +} > + > +static void acpi_thermal_update_active_trip(struct acpi_thermal *tz, int index) > +{ > + struct acpi_thermal_trip *acpi_trip = &tz->trips.active[index].trip; > + > + if (!acpi_trip->valid) > + return; > + > + update_acpi_thermal_trip_temp(acpi_trip, get_active_temp(tz, index)); > + if (!acpi_trip->valid) > + ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "state"); > +} > + > +static bool update_active_devices(struct acpi_thermal *tz, int index, bool compare) > +{ > + char method[] = { '_', 'A', 'L', '0' + index, '\0' }; > + struct acpi_handle_list devices; > + acpi_status status; > + > + memset(&devices, 0, sizeof(devices)); > + > + status = acpi_evaluate_reference(tz->device->handle, method, NULL, &devices); > + if (ACPI_FAILURE(status)) { > + acpi_handle_info(tz->device->handle, > + "Missing device list for active threshold %d\n", > + index); > + return false; > + } > + > + if (compare && memcmp(&tz->trips.active[index].devices, &devices, sizeof(devices))) > + ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "device"); > + > + memcpy(&tz->trips.active[index].devices, &devices, sizeof(devices)); > + return true; > +} > + > +static void acpi_thermal_update_active_devices(struct acpi_thermal *tz, int index) > +{ > + struct acpi_thermal_trip *acpi_trip = &tz->trips.active[index].trip; > + > + if (!acpi_trip->valid) > + return; > + > + if (update_active_devices(tz, index, true)) > + return; > + > + update_acpi_thermal_trip_temp(acpi_trip, THERMAL_TEMP_INVALID); > + ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "state"); > +} > + > static int acpi_thermal_adjust_trip(struct thermal_trip *trip, void *data) > { > struct acpi_thermal_trip *acpi_trip = trip->priv; > @@ -358,18 +342,18 @@ static void acpi_thermal_adjust_thermal_ > unsigned long data) > { > struct acpi_thermal *tz = thermal_zone_device_priv(thermal); > - int flag; > + int i; > > if (data == ACPI_THERMAL_NOTIFY_THRESHOLDS) { > acpi_thermal_update_passive_trip(tz); > - flag = ACPI_TRIPS_THRESHOLDS; > + for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) > + acpi_thermal_update_active_trip(tz, i); > } else { > acpi_thermal_update_passive_devices(tz); > - flag = ACPI_TRIPS_DEVICES; > + for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) > + acpi_thermal_update_active_devices(tz, i); > } > > - __acpi_thermal_trips_update(tz, flag); > - > for_each_thermal_trip(tz->thermal_zone, acpi_thermal_adjust_trip, tz); > } > > @@ -498,6 +482,28 @@ fail: > return false; > } > > +static bool acpi_thermal_init_active_trip(struct acpi_thermal *tz, int index) > +{ > + long temp; > + > + if (act == -1) > + goto fail; > + > + temp = get_active_temp(tz, index); > + if (temp == THERMAL_TEMP_INVALID) > + goto fail; > + > + if (!update_active_devices(tz, false, index)) This should be if (!update_active_devices(tz, index, false)) and I've already fixed it in the tree. > + goto fail; > + > + update_acpi_thermal_trip_temp(&tz->trips.active[index].trip, temp); > + return true; > + > +fail: > + update_acpi_thermal_trip_temp(&tz->trips.active[index].trip, THERMAL_TEMP_INVALID); > + return false; > +} > + > static int acpi_thermal_get_trip_points(struct acpi_thermal *tz) > { > unsigned int count = 0; > @@ -506,11 +512,8 @@ static int acpi_thermal_get_trip_points( > if (acpi_thermal_init_passive_trip(tz)) > count++; > > - /* Active trip points (optional). */ > - __acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT); > - > for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) { > - if (tz->trips.active[i].trip.valid) > + if (acpi_thermal_init_active_trip(tz, i)) > count++; > else > break; > > >
On 12/09/2023 20:43, Rafael J. Wysocki wrote: > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Separate the code needed to update active trips (in a response to a > notification from the platform firmware) as well as to initialize them > from the code that is only necessary for their initialization and > cleanly divide it into functions that each carry out a specific action. > > Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > --- Acked-by: Daniel Lezcano <daniel.lezcano@linaro.org>
Index: linux-pm/drivers/acpi/thermal.c =================================================================== --- linux-pm.orig/drivers/acpi/thermal.c +++ linux-pm/drivers/acpi/thermal.c @@ -184,94 +184,6 @@ static int acpi_thermal_temp(struct acpi tz->kelvin_offset); } -static void __acpi_thermal_trips_update(struct acpi_thermal *tz, int flag) -{ - acpi_status status; - unsigned long long tmp; - struct acpi_handle_list devices; - bool valid = false; - int i; - - /* Active (optional) */ - for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) { - char name[5] = { '_', 'A', 'C', ('0' + i), '\0' }; - valid = tz->trips.active[i].trip.valid; - - if (act == -1) - break; /* disable all active trip points */ - - if (flag == ACPI_TRIPS_INIT || ((flag & ACPI_TRIPS_ACTIVE) && - tz->trips.active[i].trip.valid)) { - status = acpi_evaluate_integer(tz->device->handle, - name, NULL, &tmp); - if (ACPI_FAILURE(status)) { - tz->trips.active[i].trip.valid = false; - if (i == 0) - break; - - if (act <= 0) - break; - - if (i == 1) - tz->trips.active[0].trip.temperature = - celsius_to_deci_kelvin(act); - else - /* - * Don't allow override higher than - * the next higher trip point - */ - tz->trips.active[i-1].trip.temperature = - min_t(unsigned long, - tz->trips.active[i-2].trip.temperature, - celsius_to_deci_kelvin(act)); - - break; - } else { - tz->trips.active[i].trip.temperature = tmp; - tz->trips.active[i].trip.valid = true; - } - } - - name[2] = 'L'; - if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.active[i].trip.valid) { - memset(&devices, 0, sizeof(struct acpi_handle_list)); - status = acpi_evaluate_reference(tz->device->handle, - name, NULL, &devices); - if (ACPI_FAILURE(status)) { - acpi_handle_info(tz->device->handle, - "Invalid active%d threshold\n", i); - tz->trips.active[i].trip.valid = false; - } else { - tz->trips.active[i].trip.valid = true; - } - - if (memcmp(&tz->trips.active[i].devices, &devices, - sizeof(struct acpi_handle_list))) { - memcpy(&tz->trips.active[i].devices, &devices, - sizeof(struct acpi_handle_list)); - ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device"); - } - } - if ((flag & ACPI_TRIPS_ACTIVE) || (flag & ACPI_TRIPS_DEVICES)) - if (valid != tz->trips.active[i].trip.valid) - ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state"); - - if (!tz->trips.active[i].trip.valid) - break; - } - - if (flag & ACPI_TRIPS_DEVICES) { - memset(&devices, 0, sizeof(devices)); - status = acpi_evaluate_reference(tz->device->handle, "_TZD", - NULL, &devices); - if (ACPI_SUCCESS(status) && - memcmp(&tz->devices, &devices, sizeof(devices))) { - tz->devices = devices; - ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "device"); - } - } -} - static void update_acpi_thermal_trip_temp(struct acpi_thermal_trip *acpi_trip, int temp) { @@ -338,6 +250,78 @@ static void acpi_thermal_update_passive_ ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_PASSIVE, tz, "state"); } +static long get_active_temp(struct acpi_thermal *tz, int index) +{ + char method[] = { '_', 'A', 'C', '0' + index, '\0' }; + unsigned long long tmp; + acpi_status status; + + status = acpi_evaluate_integer(tz->device->handle, method, NULL, &tmp); + if (ACPI_FAILURE(status)) + return THERMAL_TEMP_INVALID; + + /* + * If an override has been provided, apply it so there are no active + * trips with thresholds greater than the override. + */ + if (act > 0) { + unsigned long long override = celsius_to_deci_kelvin(act); + + if (tmp > override) + tmp = override; + } + return tmp; +} + +static void acpi_thermal_update_active_trip(struct acpi_thermal *tz, int index) +{ + struct acpi_thermal_trip *acpi_trip = &tz->trips.active[index].trip; + + if (!acpi_trip->valid) + return; + + update_acpi_thermal_trip_temp(acpi_trip, get_active_temp(tz, index)); + if (!acpi_trip->valid) + ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "state"); +} + +static bool update_active_devices(struct acpi_thermal *tz, int index, bool compare) +{ + char method[] = { '_', 'A', 'L', '0' + index, '\0' }; + struct acpi_handle_list devices; + acpi_status status; + + memset(&devices, 0, sizeof(devices)); + + status = acpi_evaluate_reference(tz->device->handle, method, NULL, &devices); + if (ACPI_FAILURE(status)) { + acpi_handle_info(tz->device->handle, + "Missing device list for active threshold %d\n", + index); + return false; + } + + if (compare && memcmp(&tz->trips.active[index].devices, &devices, sizeof(devices))) + ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "device"); + + memcpy(&tz->trips.active[index].devices, &devices, sizeof(devices)); + return true; +} + +static void acpi_thermal_update_active_devices(struct acpi_thermal *tz, int index) +{ + struct acpi_thermal_trip *acpi_trip = &tz->trips.active[index].trip; + + if (!acpi_trip->valid) + return; + + if (update_active_devices(tz, index, true)) + return; + + update_acpi_thermal_trip_temp(acpi_trip, THERMAL_TEMP_INVALID); + ACPI_THERMAL_TRIPS_EXCEPTION(ACPI_TRIPS_ACTIVE, tz, "state"); +} + static int acpi_thermal_adjust_trip(struct thermal_trip *trip, void *data) { struct acpi_thermal_trip *acpi_trip = trip->priv; @@ -358,18 +342,18 @@ static void acpi_thermal_adjust_thermal_ unsigned long data) { struct acpi_thermal *tz = thermal_zone_device_priv(thermal); - int flag; + int i; if (data == ACPI_THERMAL_NOTIFY_THRESHOLDS) { acpi_thermal_update_passive_trip(tz); - flag = ACPI_TRIPS_THRESHOLDS; + for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) + acpi_thermal_update_active_trip(tz, i); } else { acpi_thermal_update_passive_devices(tz); - flag = ACPI_TRIPS_DEVICES; + for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) + acpi_thermal_update_active_devices(tz, i); } - __acpi_thermal_trips_update(tz, flag); - for_each_thermal_trip(tz->thermal_zone, acpi_thermal_adjust_trip, tz); } @@ -498,6 +482,28 @@ fail: return false; } +static bool acpi_thermal_init_active_trip(struct acpi_thermal *tz, int index) +{ + long temp; + + if (act == -1) + goto fail; + + temp = get_active_temp(tz, index); + if (temp == THERMAL_TEMP_INVALID) + goto fail; + + if (!update_active_devices(tz, false, index)) + goto fail; + + update_acpi_thermal_trip_temp(&tz->trips.active[index].trip, temp); + return true; + +fail: + update_acpi_thermal_trip_temp(&tz->trips.active[index].trip, THERMAL_TEMP_INVALID); + return false; +} + static int acpi_thermal_get_trip_points(struct acpi_thermal *tz) { unsigned int count = 0; @@ -506,11 +512,8 @@ static int acpi_thermal_get_trip_points( if (acpi_thermal_init_passive_trip(tz)) count++; - /* Active trip points (optional). */ - __acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT); - for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) { - if (tz->trips.active[i].trip.valid) + if (acpi_thermal_init_active_trip(tz, i)) count++; else break;