Hi Folks,
These patches clean up the ACPI thermal driver in a few different ways and
none of them is expected to affect the functionality.
Thanks!
From: Rafael J. Wysocki <[email protected]>
Drop the ACPI_TRIPS_REFRESH_DEVICES symbol which is redundant, because
ACPI_TRIPS_DEVICES can be used directly instead of it without any
drawbacks and rename the ACPI_TRIPS_REFRESH_THRESHOLDS to
ACPI_TRIPS_THRESHOLDS to make the code a bit more consistent.
While at it, fix up some formatting white space used in the symbol
definitions.
No functional impact.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/thermal.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -238,12 +238,11 @@ static int acpi_thermal_set_cooling_mode
#define ACPI_TRIPS_ACTIVE BIT(3)
#define ACPI_TRIPS_DEVICES BIT(4)
-#define ACPI_TRIPS_REFRESH_THRESHOLDS (ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE)
-#define ACPI_TRIPS_REFRESH_DEVICES ACPI_TRIPS_DEVICES
+#define ACPI_TRIPS_THRESHOLDS (ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE)
-#define ACPI_TRIPS_INIT (ACPI_TRIPS_CRITICAL | ACPI_TRIPS_HOT | \
- ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE | \
- ACPI_TRIPS_DEVICES)
+#define ACPI_TRIPS_INIT (ACPI_TRIPS_CRITICAL | ACPI_TRIPS_HOT | \
+ ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE | \
+ ACPI_TRIPS_DEVICES)
/*
* This exception is thrown out in two cases:
@@ -906,13 +905,13 @@ static void acpi_thermal_notify(struct a
acpi_queue_thermal_check(tz);
break;
case ACPI_THERMAL_NOTIFY_THRESHOLDS:
- acpi_thermal_trips_update(tz, ACPI_TRIPS_REFRESH_THRESHOLDS);
+ acpi_thermal_trips_update(tz, ACPI_TRIPS_THRESHOLDS);
acpi_queue_thermal_check(tz);
acpi_bus_generate_netlink_event(device->pnp.device_class,
dev_name(&device->dev), event, 0);
break;
case ACPI_THERMAL_NOTIFY_DEVICES:
- acpi_thermal_trips_update(tz, ACPI_TRIPS_REFRESH_DEVICES);
+ acpi_thermal_trips_update(tz, ACPI_TRIPS_DEVICES);
acpi_queue_thermal_check(tz);
acpi_bus_generate_netlink_event(device->pnp.device_class,
dev_name(&device->dev), event, 0);
From: Rafael J. Wysocki <[email protected]>
Use the BIT() macro for defining flag symbols in the ACPI thermal driver
instead of using "raw" values for the flags.
No functional impact.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/thermal.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -232,11 +232,11 @@ static int acpi_thermal_set_cooling_mode
return 0;
}
-#define ACPI_TRIPS_CRITICAL 0x01
-#define ACPI_TRIPS_HOT 0x02
-#define ACPI_TRIPS_PASSIVE 0x04
-#define ACPI_TRIPS_ACTIVE 0x08
-#define ACPI_TRIPS_DEVICES 0x10
+#define ACPI_TRIPS_CRITICAL BIT(0)
+#define ACPI_TRIPS_HOT BIT(1)
+#define ACPI_TRIPS_PASSIVE BIT(2)
+#define ACPI_TRIPS_ACTIVE BIT(3)
+#define ACPI_TRIPS_DEVICES BIT(4)
#define ACPI_TRIPS_REFRESH_THRESHOLDS (ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE)
#define ACPI_TRIPS_REFRESH_DEVICES ACPI_TRIPS_DEVICES
From: Rafael J. Wysocki <[email protected]>
Move the definition of the acpi_thermal_driver structure closer to the
initialization code that registes the driver, so some function forward
declarations can be dropped.
Also move the module information to the end of the file where it is
usually located.
No functional impact.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/thermal.c | 66 +++++++++++++++++++++++--------------------------
1 file changed, 31 insertions(+), 35 deletions(-)
Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -70,10 +70,6 @@ do { \
"Please report to [email protected]\n", str); \
} while (0)
-MODULE_AUTHOR("Paul Diefenbaugh");
-MODULE_DESCRIPTION("ACPI Thermal Zone Driver");
-MODULE_LICENSE("GPL");
-
static int act;
module_param(act, int, 0644);
MODULE_PARM_DESC(act, "Disable or override all lowest active trip points.");
@@ -100,37 +96,6 @@ MODULE_PARM_DESC(psv, "Disable or overri
static struct workqueue_struct *acpi_thermal_pm_queue;
-static int acpi_thermal_add(struct acpi_device *device);
-static void acpi_thermal_remove(struct acpi_device *device);
-static void acpi_thermal_notify(struct acpi_device *device, u32 event);
-
-static const struct acpi_device_id thermal_device_ids[] = {
- {ACPI_THERMAL_HID, 0},
- {"", 0},
-};
-MODULE_DEVICE_TABLE(acpi, thermal_device_ids);
-
-#ifdef CONFIG_PM_SLEEP
-static int acpi_thermal_suspend(struct device *dev);
-static int acpi_thermal_resume(struct device *dev);
-#else
-#define acpi_thermal_suspend NULL
-#define acpi_thermal_resume NULL
-#endif
-static SIMPLE_DEV_PM_OPS(acpi_thermal_pm, acpi_thermal_suspend, acpi_thermal_resume);
-
-static struct acpi_driver acpi_thermal_driver = {
- .name = "thermal",
- .class = ACPI_THERMAL_CLASS,
- .ids = thermal_device_ids,
- .ops = {
- .add = acpi_thermal_add,
- .remove = acpi_thermal_remove,
- .notify = acpi_thermal_notify,
- },
- .drv.pm = &acpi_thermal_pm,
-};
-
struct acpi_thermal_state {
u8 critical:1;
u8 hot:1;
@@ -1131,6 +1096,33 @@ static int acpi_thermal_resume(struct de
}
#endif
+static const struct acpi_device_id thermal_device_ids[] = {
+ {ACPI_THERMAL_HID, 0},
+ {"", 0},
+};
+MODULE_DEVICE_TABLE(acpi, thermal_device_ids);
+
+#ifdef CONFIG_PM_SLEEP
+static int acpi_thermal_suspend(struct device *dev);
+static int acpi_thermal_resume(struct device *dev);
+#else
+#define acpi_thermal_suspend NULL
+#define acpi_thermal_resume NULL
+#endif
+static SIMPLE_DEV_PM_OPS(acpi_thermal_pm, acpi_thermal_suspend, acpi_thermal_resume);
+
+static struct acpi_driver acpi_thermal_driver = {
+ .name = "thermal",
+ .class = ACPI_THERMAL_CLASS,
+ .ids = thermal_device_ids,
+ .ops = {
+ .add = acpi_thermal_add,
+ .remove = acpi_thermal_remove,
+ .notify = acpi_thermal_notify,
+ },
+ .drv.pm = &acpi_thermal_pm,
+};
+
static int thermal_act(const struct dmi_system_id *d) {
if (act == 0) {
pr_notice("%s detected: disabling all active thermal trip points\n",
@@ -1235,3 +1227,7 @@ static void __exit acpi_thermal_exit(voi
module_init(acpi_thermal_init);
module_exit(acpi_thermal_exit);
+
+MODULE_AUTHOR("Paul Diefenbaugh");
+MODULE_DESCRIPTION("ACPI Thermal Zone Driver");
+MODULE_LICENSE("GPL");
From: Rafael J. Wysocki <[email protected]>
Notice that the enabled flag is only needed for active trip points,
so drop struct acpi_thermal_state_flags, add a simple "bool valid" field
to the definitions of all trip point structures instead of flags and
add a "bool enabled" field to struct acpi_thermal_active.
Adjust the code using the modified structures accordingly.
No intentional functional impact.
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/thermal.c | 132 +++++++++++++++++++++++--------------------------
1 file changed, 64 insertions(+), 68 deletions(-)
Index: linux-pm/drivers/acpi/thermal.c
===================================================================
--- linux-pm.orig/drivers/acpi/thermal.c
+++ linux-pm/drivers/acpi/thermal.c
@@ -105,35 +105,30 @@ struct acpi_thermal_state {
int active_index;
};
-struct acpi_thermal_state_flags {
- u8 valid:1;
- u8 enabled:1;
- u8 reserved:6;
-};
-
struct acpi_thermal_critical {
- struct acpi_thermal_state_flags flags;
unsigned long temperature;
+ bool valid;
};
struct acpi_thermal_hot {
- struct acpi_thermal_state_flags flags;
unsigned long temperature;
+ bool valid;
};
struct acpi_thermal_passive {
- struct acpi_thermal_state_flags flags;
+ struct acpi_handle_list devices;
unsigned long temperature;
unsigned long tc1;
unsigned long tc2;
unsigned long tsp;
- struct acpi_handle_list devices;
+ bool valid;
};
struct acpi_thermal_active {
- struct acpi_thermal_state_flags flags;
- unsigned long temperature;
struct acpi_handle_list devices;
+ unsigned long temperature;
+ bool valid;
+ bool enabled;
};
struct acpi_thermal_trips {
@@ -229,7 +224,7 @@ static int acpi_thermal_trips_update(str
acpi_status status;
unsigned long long tmp;
struct acpi_handle_list devices;
- int valid = 0;
+ bool valid = false;
int i;
/* Critical Shutdown */
@@ -243,21 +238,21 @@ static int acpi_thermal_trips_update(str
* ... so lets discard those as invalid.
*/
if (ACPI_FAILURE(status)) {
- tz->trips.critical.flags.valid = 0;
+ tz->trips.critical.valid = false;
acpi_handle_debug(tz->device->handle,
"No critical threshold\n");
} else if (tmp <= 2732) {
pr_info(FW_BUG "Invalid critical threshold (%llu)\n", tmp);
- tz->trips.critical.flags.valid = 0;
+ tz->trips.critical.valid = false;
} else {
- tz->trips.critical.flags.valid = 1;
+ tz->trips.critical.valid = true;
acpi_handle_debug(tz->device->handle,
"Found critical threshold [%lu]\n",
tz->trips.critical.temperature);
}
- if (tz->trips.critical.flags.valid) {
+ if (tz->trips.critical.valid) {
if (crt == -1) {
- tz->trips.critical.flags.valid = 0;
+ tz->trips.critical.valid = false;
} else if (crt > 0) {
unsigned long crt_k = celsius_to_deci_kelvin(crt);
@@ -276,12 +271,12 @@ static int acpi_thermal_trips_update(str
if (flag & ACPI_TRIPS_HOT) {
status = acpi_evaluate_integer(tz->device->handle, "_HOT", NULL, &tmp);
if (ACPI_FAILURE(status)) {
- tz->trips.hot.flags.valid = 0;
+ tz->trips.hot.valid = false;
acpi_handle_debug(tz->device->handle,
"No hot threshold\n");
} else {
tz->trips.hot.temperature = tmp;
- tz->trips.hot.flags.valid = 1;
+ tz->trips.hot.valid = true;
acpi_handle_debug(tz->device->handle,
"Found hot threshold [%lu]\n",
tz->trips.hot.temperature);
@@ -289,9 +284,9 @@ static int acpi_thermal_trips_update(str
}
/* Passive (optional) */
- if (((flag & ACPI_TRIPS_PASSIVE) && tz->trips.passive.flags.valid) ||
+ if (((flag & ACPI_TRIPS_PASSIVE) && tz->trips.passive.valid) ||
flag == ACPI_TRIPS_INIT) {
- valid = tz->trips.passive.flags.valid;
+ valid = tz->trips.passive.valid;
if (psv == -1) {
status = AE_SUPPORT;
} else if (psv > 0) {
@@ -303,44 +298,44 @@ static int acpi_thermal_trips_update(str
}
if (ACPI_FAILURE(status)) {
- tz->trips.passive.flags.valid = 0;
+ tz->trips.passive.valid = false;
} else {
tz->trips.passive.temperature = tmp;
- tz->trips.passive.flags.valid = 1;
+ tz->trips.passive.valid = true;
if (flag == ACPI_TRIPS_INIT) {
status = acpi_evaluate_integer(tz->device->handle,
"_TC1", NULL, &tmp);
if (ACPI_FAILURE(status))
- tz->trips.passive.flags.valid = 0;
+ tz->trips.passive.valid = false;
else
tz->trips.passive.tc1 = tmp;
status = acpi_evaluate_integer(tz->device->handle,
"_TC2", NULL, &tmp);
if (ACPI_FAILURE(status))
- tz->trips.passive.flags.valid = 0;
+ tz->trips.passive.valid = false;
else
tz->trips.passive.tc2 = tmp;
status = acpi_evaluate_integer(tz->device->handle,
"_TSP", NULL, &tmp);
if (ACPI_FAILURE(status))
- tz->trips.passive.flags.valid = 0;
+ tz->trips.passive.valid = false;
else
tz->trips.passive.tsp = tmp;
}
}
}
- if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.passive.flags.valid) {
+ if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.passive.valid) {
memset(&devices, 0, sizeof(struct acpi_handle_list));
status = acpi_evaluate_reference(tz->device->handle, "_PSL",
NULL, &devices);
if (ACPI_FAILURE(status)) {
acpi_handle_info(tz->device->handle,
"Invalid passive threshold\n");
- tz->trips.passive.flags.valid = 0;
+ tz->trips.passive.valid = false;
} else {
- tz->trips.passive.flags.valid = 1;
+ tz->trips.passive.valid = true;
}
if (memcmp(&tz->trips.passive.devices, &devices,
@@ -351,24 +346,24 @@ static int acpi_thermal_trips_update(str
}
}
if ((flag & ACPI_TRIPS_PASSIVE) || (flag & ACPI_TRIPS_DEVICES)) {
- if (valid != tz->trips.passive.flags.valid)
+ if (valid != tz->trips.passive.valid)
ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
}
/* Active (optional) */
for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
char name[5] = { '_', 'A', 'C', ('0' + i), '\0' };
- valid = tz->trips.active[i].flags.valid;
+ valid = tz->trips.active[i].valid;
if (act == -1)
break; /* disable all active trip points */
if (flag == ACPI_TRIPS_INIT || ((flag & ACPI_TRIPS_ACTIVE) &&
- tz->trips.active[i].flags.valid)) {
+ tz->trips.active[i].valid)) {
status = acpi_evaluate_integer(tz->device->handle,
name, NULL, &tmp);
if (ACPI_FAILURE(status)) {
- tz->trips.active[i].flags.valid = 0;
+ tz->trips.active[i].valid = 0;
if (i == 0)
break;
@@ -390,21 +385,21 @@ static int acpi_thermal_trips_update(str
break;
} else {
tz->trips.active[i].temperature = tmp;
- tz->trips.active[i].flags.valid = 1;
+ tz->trips.active[i].valid = true;
}
}
name[2] = 'L';
- if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.active[i].flags.valid) {
+ if ((flag & ACPI_TRIPS_DEVICES) && tz->trips.active[i].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].flags.valid = 0;
+ tz->trips.active[i].valid = false;
} else {
- tz->trips.active[i].flags.valid = 1;
+ tz->trips.active[i].valid = true;
}
if (memcmp(&tz->trips.active[i].devices, &devices,
@@ -415,10 +410,10 @@ static int acpi_thermal_trips_update(str
}
}
if ((flag & ACPI_TRIPS_ACTIVE) || (flag & ACPI_TRIPS_DEVICES))
- if (valid != tz->trips.active[i].flags.valid)
+ if (valid != tz->trips.active[i].valid)
ACPI_THERMAL_TRIPS_EXCEPTION(flag, tz, "state");
- if (!tz->trips.active[i].flags.valid)
+ if (!tz->trips.active[i].valid)
break;
}
@@ -438,17 +433,18 @@ static int acpi_thermal_trips_update(str
static int acpi_thermal_get_trip_points(struct acpi_thermal *tz)
{
- int i, valid, ret = acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT);
+ int i, ret = acpi_thermal_trips_update(tz, ACPI_TRIPS_INIT);
+ bool valid;
if (ret)
return ret;
- valid = tz->trips.critical.flags.valid |
- tz->trips.hot.flags.valid |
- tz->trips.passive.flags.valid;
+ valid = tz->trips.critical.valid |
+ tz->trips.hot.valid |
+ tz->trips.passive.valid;
for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++)
- valid |= tz->trips.active[i].flags.valid;
+ valid = valid || tz->trips.active[i].valid;
if (!valid) {
pr_warn(FW_BUG "No valid trip found\n");
@@ -485,7 +481,7 @@ static int thermal_get_trip_type(struct
if (!tz || trip < 0)
return -EINVAL;
- if (tz->trips.critical.flags.valid) {
+ if (tz->trips.critical.valid) {
if (!trip) {
*type = THERMAL_TRIP_CRITICAL;
return 0;
@@ -493,7 +489,7 @@ static int thermal_get_trip_type(struct
trip--;
}
- if (tz->trips.hot.flags.valid) {
+ if (tz->trips.hot.valid) {
if (!trip) {
*type = THERMAL_TRIP_HOT;
return 0;
@@ -501,7 +497,7 @@ static int thermal_get_trip_type(struct
trip--;
}
- if (tz->trips.passive.flags.valid) {
+ if (tz->trips.passive.valid) {
if (!trip) {
*type = THERMAL_TRIP_PASSIVE;
return 0;
@@ -509,7 +505,7 @@ static int thermal_get_trip_type(struct
trip--;
}
- for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE && tz->trips.active[i].flags.valid; i++) {
+ for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE && tz->trips.active[i].valid; i++) {
if (!trip) {
*type = THERMAL_TRIP_ACTIVE;
return 0;
@@ -529,7 +525,7 @@ static int thermal_get_trip_temp(struct
if (!tz || trip < 0)
return -EINVAL;
- if (tz->trips.critical.flags.valid) {
+ if (tz->trips.critical.valid) {
if (!trip) {
*temp = deci_kelvin_to_millicelsius_with_offset(
tz->trips.critical.temperature,
@@ -539,7 +535,7 @@ static int thermal_get_trip_temp(struct
trip--;
}
- if (tz->trips.hot.flags.valid) {
+ if (tz->trips.hot.valid) {
if (!trip) {
*temp = deci_kelvin_to_millicelsius_with_offset(
tz->trips.hot.temperature,
@@ -549,7 +545,7 @@ static int thermal_get_trip_temp(struct
trip--;
}
- if (tz->trips.passive.flags.valid) {
+ if (tz->trips.passive.valid) {
if (!trip) {
*temp = deci_kelvin_to_millicelsius_with_offset(
tz->trips.passive.temperature,
@@ -560,7 +556,7 @@ static int thermal_get_trip_temp(struct
}
for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE &&
- tz->trips.active[i].flags.valid; i++) {
+ tz->trips.active[i].valid; i++) {
if (!trip) {
*temp = deci_kelvin_to_millicelsius_with_offset(
tz->trips.active[i].temperature,
@@ -578,7 +574,7 @@ static int thermal_get_crit_temp(struct
{
struct acpi_thermal *tz = thermal_zone_device_priv(thermal);
- if (tz->trips.critical.flags.valid) {
+ if (tz->trips.critical.valid) {
*temperature = deci_kelvin_to_millicelsius_with_offset(
tz->trips.critical.temperature,
tz->kelvin_offset);
@@ -664,13 +660,13 @@ static int acpi_thermal_cooling_device_c
int trip = -1;
int result = 0;
- if (tz->trips.critical.flags.valid)
+ if (tz->trips.critical.valid)
trip++;
- if (tz->trips.hot.flags.valid)
+ if (tz->trips.hot.valid)
trip++;
- if (tz->trips.passive.flags.valid) {
+ if (tz->trips.passive.valid) {
trip++;
for (i = 0; i < tz->trips.passive.devices.count; i++) {
handle = tz->trips.passive.devices.handles[i];
@@ -695,7 +691,7 @@ static int acpi_thermal_cooling_device_c
}
for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
- if (!tz->trips.active[i].flags.valid)
+ if (!tz->trips.active[i].valid)
break;
trip++;
@@ -783,19 +779,19 @@ static int acpi_thermal_register_thermal
acpi_status status;
int i;
- if (tz->trips.critical.flags.valid)
+ if (tz->trips.critical.valid)
trips++;
- if (tz->trips.hot.flags.valid)
+ if (tz->trips.hot.valid)
trips++;
- if (tz->trips.passive.flags.valid)
+ if (tz->trips.passive.valid)
trips++;
- for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE && tz->trips.active[i].flags.valid;
+ for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE && tz->trips.active[i].valid;
i++, trips++);
- if (tz->trips.passive.flags.valid)
+ if (tz->trips.passive.valid)
tz->thermal_zone = thermal_zone_device_register("acpitz", trips, 0, tz,
&acpi_thermal_zone_ops, NULL,
tz->trips.passive.tsp * 100,
@@ -965,7 +961,7 @@ static int acpi_thermal_get_info(struct
*/
static void acpi_thermal_guess_offset(struct acpi_thermal *tz)
{
- if (tz->trips.critical.flags.valid &&
+ if (tz->trips.critical.valid &&
(tz->trips.critical.temperature % 5) == 1)
tz->kelvin_offset = 273100;
else
@@ -1074,20 +1070,20 @@ static int acpi_thermal_resume(struct de
return -EINVAL;
for (i = 0; i < ACPI_THERMAL_MAX_ACTIVE; i++) {
- if (!tz->trips.active[i].flags.valid)
+ if (!tz->trips.active[i].valid)
break;
- tz->trips.active[i].flags.enabled = 1;
+ tz->trips.active[i].enabled = true;
for (j = 0; j < tz->trips.active[i].devices.count; j++) {
result = acpi_bus_update_power(
tz->trips.active[i].devices.handles[j],
&power_state);
if (result || (power_state != ACPI_STATE_D0)) {
- tz->trips.active[i].flags.enabled = 0;
+ tz->trips.active[i].enabled = false;
break;
}
}
- tz->state.active |= tz->trips.active[i].flags.enabled;
+ tz->state.active |= tz->trips.active[i].enabled;
}
acpi_queue_thermal_check(tz);
On 5/30/2023 5:44 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Use the BIT() macro for defining flag symbols in the ACPI thermal driver
> instead of using "raw" values for the flags.
>
> No functional impact.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/acpi/thermal.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> Index: linux-pm/drivers/acpi/thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/thermal.c
> +++ linux-pm/drivers/acpi/thermal.c
> @@ -232,11 +232,11 @@ static int acpi_thermal_set_cooling_mode
> return 0;
> }
>
> -#define ACPI_TRIPS_CRITICAL 0x01
> -#define ACPI_TRIPS_HOT 0x02
> -#define ACPI_TRIPS_PASSIVE 0x04
> -#define ACPI_TRIPS_ACTIVE 0x08
> -#define ACPI_TRIPS_DEVICES 0x10
> +#define ACPI_TRIPS_CRITICAL BIT(0)
> +#define ACPI_TRIPS_HOT BIT(1)
> +#define ACPI_TRIPS_PASSIVE BIT(2)
> +#define ACPI_TRIPS_ACTIVE BIT(3)
> +#define ACPI_TRIPS_DEVICES BIT(4)
>
> #define ACPI_TRIPS_REFRESH_THRESHOLDS (ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE)
> #define ACPI_TRIPS_REFRESH_DEVICES ACPI_TRIPS_DEVICES
Looks good to me,
Reviewed-by: Michal Wilczynski <[email protected]>
>
>
On 5/30/2023 5:44 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Drop the ACPI_TRIPS_REFRESH_DEVICES symbol which is redundant, because
> ACPI_TRIPS_DEVICES can be used directly instead of it without any
> drawbacks and rename the ACPI_TRIPS_REFRESH_THRESHOLDS to
> ACPI_TRIPS_THRESHOLDS to make the code a bit more consistent.
>
> While at it, fix up some formatting white space used in the symbol
> definitions.
>
> No functional impact.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/acpi/thermal.c | 13 ++++++-------
> 1 file changed, 6 insertions(+), 7 deletions(-)
>
> Index: linux-pm/drivers/acpi/thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/thermal.c
> +++ linux-pm/drivers/acpi/thermal.c
> @@ -238,12 +238,11 @@ static int acpi_thermal_set_cooling_mode
> #define ACPI_TRIPS_ACTIVE BIT(3)
> #define ACPI_TRIPS_DEVICES BIT(4)
>
> -#define ACPI_TRIPS_REFRESH_THRESHOLDS (ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE)
> -#define ACPI_TRIPS_REFRESH_DEVICES ACPI_TRIPS_DEVICES
> +#define ACPI_TRIPS_THRESHOLDS (ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE)
>
> -#define ACPI_TRIPS_INIT (ACPI_TRIPS_CRITICAL | ACPI_TRIPS_HOT | \
> - ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE | \
> - ACPI_TRIPS_DEVICES)
> +#define ACPI_TRIPS_INIT (ACPI_TRIPS_CRITICAL | ACPI_TRIPS_HOT | \
> + ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE | \
> + ACPI_TRIPS_DEVICES)
>
> /*
> * This exception is thrown out in two cases:
> @@ -906,13 +905,13 @@ static void acpi_thermal_notify(struct a
> acpi_queue_thermal_check(tz);
> break;
> case ACPI_THERMAL_NOTIFY_THRESHOLDS:
> - acpi_thermal_trips_update(tz, ACPI_TRIPS_REFRESH_THRESHOLDS);
> + acpi_thermal_trips_update(tz, ACPI_TRIPS_THRESHOLDS);
> acpi_queue_thermal_check(tz);
> acpi_bus_generate_netlink_event(device->pnp.device_class,
> dev_name(&device->dev), event, 0);
> break;
> case ACPI_THERMAL_NOTIFY_DEVICES:
> - acpi_thermal_trips_update(tz, ACPI_TRIPS_REFRESH_DEVICES);
> + acpi_thermal_trips_update(tz, ACPI_TRIPS_DEVICES);
> acpi_queue_thermal_check(tz);
> acpi_bus_generate_netlink_event(device->pnp.device_class,
> dev_name(&device->dev), event, 0);
>
Looks good to me,
Reviewed-by: Michal Wilczynski <[email protected]>
Also I wonder, whether I should wait with another revision of my patchset 'Remove .notify', since it will
obviously need to be rebased on top of that changes.
>
On 5/30/2023 5:44 PM, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <[email protected]>
>
> Move the definition of the acpi_thermal_driver structure closer to the
> initialization code that registes the driver, so some function forward
registe -> registers
I heard some people use 'codespell' for some automatic checking, but haven't
used that so far
> declarations can be dropped.
>
> Also move the module information to the end of the file where it is
> usually located.
>
> No functional impact.
>
> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/acpi/thermal.c | 66 +++++++++++++++++++++++--------------------------
> 1 file changed, 31 insertions(+), 35 deletions(-)
>
> Index: linux-pm/drivers/acpi/thermal.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/thermal.c
> +++ linux-pm/drivers/acpi/thermal.c
> @@ -70,10 +70,6 @@ do { \
> "Please report to [email protected]\n", str); \
> } while (0)
>
> -MODULE_AUTHOR("Paul Diefenbaugh");
> -MODULE_DESCRIPTION("ACPI Thermal Zone Driver");
> -MODULE_LICENSE("GPL");
> -
> static int act;
> module_param(act, int, 0644);
> MODULE_PARM_DESC(act, "Disable or override all lowest active trip points.");
> @@ -100,37 +96,6 @@ MODULE_PARM_DESC(psv, "Disable or overri
>
> static struct workqueue_struct *acpi_thermal_pm_queue;
>
> -static int acpi_thermal_add(struct acpi_device *device);
> -static void acpi_thermal_remove(struct acpi_device *device);
> -static void acpi_thermal_notify(struct acpi_device *device, u32 event);
> -
> -static const struct acpi_device_id thermal_device_ids[] = {
> - {ACPI_THERMAL_HID, 0},
> - {"", 0},
> -};
> -MODULE_DEVICE_TABLE(acpi, thermal_device_ids);
> -
> -#ifdef CONFIG_PM_SLEEP
> -static int acpi_thermal_suspend(struct device *dev);
> -static int acpi_thermal_resume(struct device *dev);
> -#else
> -#define acpi_thermal_suspend NULL
> -#define acpi_thermal_resume NULL
> -#endif
> -static SIMPLE_DEV_PM_OPS(acpi_thermal_pm, acpi_thermal_suspend, acpi_thermal_resume);
> -
> -static struct acpi_driver acpi_thermal_driver = {
> - .name = "thermal",
> - .class = ACPI_THERMAL_CLASS,
> - .ids = thermal_device_ids,
> - .ops = {
> - .add = acpi_thermal_add,
> - .remove = acpi_thermal_remove,
> - .notify = acpi_thermal_notify,
> - },
> - .drv.pm = &acpi_thermal_pm,
> -};
> -
> struct acpi_thermal_state {
> u8 critical:1;
> u8 hot:1;
> @@ -1131,6 +1096,33 @@ static int acpi_thermal_resume(struct de
> }
> #endif
>
> +static const struct acpi_device_id thermal_device_ids[] = {
> + {ACPI_THERMAL_HID, 0},
> + {"", 0},
> +};
> +MODULE_DEVICE_TABLE(acpi, thermal_device_ids);
> +
> +#ifdef CONFIG_PM_SLEEP
> +static int acpi_thermal_suspend(struct device *dev);
> +static int acpi_thermal_resume(struct device *dev);
> +#else
> +#define acpi_thermal_suspend NULL
> +#define acpi_thermal_resume NULL
> +#endif
> +static SIMPLE_DEV_PM_OPS(acpi_thermal_pm, acpi_thermal_suspend, acpi_thermal_resume);
> +
> +static struct acpi_driver acpi_thermal_driver = {
> + .name = "thermal",
> + .class = ACPI_THERMAL_CLASS,
> + .ids = thermal_device_ids,
> + .ops = {
> + .add = acpi_thermal_add,
> + .remove = acpi_thermal_remove,
> + .notify = acpi_thermal_notify,
> + },
> + .drv.pm = &acpi_thermal_pm,
> +};
> +
> static int thermal_act(const struct dmi_system_id *d) {
> if (act == 0) {
> pr_notice("%s detected: disabling all active thermal trip points\n",
> @@ -1235,3 +1227,7 @@ static void __exit acpi_thermal_exit(voi
>
> module_init(acpi_thermal_init);
> module_exit(acpi_thermal_exit);
> +
> +MODULE_AUTHOR("Paul Diefenbaugh");
> +MODULE_DESCRIPTION("ACPI Thermal Zone Driver");
> +MODULE_LICENSE("GPL");
>
Reviewed-by: Michal Wilczynski <[email protected]>
>
On 5/30/2023 5:45 PM, Rafael J. Wysocki wrote:
> |From: Rafael J. Wysocki <[email protected]> Notice that the enabled flag is only needed for active trip points, so drop struct acpi_thermal_state_flags, add a simple "bool valid" field to the definitions of all trip point structures instead of flags and add a "bool enabled" field to struct acpi_thermal_active. Adjust the code using the modified structures accordingly. No intentional functional impact. Signed-off-by: Rafael J. Wysocki <[email protected]> --- drivers/acpi/thermal.c | 132 +++++++++++++++++++++++-------------------------- 1 file changed, 64 insertions(+), 68 deletions(-)|
Reviewed-by: Michal Wilczynski <[email protected]>
> ||
On Wed, May 31, 2023 at 9:50 AM Wilczynski, Michal
<[email protected]> wrote:
>
>
>
> On 5/30/2023 5:44 PM, Rafael J. Wysocki wrote:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > Drop the ACPI_TRIPS_REFRESH_DEVICES symbol which is redundant, because
> > ACPI_TRIPS_DEVICES can be used directly instead of it without any
> > drawbacks and rename the ACPI_TRIPS_REFRESH_THRESHOLDS to
> > ACPI_TRIPS_THRESHOLDS to make the code a bit more consistent.
> >
> > While at it, fix up some formatting white space used in the symbol
> > definitions.
> >
> > No functional impact.
> >
> > Signed-off-by: Rafael J. Wysocki <[email protected]>
> > ---
> > drivers/acpi/thermal.c | 13 ++++++-------
> > 1 file changed, 6 insertions(+), 7 deletions(-)
> >
> > Index: linux-pm/drivers/acpi/thermal.c
> > ===================================================================
> > --- linux-pm.orig/drivers/acpi/thermal.c
> > +++ linux-pm/drivers/acpi/thermal.c
> > @@ -238,12 +238,11 @@ static int acpi_thermal_set_cooling_mode
> > #define ACPI_TRIPS_ACTIVE BIT(3)
> > #define ACPI_TRIPS_DEVICES BIT(4)
> >
> > -#define ACPI_TRIPS_REFRESH_THRESHOLDS (ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE)
> > -#define ACPI_TRIPS_REFRESH_DEVICES ACPI_TRIPS_DEVICES
> > +#define ACPI_TRIPS_THRESHOLDS (ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE)
> >
> > -#define ACPI_TRIPS_INIT (ACPI_TRIPS_CRITICAL | ACPI_TRIPS_HOT | \
> > - ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE | \
> > - ACPI_TRIPS_DEVICES)
> > +#define ACPI_TRIPS_INIT (ACPI_TRIPS_CRITICAL | ACPI_TRIPS_HOT | \
> > + ACPI_TRIPS_PASSIVE | ACPI_TRIPS_ACTIVE | \
> > + ACPI_TRIPS_DEVICES)
> >
> > /*
> > * This exception is thrown out in two cases:
> > @@ -906,13 +905,13 @@ static void acpi_thermal_notify(struct a
> > acpi_queue_thermal_check(tz);
> > break;
> > case ACPI_THERMAL_NOTIFY_THRESHOLDS:
> > - acpi_thermal_trips_update(tz, ACPI_TRIPS_REFRESH_THRESHOLDS);
> > + acpi_thermal_trips_update(tz, ACPI_TRIPS_THRESHOLDS);
> > acpi_queue_thermal_check(tz);
> > acpi_bus_generate_netlink_event(device->pnp.device_class,
> > dev_name(&device->dev), event, 0);
> > break;
> > case ACPI_THERMAL_NOTIFY_DEVICES:
> > - acpi_thermal_trips_update(tz, ACPI_TRIPS_REFRESH_DEVICES);
> > + acpi_thermal_trips_update(tz, ACPI_TRIPS_DEVICES);
> > acpi_queue_thermal_check(tz);
> > acpi_bus_generate_netlink_event(device->pnp.device_class,
> > dev_name(&device->dev), event, 0);
> >
>
> Looks good to me,
>
> Reviewed-by: Michal Wilczynski <[email protected]>
>
> Also I wonder, whether I should wait with another revision of my patchset 'Remove .notify', since it will
> obviously need to be rebased on top of that changes.
No need to wait, I can deal with merge conflicts just fine.