From fe6603cafabbe9c0213cfaaca24bcbb3cc6e2a51 Mon Sep 17 00:00:00 2001 From: Tony Luck Date: Mon, 13 Mar 2023 17:46:11 -0700 Subject: [PATCH 1/7] ACPI: APEI: EINJ: Add CXL error types ACPI 6.5 added six new error types for CXL. See chapter 18 table 18.30. Add strings for the new types so that Linux will list them in the /sys/kernel/debug/apei/einj/available_error_types file. It seems no other changes are needed. Linux already accepts the CXL codes (on a BIOS that advertises them). Signed-off-by: Tony Luck Reviewed-by: Davidlohr Bueso Reviewed-by: Dave Jiang Signed-off-by: Rafael J. Wysocki --- drivers/acpi/apei/einj.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c index b4373e575660..39bee5a067cc 100644 --- a/drivers/acpi/apei/einj.c +++ b/drivers/acpi/apei/einj.c @@ -584,6 +584,12 @@ static const char * const einj_error_type_string[] = { "0x00000200\tPlatform Correctable\n", "0x00000400\tPlatform Uncorrectable non-fatal\n", "0x00000800\tPlatform Uncorrectable fatal\n", + "0x00001000\tCXL.cache Protocol Correctable\n", + "0x00002000\tCXL.cache Protocol Uncorrectable non-fatal\n", + "0x00004000\tCXL.cache Protocol Uncorrectable fatal\n", + "0x00008000\tCXL.mem Protocol Correctable\n", + "0x00010000\tCXL.mem Protocol Uncorrectable non-fatal\n", + "0x00020000\tCXL.mem Protocol Uncorrectable fatal\n", }; static int available_error_type_show(struct seq_file *m, void *v) From f1e65718ec1855263c349997dc294e186e55d67a Mon Sep 17 00:00:00 2001 From: Shuai Xue Date: Thu, 23 Mar 2023 09:53:57 +0800 Subject: [PATCH 2/7] ACPI: APEI: EINJ: warn on invalid argument when explicitly indicated by platform OSPM executes an EXECUTE_OPERATION action to instruct the platform to begin the injection operation, then executes a GET_COMMAND_STATUS action to determine the status of the completed operation. The ACPI Specification documented error codes[1] are: 0 = Success (Linux #define EINJ_STATUS_SUCCESS) 1 = Unknown failure (Linux #define EINJ_STATUS_FAIL) 2 = Invalid Access (Linux #define EINJ_STATUS_INVAL) The original code report -EBUSY for both "Unknown Failure" and "Invalid Access" cases. Actually, firmware could do some platform dependent sanity checks and returns different error codes, e.g. "Invalid Access" to indicate to the user that the parameters they supplied cannot be used for injection. To this end, fix to return -EINVAL in the __einj_error_inject() error handling case instead of always -EBUSY, when explicitly indicated by the platform in the status of the completed operation. [1] ACPI Specification 6.5 18.6.1. Error Injection Table Signed-off-by: Shuai Xue Reviewed-by: Tony Luck Signed-off-by: Rafael J. Wysocki --- drivers/acpi/apei/einj.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/apei/einj.c b/drivers/acpi/apei/einj.c index 39bee5a067cc..013eb621dc92 100644 --- a/drivers/acpi/apei/einj.c +++ b/drivers/acpi/apei/einj.c @@ -489,9 +489,15 @@ static int __einj_error_inject(u32 type, u32 flags, u64 param1, u64 param2, if (rc) return rc; val = apei_exec_ctx_get_output(&ctx); - if (val != EINJ_STATUS_SUCCESS) + if (val == EINJ_STATUS_FAIL) return -EBUSY; + else if (val == EINJ_STATUS_INVAL) + return -EINVAL; + /* + * The error is injected into the platform successfully, then it needs + * to trigger the error. + */ rc = apei_exec_run(&ctx, ACPI_EINJ_GET_TRIGGER_TABLE); if (rc) return rc; From 1fbd9029c8d5ccd3de49cb0f5382cffc7444a32b Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Fri, 24 Mar 2023 13:41:46 +0200 Subject: [PATCH 3/7] ACPI: property: Refactor acpi_data_prop_read_single() Refactor acpi_data_prop_read_single() for decreased indentation and better structure. No functional changes intended. Signed-off-by: Andy Shevchenko Reviewed-by: Sakari Ailus Signed-off-by: Rafael J. Wysocki --- drivers/acpi/property.c | 80 ++++++++++++++++++----------------------- 1 file changed, 34 insertions(+), 46 deletions(-) diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c index b8d9eb9a433e..413e4fcadcaf 100644 --- a/drivers/acpi/property.c +++ b/drivers/acpi/property.c @@ -971,60 +971,48 @@ static int acpi_data_prop_read_single(const struct acpi_device_data *data, enum dev_prop_type proptype, void *val) { const union acpi_object *obj; - int ret; + int ret = 0; - if (proptype >= DEV_PROP_U8 && proptype <= DEV_PROP_U64) { + if (proptype >= DEV_PROP_U8 && proptype <= DEV_PROP_U64) ret = acpi_data_get_property(data, propname, ACPI_TYPE_INTEGER, &obj); - if (ret) - return ret; - - switch (proptype) { - case DEV_PROP_U8: - if (obj->integer.value > U8_MAX) - return -EOVERFLOW; - - if (val) - *(u8 *)val = obj->integer.value; - - break; - case DEV_PROP_U16: - if (obj->integer.value > U16_MAX) - return -EOVERFLOW; - - if (val) - *(u16 *)val = obj->integer.value; - - break; - case DEV_PROP_U32: - if (obj->integer.value > U32_MAX) - return -EOVERFLOW; - - if (val) - *(u32 *)val = obj->integer.value; - - break; - default: - if (val) - *(u64 *)val = obj->integer.value; - - break; - } - - if (!val) - return 1; - } else if (proptype == DEV_PROP_STRING) { + else if (proptype == DEV_PROP_STRING) ret = acpi_data_get_property(data, propname, ACPI_TYPE_STRING, &obj); - if (ret) - return ret; + if (ret) + return ret; + switch (proptype) { + case DEV_PROP_U8: + if (obj->integer.value > U8_MAX) + return -EOVERFLOW; + if (val) + *(u8 *)val = obj->integer.value; + break; + case DEV_PROP_U16: + if (obj->integer.value > U16_MAX) + return -EOVERFLOW; + if (val) + *(u16 *)val = obj->integer.value; + break; + case DEV_PROP_U32: + if (obj->integer.value > U32_MAX) + return -EOVERFLOW; + if (val) + *(u32 *)val = obj->integer.value; + break; + case DEV_PROP_U64: + if (val) + *(u64 *)val = obj->integer.value; + break; + case DEV_PROP_STRING: if (val) *(char **)val = obj->string.pointer; - return 1; - } else { - ret = -EINVAL; + default: + return -EINVAL; } - return ret; + + /* When no storage provided return number of available values */ + return val ? 0 : 1; } #define acpi_copy_property_array_uint(items, val, nval) \ From 28f7b85895fce996c0f34827a7623ef6a031e4f3 Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Fri, 24 Mar 2023 21:26:26 +0100 Subject: [PATCH 4/7] ACPI: EC: Limit explicit removal of query handlers to custom query handlers According to the ACPI spec part 5.6.4.1.2, EC query handlers discovered thru ACPI should not be removed when a driver removes his custom query handler. On the Acer Travelmate 4002WLMi for example, such a query handler is used as a fallback to handle the EC SMBus alert when no driver is present. Change acpi_ec_remove_query_handlers() so that only custom query handlers are removed then remove_all is false. Query handlers discovered thru ACPI will still get removed when remove_all is true, which happens on device removal. Also add a simple check to ensure that acpi_ec_add_query_handler() is always called with either handle or func being set, since custom query handlers are detected based whether handlers->func is set or not. Tested on a Acer Travelmate 4002WLMi. Signed-off-by: Armin Wolf [ rjw: Comment adjustment ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 105d2e795afa..5ef72287d1be 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1083,9 +1083,12 @@ int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, acpi_handle handle, acpi_ec_query_func func, void *data) { - struct acpi_ec_query_handler *handler = - kzalloc(sizeof(struct acpi_ec_query_handler), GFP_KERNEL); + struct acpi_ec_query_handler *handler; + if (!handle && !func) + return -EINVAL; + + handler = kzalloc(sizeof(*handler), GFP_KERNEL); if (!handler) return -ENOMEM; @@ -1097,6 +1100,7 @@ int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit, kref_init(&handler->kref); list_add(&handler->node, &ec->list); mutex_unlock(&ec->mutex); + return 0; } EXPORT_SYMBOL_GPL(acpi_ec_add_query_handler); @@ -1109,9 +1113,16 @@ static void acpi_ec_remove_query_handlers(struct acpi_ec *ec, mutex_lock(&ec->mutex); list_for_each_entry_safe(handler, tmp, &ec->list, node) { - if (remove_all || query_bit == handler->query_bit) { + /* + * When remove_all is false, only remove custom query handlers + * which have handler->func set. This is done to preserve query + * handlers discovered thru ACPI, as they should continue handling + * EC queries. + */ + if (remove_all || (handler->func && handler->query_bit == query_bit)) { list_del_init(&handler->node); list_add(&handler->node, &free_list); + } } mutex_unlock(&ec->mutex); From e5b492c6bb900fcf9722e05f4a10924410e170c1 Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Fri, 24 Mar 2023 21:26:27 +0100 Subject: [PATCH 5/7] ACPI: EC: Fix oops when removing custom query handlers When removing custom query handlers, the handler might still be used inside the EC query workqueue, causing a kernel oops if the module holding the callback function was already unloaded. Fix this by flushing the EC query workqueue when removing custom query handlers. Tested on a Acer Travelmate 4002WLMi Signed-off-by: Armin Wolf Signed-off-by: Rafael J. Wysocki --- drivers/acpi/ec.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c index 5ef72287d1be..928899ab9502 100644 --- a/drivers/acpi/ec.c +++ b/drivers/acpi/ec.c @@ -1133,6 +1133,7 @@ static void acpi_ec_remove_query_handlers(struct acpi_ec *ec, void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit) { acpi_ec_remove_query_handlers(ec, false, query_bit); + flush_workqueue(ec_query_wq); } EXPORT_SYMBOL_GPL(acpi_ec_remove_query_handler); From 3bd554e03ef412438b42862cb088ddc4e2a2561b Mon Sep 17 00:00:00 2001 From: Armin Wolf Date: Fri, 24 Mar 2023 21:26:28 +0100 Subject: [PATCH 6/7] ACPI: SBS: Fix handling of Smart Battery Selectors The "Smart Battery Selector" standard says that when writing SelectorState (0x1), the nibbles which should not be modified need to be masked with 0xff. This is necessary since in contrast to a "Smart Battery Manager", the last three nibbles are writable. Failing to do so might trigger the following cycle: 1. Host accidentally changes power source of the system (3rd nibble) when selecting a battery. 2. Power source is invalid, Selector changes to another power source. 3. Selector notifies host that it changed the power source. 4. Host re-reads some batteries. 5. goto 1 for each re-read battery. This loop might also be entered when a battery which is not present is selected for SMBus access. In the end some workqueues fill up, which causes the system to lockup upon suspend/shutdown. Fix this by correctly masking the value to be written, and avoid selecting batteries which are absent. Tested on a Acer Travelmate 4002WLMi. Signed-off-by: Armin Wolf Signed-off-by: Rafael J. Wysocki --- drivers/acpi/sbs.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/drivers/acpi/sbs.c b/drivers/acpi/sbs.c index e90752d4f488..94e3c000df2e 100644 --- a/drivers/acpi/sbs.c +++ b/drivers/acpi/sbs.c @@ -473,23 +473,32 @@ static const struct device_attribute alarm_attr = { -------------------------------------------------------------------------- */ static int acpi_battery_read(struct acpi_battery *battery) { - int result = 0, saved_present = battery->present; + int result, saved_present = battery->present; u16 state; if (battery->sbs->manager_present) { result = acpi_smbus_read(battery->sbs->hc, SMBUS_READ_WORD, ACPI_SBS_MANAGER, 0x01, (u8 *)&state); - if (!result) - battery->present = state & (1 << battery->id); - state &= 0x0fff; + if (result) + return result; + + battery->present = state & (1 << battery->id); + if (!battery->present) + return 0; + + /* Masking necessary for Smart Battery Selectors */ + state = 0x0fff; state |= 1 << (battery->id + 12); acpi_smbus_write(battery->sbs->hc, SMBUS_WRITE_WORD, ACPI_SBS_MANAGER, 0x01, (u8 *)&state, 2); - } else if (battery->id == 0) - battery->present = 1; - - if (result || !battery->present) - return result; + } else { + if (battery->id == 0) { + battery->present = 1; + } else { + if (!battery->present) + return 0; + } + } if (saved_present != battery->present) { battery->update_time = 0; From 0dc9a715578b041b6159714f23f94cf71a214411 Mon Sep 17 00:00:00 2001 From: Jiangshan Yi Date: Tue, 28 Mar 2023 11:16:29 +0800 Subject: [PATCH 7/7] ACPI: thermal: Replace ternary operator with min_t() Modify the code in accordance with the coccicheck warning: drivers/acpi/thermal.c:422: WARNING opportunity for min(). min_t() macro is defined in include/linux/minmax.h. It avoids multiple evaluations of the arguments when non-constant and performs strict type-checking. Signed-off-by: Jiangshan Yi [ rjw: Changelog edits ] Signed-off-by: Rafael J. Wysocki --- drivers/acpi/thermal.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c index 0b4b844f9d4c..179f41196a9d 100644 --- a/drivers/acpi/thermal.c +++ b/drivers/acpi/thermal.c @@ -419,10 +419,9 @@ static int acpi_thermal_trips_update(struct acpi_thermal *tz, int flag) * the next higher trip point */ tz->trips.active[i-1].temperature = - (tz->trips.active[i-2].temperature < - celsius_to_deci_kelvin(act) ? - tz->trips.active[i-2].temperature : - celsius_to_deci_kelvin(act)); + min_t(unsigned long, + tz->trips.active[i-2].temperature, + celsius_to_deci_kelvin(act)); break; } else {