From c1ff2354e41ab7449c6c6cf35bff576807b6f594 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Wed, 27 Sep 2023 17:26:13 +0100 Subject: [PATCH 01/11] soc: kunpeng_hccs: Migrate to use generic PCC shmem related macros ANBZ: #28799 commit a46e42c097982e258f89c64c5f52f30994bcfeda upstream Use the newly defined common and generic PCC shared memory region related macros in this driver to replace the locally defined ones. Cc: Huisong Li Reviewed-by: Huisong Li Link: https://lore.kernel.org/r/20230927-pcc_defines-v2-4-0b8ffeaef2e5@arm.com Signed-off-by: Sudeep Holla Signed-off-bu: Wanhan He --- drivers/soc/hisilicon/kunpeng_hccs.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/drivers/soc/hisilicon/kunpeng_hccs.c b/drivers/soc/hisilicon/kunpeng_hccs.c index f3810d9d1caa..27a96cafd1ea 100644 --- a/drivers/soc/hisilicon/kunpeng_hccs.c +++ b/drivers/soc/hisilicon/kunpeng_hccs.c @@ -31,10 +31,6 @@ #include "kunpeng_hccs.h" -/* PCC defines */ -#define HCCS_PCC_SIGNATURE_MASK 0x50434300 -#define HCCS_PCC_STATUS_CMD_COMPLETE BIT(0) - /* * Arbitrary retries in case the remote processor is slow to respond * to PCC commands @@ -187,7 +183,7 @@ static int hccs_check_chan_cmd_complete(struct hccs_dev *hdev) * deadline_us(timeout_us) until PCC command complete bit is set(cond) */ ret = readw_poll_timeout(&comm_base->status, status, - status & HCCS_PCC_STATUS_CMD_COMPLETE, + status & PCC_STATUS_CMD_COMPLETE, HCCS_POLL_STATUS_TIME_INTERVAL_US, cl_info->deadline_us); if (unlikely(ret)) @@ -208,7 +204,7 @@ static int hccs_pcc_cmd_send(struct hccs_dev *hdev, u8 cmd, int ret; /* Write signature for this subspace */ - tmp.signature = HCCS_PCC_SIGNATURE_MASK | hdev->chan_id; + tmp.signature = PCC_SIGNATURE | hdev->chan_id; /* Write to the shared command region */ tmp.command = cmd; /* Clear cmd complete bit */ -- Gitee From 90a280a52a31d351c48c131c2cb4c23899a6f201 Mon Sep 17 00:00:00 2001 From: Sudeep Holla Date: Wed, 27 Sep 2023 17:26:12 +0100 Subject: [PATCH 02/11] hwmon: (xgene) Migrate to use generic PCC shmem related macros ANBZ: #28799 commit 2cf39b806be74661cc347e99796c97d9f54b7c42 upstream Use the newly defined common and generic PCC shared memory region related macros in this driver to replace the locally defined ones. Cc: Jean Delvare Acked-by: Guenter Roeck Link: https://lore.kernel.org/r/20230927-pcc_defines-v2-3-0b8ffeaef2e5@arm.com Signed-off-by: Sudeep Holla Signed-off-by: Wanhan He --- drivers/hwmon/xgene-hwmon.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/drivers/hwmon/xgene-hwmon.c b/drivers/hwmon/xgene-hwmon.c index 6768dbf39039..a3ca1d5ff321 100644 --- a/drivers/hwmon/xgene-hwmon.c +++ b/drivers/hwmon/xgene-hwmon.c @@ -57,12 +57,6 @@ (MSG_TYPE_SET(MSG_TYPE_PWRMGMT) | \ MSG_SUBTYPE_SET(hndl) | TPC_CMD_SET(cmd) | type) -/* PCC defines */ -#define PCC_SIGNATURE_MASK 0x50424300 -#define PCCC_GENERATE_DB_INT BIT(15) -#define PCCS_CMD_COMPLETE BIT(0) -#define PCCS_SCI_DOORBEL BIT(1) -#define PCCS_PLATFORM_NOTIFICATION BIT(3) /* * Arbitrary retries in case the remote processor is slow to respond * to PCC commands @@ -142,15 +136,15 @@ static int xgene_hwmon_pcc_rd(struct xgene_hwmon_dev *ctx, u32 *msg) /* Write signature for subspace */ WRITE_ONCE(generic_comm_base->signature, - cpu_to_le32(PCC_SIGNATURE_MASK | ctx->mbox_idx)); + cpu_to_le32(PCC_SIGNATURE | ctx->mbox_idx)); /* Write to the shared command region */ WRITE_ONCE(generic_comm_base->command, - cpu_to_le16(MSG_TYPE(msg[0]) | PCCC_GENERATE_DB_INT)); + cpu_to_le16(MSG_TYPE(msg[0]) | PCC_CMD_GENERATE_DB_INTR)); /* Flip CMD COMPLETE bit */ val = le16_to_cpu(READ_ONCE(generic_comm_base->status)); - val &= ~PCCS_CMD_COMPLETE; + val &= ~PCC_STATUS_CMD_COMPLETE; WRITE_ONCE(generic_comm_base->status, cpu_to_le16(val)); /* Copy the message to the PCC comm space */ @@ -544,7 +538,7 @@ static void xgene_hwmon_pcc_rx_cb(struct mbox_client *cl, void *msg) msg = generic_comm_base + 1; /* Check if platform sends interrupt */ if (!xgene_word_tst_and_clr(&generic_comm_base->status, - PCCS_SCI_DOORBEL)) + PCC_STATUS_SCI_DOORBELL)) return; /* @@ -566,7 +560,7 @@ static void xgene_hwmon_pcc_rx_cb(struct mbox_client *cl, void *msg) TPC_CMD(((u32 *)msg)[0]) == TPC_ALARM))) { /* Check if platform completes command */ if (xgene_word_tst_and_clr(&generic_comm_base->status, - PCCS_CMD_COMPLETE)) { + PCC_STATUS_CMD_COMPLETE)) { ctx->sync_msg.msg = ((u32 *)msg)[0]; ctx->sync_msg.param1 = ((u32 *)msg)[1]; ctx->sync_msg.param2 = ((u32 *)msg)[2]; -- Gitee From 0880aa2c60c74c4e3b6c8f630b5ccf697c3db899 Mon Sep 17 00:00:00 2001 From: Kai-Heng Feng Date: Wed, 20 Mar 2024 16:43:16 +0800 Subject: [PATCH 03/11] ACPI: IPMI: Add helper to wait for when SMI is selected ANBZ: #28799 commit 670e98a34a9e44cd384bafbda681c8c8e072b714 upstream On Dell servers, many APCI methods of acpi_power_meter module evaluate variables inside IPMI region, so the region handler needs to be installed. In addition to that, the handler needs to be fully functional, and that depends on SMI being selected. So add a helper to let acpi_power_meter know when the handler is installed and ready to be used. Signed-off-by: Kai-Heng Feng Acked-by: Rafael J. Wysocki Link: https://lore.kernel.org/r/20240320084317.366853-1-kai.heng.feng@canonical.com Signed-off-by: Guenter Roeck Signed-off-by: Wanhan He --- drivers/acpi/acpi_ipmi.c | 23 ++++++++++++++++++++++- include/acpi/acpi_bus.h | 5 +++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index 0555f68c2dfd..5fba4dab5d08 100644 --- a/drivers/acpi/acpi_ipmi.c +++ b/drivers/acpi/acpi_ipmi.c @@ -22,6 +22,8 @@ MODULE_LICENSE("GPL"); /* the IPMI timeout is 5s */ #define IPMI_TIMEOUT (5000) #define ACPI_IPMI_MAX_MSG_LENGTH 64 +/* 2s should be suffient for SMI being selected */ +#define ACPI_IPMI_SMI_SELECTION_TIMEOUT (2 * HZ) struct acpi_ipmi_device { /* the device list attached to driver_data.ipmi_devices */ @@ -54,6 +56,7 @@ struct ipmi_driver_data { * to this selected global IPMI system interface. */ struct acpi_ipmi_device *selected_smi; + struct completion smi_selection_done; }; struct acpi_ipmi_msg { @@ -463,8 +466,10 @@ static void ipmi_register_bmc(int iface, struct device *dev) if (temp->handle == handle) goto err_lock; } - if (!driver_data.selected_smi) + if (!driver_data.selected_smi) { driver_data.selected_smi = ipmi_device; + complete(&driver_data.smi_selection_done); + } list_add_tail(&ipmi_device->head, &driver_data.ipmi_devices); mutex_unlock(&driver_data.ipmi_lock); @@ -578,6 +583,20 @@ acpi_ipmi_space_handler(u32 function, acpi_physical_address address, return status; } +int acpi_wait_for_acpi_ipmi(void) +{ + long ret; + + ret = wait_for_completion_interruptible_timeout(&driver_data.smi_selection_done, + ACPI_IPMI_SMI_SELECTION_TIMEOUT); + + if (ret <= 0) + return -ETIMEDOUT; + + return 0; +} +EXPORT_SYMBOL_GPL(acpi_wait_for_acpi_ipmi); + static int __init acpi_ipmi_init(void) { int result; @@ -586,6 +605,8 @@ static int __init acpi_ipmi_init(void) if (acpi_disabled) return 0; + init_completion(&driver_data.smi_selection_done); + status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT, ACPI_ADR_SPACE_IPMI, &acpi_ipmi_space_handler, diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index 43b353a05d37..9cb733b2329e 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -911,11 +911,16 @@ static inline void acpi_put_acpi_dev(struct acpi_device *adev) int acpi_scan_add_dep(acpi_handle handle, struct acpi_handle_list *dep_devices); u32 arch_acpi_add_auto_dep(acpi_handle handle); + +int acpi_wait_for_acpi_ipmi(void); + #else /* CONFIG_ACPI */ static inline int register_acpi_bus_type(void *bus) { return 0; } static inline int unregister_acpi_bus_type(void *bus) { return 0; } +static inline int acpi_wait_for_acpi_ipmi(void) { return 0; } + #endif /* CONFIG_ACPI */ #endif /*__ACPI_BUS_H__*/ -- Gitee From fc7c660c4a6f2c0dd710ace618e0fc7c65c660aa Mon Sep 17 00:00:00 2001 From: Kai-Heng Feng Date: Wed, 20 Mar 2024 16:43:17 +0800 Subject: [PATCH 04/11] hwmon: (acpi_power_meter) Ensure IPMI space handler is ready on Dell systems ANBZ: #28799 commit 71113b9be4259d4bd396f22034653a9035ba07fb upstream The following error can be observed at boot: [ 3.717920] ACPI Error: No handler for Region [SYSI] (00000000ab9e62c5) [IPMI] (20230628/evregion-130) [ 3.717928] ACPI Error: Region IPMI (ID=7) has no handler (20230628/exfldio-261) [ 3.717936] No Local Variables are initialized for Method [_GHL] [ 3.717938] No Arguments are initialized for method [_GHL] [ 3.717940] ACPI Error: Aborting method \_SB.PMI0._GHL due to previous error (AE_NOT_EXIST) (20230628/psparse-529) [ 3.717949] ACPI Error: Aborting method \_SB.PMI0._PMC due to previous error (AE_NOT_EXIST) (20230628/psparse-529) [ 3.717957] ACPI: \_SB_.PMI0: _PMC evaluation failed: AE_NOT_EXIST On Dell systems several methods of acpi_power_meter access variables in IPMI region [0], so wait until IPMI space handler is installed by acpi_ipmi and also wait until SMI is selected to make the space handler fully functional. Since the dependency is inside BIOS's ASL code and it's not discoverable, so use this fixup is a hack to workaround BIOS issue. [0] https://www.dell.com/support/manuals/en-us/redhat-enterprise-linux-v8.0/rhel8_rn_pub/advanced-configuration-and-power-interface-acpi-error-messages-displayed-in-dmesg?guid=guid-0d5ae482-1977-42cf-b417-3ed5c3f5ee62 Signed-off-by: Kai-Heng Feng Link: https://lore.kernel.org/r/20240320084317.366853-2-kai.heng.feng@canonical.com [groeck: Simplified added code] Signed-off-by: Guenter Roeck Signed-off-by: Wanhan He --- drivers/hwmon/acpi_power_meter.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c index b772c076a5ae..f2cf7879b4a7 100644 --- a/drivers/hwmon/acpi_power_meter.c +++ b/drivers/hwmon/acpi_power_meter.c @@ -884,6 +884,22 @@ static int acpi_power_meter_add(struct acpi_device *device) strcpy(acpi_device_class(device), ACPI_POWER_METER_CLASS); device->driver_data = resource; +#if IS_REACHABLE(CONFIG_ACPI_IPMI) + /* + * On Dell systems several methods of acpi_power_meter access + * variables in IPMI region, so wait until IPMI space handler is + * installed by acpi_ipmi and also wait until SMI is selected to make + * the space handler fully functional. + */ + if (dmi_match(DMI_SYS_VENDOR, "Dell Inc.")) { + struct acpi_device *ipi_device = acpi_dev_get_first_match_dev("IPI0001", NULL, -1); + + if (ipi_device && acpi_wait_for_acpi_ipmi()) + dev_warn(&device->dev, "Waiting for ACPI IPMI timeout"); + acpi_dev_put(ipi_device); + } +#endif + res = read_capabilities(resource); if (res) goto exit_free; -- Gitee From a6e5cf98da9949442d945e528b9755af57eb6069 Mon Sep 17 00:00:00 2001 From: Huisong Li Date: Tue, 12 Nov 2024 10:12:28 +0800 Subject: [PATCH 05/11] hwmon: (acpi_power_meter) Fix fail to load module on platform without _PMD method ANBZ: #28799 commit fabb1f813ec05975fd3428e72a62ef9f855fd3b4 upstream According to the ACPI specification, the _PMD method is optional. The acpi_power_meter driver shouldn't fail to load if the platform has no _PMD method. Signed-off-by: Huisong Li Message-ID: <20241112021228.22914-1-lihuisong@huawei.com> [groeck: Reworded commit description] Signed-off-by: Guenter Roeck Signed-off-by: Wanhan He --- drivers/hwmon/acpi_power_meter.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c index f2cf7879b4a7..8624231d255f 100644 --- a/drivers/hwmon/acpi_power_meter.c +++ b/drivers/hwmon/acpi_power_meter.c @@ -680,8 +680,9 @@ static int setup_attrs(struct acpi_power_meter_resource *resource) { int res = 0; + /* _PMD method is optional. */ res = read_domain_devices(resource); - if (res) + if (res != -ENODEV) return res; if (resource->caps.flags & POWER_METER_CAN_MEASURE) { -- Gitee From 0a6c87405d63fb496f48c0894f7c1cb9d8191a72 Mon Sep 17 00:00:00 2001 From: Huisong Li Date: Thu, 9 Jan 2025 16:17:07 +0800 Subject: [PATCH 06/11] hwmon: (acpi_power_meter) Fix uninitialized variables ANBZ: #28799 commit 7532e68f5d8f05353765d7585a791a14985d68b7 upstream The 'power1_alarm' attribute uses the 'power' and 'cap' in the acpi_power_meter_resource structure. Currently, these two fields are just updated when user query 'power' and 'cap' attribute. If user directly query the 'power1_alarm' attribute without queryng above two attributes, driver will use uninitialized variables to judge. So this patch adds the setting of alarm state and update 'cap' in the notification callback and update 'power' and 'cap' if needed to show the real alarm state. Signed-off-by: Huisong Li Link: https://lore.kernel.org/r/20250109081708.27366-2-lihuisong@huawei.com Signed-off-by: Guenter Roeck Signed-off-by: Wanhan He --- drivers/hwmon/acpi_power_meter.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c index 8624231d255f..e724a51d982d 100644 --- a/drivers/hwmon/acpi_power_meter.c +++ b/drivers/hwmon/acpi_power_meter.c @@ -84,6 +84,7 @@ struct acpi_power_meter_resource { u64 power; u64 cap; u64 avg_interval; + bool power_alarm; int sensors_valid; unsigned long sensors_last_updated; struct sensor_device_attribute sensors[NUM_SENSORS]; @@ -396,6 +397,9 @@ static ssize_t show_val(struct device *dev, struct acpi_device *acpi_dev = to_acpi_device(dev); struct acpi_power_meter_resource *resource = acpi_dev->driver_data; u64 val = 0; + int ret; + + guard(mutex)(&resource->lock); switch (attr->index) { case 0: @@ -423,10 +427,17 @@ static ssize_t show_val(struct device *dev, val = 0; break; case 6: - if (resource->power > resource->cap) - val = 1; - else - val = 0; + ret = update_meter(resource); + if (ret) + return ret; + /* need to update cap if not to support the notification. */ + if (!(resource->caps.flags & POWER_METER_CAN_NOTIFY)) { + ret = update_cap(resource); + if (ret) + return ret; + } + val = resource->power_alarm || resource->power > resource->cap; + resource->power_alarm = resource->power > resource->cap; break; case 7: case 8: @@ -848,12 +859,20 @@ static void acpi_power_meter_notify(struct acpi_device *device, u32 event) sysfs_notify(&device->dev.kobj, NULL, POWER_AVERAGE_NAME); break; case METER_NOTIFY_CAP: + mutex_lock(&resource->lock); + res = update_cap(resource); + if (res) + dev_err_once(&device->dev, "update cap failed when capping value is changed.\n"); + mutex_unlock(&resource->lock); sysfs_notify(&device->dev.kobj, NULL, POWER_CAP_NAME); break; case METER_NOTIFY_INTERVAL: sysfs_notify(&device->dev.kobj, NULL, POWER_AVG_INTERVAL_NAME); break; case METER_NOTIFY_CAPPING: + mutex_lock(&resource->lock); + resource->power_alarm = true; + mutex_unlock(&resource->lock); sysfs_notify(&device->dev.kobj, NULL, POWER_ALARM_NAME); dev_info(&device->dev, "Capping in progress.\n"); break; -- Gitee From fc3bedb3e95e7d1b5c885abd9e796518ab34f02b Mon Sep 17 00:00:00 2001 From: Huisong Li Date: Thu, 9 Jan 2025 16:17:08 +0800 Subject: [PATCH 07/11] hwmon: (acpi_power_meter) Fix update the power trip points on failure ANBZ: #28799 commit 02f1a5911550bd6b39526d18282b10d441e04ed1 upstream The power trip points maintained in local should not be updated when '_PTP' method fails to evaluate. Signed-off-by: Huisong Li Link: https://lore.kernel.org/r/20250109081708.27366-3-lihuisong@huawei.com Signed-off-by: Guenter Roeck Signed-off-by: Wanhan He --- drivers/hwmon/acpi_power_meter.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c index e724a51d982d..b9bee089227a 100644 --- a/drivers/hwmon/acpi_power_meter.c +++ b/drivers/hwmon/acpi_power_meter.c @@ -293,8 +293,8 @@ static ssize_t set_trip(struct device *dev, struct device_attribute *devattr, struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); struct acpi_device *acpi_dev = to_acpi_device(dev); struct acpi_power_meter_resource *resource = acpi_dev->driver_data; + unsigned long temp, trip_bk; int res; - unsigned long temp; res = kstrtoul(buf, 10, &temp); if (res) @@ -302,13 +302,15 @@ static ssize_t set_trip(struct device *dev, struct device_attribute *devattr, temp = DIV_ROUND_CLOSEST(temp, 1000); - mutex_lock(&resource->lock); + guard(mutex)(&resource->lock); + + trip_bk = resource->trip[attr->index - 7]; resource->trip[attr->index - 7] = temp; res = set_acpi_trip(resource); - mutex_unlock(&resource->lock); - - if (res) + if (res) { + resource->trip[attr->index - 7] = trip_bk; return res; + } return count; } -- Gitee From ac949d0d90daa753c3ac89377eaa91a46759ac11 Mon Sep 17 00:00:00 2001 From: Kazuhiro Abe Date: Wed, 15 Jan 2025 07:35:32 +0000 Subject: [PATCH 08/11] hwmon: (acpi_power_meter) Fix a check for the return value of read_domain_devices(). ANBZ: #28799 commit 8d6bf2e1055fa2cca4bf233f46d4d1e2086cc5ff upstream After commit fabb1f813ec0 ("hwmon: (acpi_power_meter) Fix fail to load module on platform without _PMD method"), the acpi_power_meter driver fails to load if the platform has _PMD method. To address this, add a check for successful read_domain_devices(). Tested on Nvidia Grace machine. Fixes: fabb1f813ec0 ("hwmon: (acpi_power_meter) Fix fail to load module on platform without _PMD method") Signed-off-by: Kazuhiro Abe Link: https://lore.kernel.org/r/20250115073532.3211000-1-fj1078ii@aa.jp.fujitsu.com [groeck: Dropped unnecessary () from expression] Signed-off-by: Guenter Roeck Signed-off-by: Wanhan He --- drivers/hwmon/acpi_power_meter.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c index b9bee089227a..9c287c1bc98e 100644 --- a/drivers/hwmon/acpi_power_meter.c +++ b/drivers/hwmon/acpi_power_meter.c @@ -695,7 +695,7 @@ static int setup_attrs(struct acpi_power_meter_resource *resource) /* _PMD method is optional. */ res = read_domain_devices(resource); - if (res != -ENODEV) + if (res && res != -ENODEV) return res; if (resource->caps.flags & POWER_METER_CAN_MEASURE) { -- Gitee From d8af235e8e683ffb59370ef8bd9669acec8b3c93 Mon Sep 17 00:00:00 2001 From: Huisong Li Date: Thu, 20 Feb 2025 11:08:32 +0800 Subject: [PATCH 09/11] hwmon: (acpi_power_meter) Fix the fake power alarm reporting ANBZ: #28799 commit 0ea627381eb527a0ebd262c690c3992085b87ff4 upstream We encountered a problem that a fake power alarm is reported to user on the platform unsupported notifications at the second step below: 1> Query 'power1_alarm' attribute when the power capping occurs. 2> Query 'power1_alarm' attribute when the power capping is over and the current average power is less then power cap value. The root cause is that the resource->power_alarm is set to true at the first step. And power meter use this old value to show the power alarm state instead of the current the comparison value. Signed-off-by: Huisong Li Link: https://lore.kernel.org/r/20250220030832.2976-1-lihuisong@huawei.com Signed-off-by: Guenter Roeck Signed-off-by: Wanhan He --- drivers/hwmon/acpi_power_meter.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c index 9c287c1bc98e..687141f088bc 100644 --- a/drivers/hwmon/acpi_power_meter.c +++ b/drivers/hwmon/acpi_power_meter.c @@ -437,9 +437,13 @@ static ssize_t show_val(struct device *dev, ret = update_cap(resource); if (ret) return ret; + resource->power_alarm = resource->power > resource->cap; + val = resource->power_alarm; + } else { + val = resource->power_alarm || + resource->power > resource->cap; + resource->power_alarm = resource->power > resource->cap; } - val = resource->power_alarm || resource->power > resource->cap; - resource->power_alarm = resource->power > resource->cap; break; case 7: case 8: -- Gitee From 35aa3f6ea765188ed684a7b1d7e5e8c73c6e223e Mon Sep 17 00:00:00 2001 From: Huisong Li Date: Wed, 19 Mar 2025 10:06:38 +0800 Subject: [PATCH 10/11] hwmon: (acpi_power_meter) Replace the deprecated hwmon_device_register ANBZ: #28799 commit 16746ce8adfe04f9ff8df75c1133286ba93c0e17 upstream When load this mode, we can see the following log: "power_meter ACPI000D:00: hwmon_device_register() is deprecated. Please convert the driver to use hwmon_device_register_with_info()." So replace hwmon_device_register with hwmon_device_register_with_info. These attributes, 'power_accuracy', 'power_cap_hyst', 'power_average_min' and 'power_average_max', should have been placed in hwmon_chip_info as power data type. But these attributes are displayed as string format on the following case: a) power1_accuracy --> display like '90.0%' b) power1_cap_hyst --> display 'unknown' when its value is 0xFFFFFFFF c) power1_average_min/max --> display 'unknown' when its value is negative. To avoid any changes in the display of these sysfs interfaces, we can't modifiy the type of these attributes in hwmon core and have to put them to extra_groups. Please note that the path of these sysfs interfaces are modified accordingly if use hwmon_device_register_with_info(): old: all sysfs interfaces are under acpi device, namely, /sys/class/hwmon/hwmonX/device/ now: all sysfs interfaces are under hwmon device, namely, /sys/class/hwmon/hwmonX/ The new ABI does not guarantee that the underlying path remains the same. But we have to accept this change so as to replace the deprecated API. Fortunately, some userspace application, like libsensors, would scan the two path and handles this automatically. So we can accept this change so as to drop the deprecated message. Signed-off-by: Huisong Li Link: https://lore.kernel.org/r/20250319020638.59925-1-lihuisong@huawei.com [groeck: Fixed some multi-line alignment issues; reverted to 32-bit arithmetic in power1_accuracy_show() fixed bad return code from power_meter_is_visible()] Signed-off-by: Guenter Roeck Signed-off-by: Wanhan He --- drivers/hwmon/acpi_power_meter.c | 858 +++++++++++++++---------------- 1 file changed, 424 insertions(+), 434 deletions(-) diff --git a/drivers/hwmon/acpi_power_meter.c b/drivers/hwmon/acpi_power_meter.c index 687141f088bc..743cd6130d95 100644 --- a/drivers/hwmon/acpi_power_meter.c +++ b/drivers/hwmon/acpi_power_meter.c @@ -87,25 +87,14 @@ struct acpi_power_meter_resource { bool power_alarm; int sensors_valid; unsigned long sensors_last_updated; - struct sensor_device_attribute sensors[NUM_SENSORS]; - int num_sensors; +#define POWER_METER_TRIP_AVERAGE_MIN_IDX 0 +#define POWER_METER_TRIP_AVERAGE_MAX_IDX 1 s64 trip[2]; int num_domain_devices; struct acpi_device **domain_devices; struct kobject *holders_dir; }; -struct sensor_template { - char *label; - ssize_t (*show)(struct device *dev, - struct device_attribute *devattr, - char *buf); - ssize_t (*set)(struct device *dev, - struct device_attribute *devattr, - const char *buf, size_t count); - int index; -}; - /* Averaging interval */ static int update_avg_interval(struct acpi_power_meter_resource *resource) { @@ -124,62 +113,6 @@ static int update_avg_interval(struct acpi_power_meter_resource *resource) return 0; } -static ssize_t show_avg_interval(struct device *dev, - struct device_attribute *devattr, - char *buf) -{ - struct acpi_device *acpi_dev = to_acpi_device(dev); - struct acpi_power_meter_resource *resource = acpi_dev->driver_data; - - mutex_lock(&resource->lock); - update_avg_interval(resource); - mutex_unlock(&resource->lock); - - return sprintf(buf, "%llu\n", resource->avg_interval); -} - -static ssize_t set_avg_interval(struct device *dev, - struct device_attribute *devattr, - const char *buf, size_t count) -{ - struct acpi_device *acpi_dev = to_acpi_device(dev); - struct acpi_power_meter_resource *resource = acpi_dev->driver_data; - union acpi_object arg0 = { ACPI_TYPE_INTEGER }; - struct acpi_object_list args = { 1, &arg0 }; - int res; - unsigned long temp; - unsigned long long data; - acpi_status status; - - res = kstrtoul(buf, 10, &temp); - if (res) - return res; - - if (temp > resource->caps.max_avg_interval || - temp < resource->caps.min_avg_interval) - return -EINVAL; - arg0.integer.value = temp; - - mutex_lock(&resource->lock); - status = acpi_evaluate_integer(resource->acpi_dev->handle, "_PAI", - &args, &data); - if (ACPI_SUCCESS(status)) - resource->avg_interval = temp; - mutex_unlock(&resource->lock); - - if (ACPI_FAILURE(status)) { - acpi_evaluation_failure_warn(resource->acpi_dev->handle, "_PAI", - status); - return -EINVAL; - } - - /* _PAI returns 0 on success, nonzero otherwise */ - if (data) - return -EINVAL; - - return count; -} - /* Cap functions */ static int update_cap(struct acpi_power_meter_resource *resource) { @@ -198,61 +131,6 @@ static int update_cap(struct acpi_power_meter_resource *resource) return 0; } -static ssize_t show_cap(struct device *dev, - struct device_attribute *devattr, - char *buf) -{ - struct acpi_device *acpi_dev = to_acpi_device(dev); - struct acpi_power_meter_resource *resource = acpi_dev->driver_data; - - mutex_lock(&resource->lock); - update_cap(resource); - mutex_unlock(&resource->lock); - - return sprintf(buf, "%llu\n", resource->cap * 1000); -} - -static ssize_t set_cap(struct device *dev, struct device_attribute *devattr, - const char *buf, size_t count) -{ - struct acpi_device *acpi_dev = to_acpi_device(dev); - struct acpi_power_meter_resource *resource = acpi_dev->driver_data; - union acpi_object arg0 = { ACPI_TYPE_INTEGER }; - struct acpi_object_list args = { 1, &arg0 }; - int res; - unsigned long temp; - unsigned long long data; - acpi_status status; - - res = kstrtoul(buf, 10, &temp); - if (res) - return res; - - temp = DIV_ROUND_CLOSEST(temp, 1000); - if (temp > resource->caps.max_cap || temp < resource->caps.min_cap) - return -EINVAL; - arg0.integer.value = temp; - - mutex_lock(&resource->lock); - status = acpi_evaluate_integer(resource->acpi_dev->handle, "_SHL", - &args, &data); - if (ACPI_SUCCESS(status)) - resource->cap = temp; - mutex_unlock(&resource->lock); - - if (ACPI_FAILURE(status)) { - acpi_evaluation_failure_warn(resource->acpi_dev->handle, "_SHL", - status); - return -EINVAL; - } - - /* _SHL returns 0 on success, nonzero otherwise */ - if (data) - return -EINVAL; - - return count; -} - /* Power meter trip points */ static int set_acpi_trip(struct acpi_power_meter_resource *resource) { @@ -287,34 +165,6 @@ static int set_acpi_trip(struct acpi_power_meter_resource *resource) return 0; } -static ssize_t set_trip(struct device *dev, struct device_attribute *devattr, - const char *buf, size_t count) -{ - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); - struct acpi_device *acpi_dev = to_acpi_device(dev); - struct acpi_power_meter_resource *resource = acpi_dev->driver_data; - unsigned long temp, trip_bk; - int res; - - res = kstrtoul(buf, 10, &temp); - if (res) - return res; - - temp = DIV_ROUND_CLOSEST(temp, 1000); - - guard(mutex)(&resource->lock); - - trip_bk = resource->trip[attr->index - 7]; - resource->trip[attr->index - 7] = temp; - res = set_acpi_trip(resource); - if (res) { - resource->trip[attr->index - 7] = trip_bk; - return res; - } - - return count; -} - /* Power meter */ static int update_meter(struct acpi_power_meter_resource *resource) { @@ -341,206 +191,6 @@ static int update_meter(struct acpi_power_meter_resource *resource) return 0; } -static ssize_t show_power(struct device *dev, - struct device_attribute *devattr, - char *buf) -{ - struct acpi_device *acpi_dev = to_acpi_device(dev); - struct acpi_power_meter_resource *resource = acpi_dev->driver_data; - - mutex_lock(&resource->lock); - update_meter(resource); - mutex_unlock(&resource->lock); - - if (resource->power == UNKNOWN_POWER) - return -ENODATA; - - return sprintf(buf, "%llu\n", resource->power * 1000); -} - -/* Miscellaneous */ -static ssize_t show_str(struct device *dev, - struct device_attribute *devattr, - char *buf) -{ - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); - struct acpi_device *acpi_dev = to_acpi_device(dev); - struct acpi_power_meter_resource *resource = acpi_dev->driver_data; - acpi_string val; - int ret; - - mutex_lock(&resource->lock); - switch (attr->index) { - case 0: - val = resource->model_number; - break; - case 1: - val = resource->serial_number; - break; - case 2: - val = resource->oem_info; - break; - default: - WARN(1, "Implementation error: unexpected attribute index %d\n", - attr->index); - val = ""; - break; - } - ret = sprintf(buf, "%s\n", val); - mutex_unlock(&resource->lock); - return ret; -} - -static ssize_t show_val(struct device *dev, - struct device_attribute *devattr, - char *buf) -{ - struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); - struct acpi_device *acpi_dev = to_acpi_device(dev); - struct acpi_power_meter_resource *resource = acpi_dev->driver_data; - u64 val = 0; - int ret; - - guard(mutex)(&resource->lock); - - switch (attr->index) { - case 0: - val = resource->caps.min_avg_interval; - break; - case 1: - val = resource->caps.max_avg_interval; - break; - case 2: - val = resource->caps.min_cap * 1000; - break; - case 3: - val = resource->caps.max_cap * 1000; - break; - case 4: - if (resource->caps.hysteresis == UNKNOWN_HYSTERESIS) - return sprintf(buf, "unknown\n"); - - val = resource->caps.hysteresis * 1000; - break; - case 5: - if (resource->caps.flags & POWER_METER_IS_BATTERY) - val = 1; - else - val = 0; - break; - case 6: - ret = update_meter(resource); - if (ret) - return ret; - /* need to update cap if not to support the notification. */ - if (!(resource->caps.flags & POWER_METER_CAN_NOTIFY)) { - ret = update_cap(resource); - if (ret) - return ret; - resource->power_alarm = resource->power > resource->cap; - val = resource->power_alarm; - } else { - val = resource->power_alarm || - resource->power > resource->cap; - resource->power_alarm = resource->power > resource->cap; - } - break; - case 7: - case 8: - if (resource->trip[attr->index - 7] < 0) - return sprintf(buf, "unknown\n"); - - val = resource->trip[attr->index - 7] * 1000; - break; - default: - WARN(1, "Implementation error: unexpected attribute index %d\n", - attr->index); - break; - } - - return sprintf(buf, "%llu\n", val); -} - -static ssize_t show_accuracy(struct device *dev, - struct device_attribute *devattr, - char *buf) -{ - struct acpi_device *acpi_dev = to_acpi_device(dev); - struct acpi_power_meter_resource *resource = acpi_dev->driver_data; - unsigned int acc = resource->caps.accuracy; - - return sprintf(buf, "%u.%u%%\n", acc / 1000, acc % 1000); -} - -static ssize_t show_name(struct device *dev, - struct device_attribute *devattr, - char *buf) -{ - return sprintf(buf, "%s\n", ACPI_POWER_METER_NAME); -} - -#define RO_SENSOR_TEMPLATE(_label, _show, _index) \ - { \ - .label = _label, \ - .show = _show, \ - .index = _index, \ - } - -#define RW_SENSOR_TEMPLATE(_label, _show, _set, _index) \ - { \ - .label = _label, \ - .show = _show, \ - .set = _set, \ - .index = _index, \ - } - -/* Sensor descriptions. If you add a sensor, update NUM_SENSORS above! */ -static struct sensor_template meter_attrs[] = { - RO_SENSOR_TEMPLATE(POWER_AVERAGE_NAME, show_power, 0), - RO_SENSOR_TEMPLATE("power1_accuracy", show_accuracy, 0), - RO_SENSOR_TEMPLATE("power1_average_interval_min", show_val, 0), - RO_SENSOR_TEMPLATE("power1_average_interval_max", show_val, 1), - RO_SENSOR_TEMPLATE("power1_is_battery", show_val, 5), - RW_SENSOR_TEMPLATE(POWER_AVG_INTERVAL_NAME, show_avg_interval, - set_avg_interval, 0), - {}, -}; - -static struct sensor_template misc_cap_attrs[] = { - RO_SENSOR_TEMPLATE("power1_cap_min", show_val, 2), - RO_SENSOR_TEMPLATE("power1_cap_max", show_val, 3), - RO_SENSOR_TEMPLATE("power1_cap_hyst", show_val, 4), - RO_SENSOR_TEMPLATE(POWER_ALARM_NAME, show_val, 6), - {}, -}; - -static struct sensor_template ro_cap_attrs[] = { - RO_SENSOR_TEMPLATE(POWER_CAP_NAME, show_cap, 0), - {}, -}; - -static struct sensor_template rw_cap_attrs[] = { - RW_SENSOR_TEMPLATE(POWER_CAP_NAME, show_cap, set_cap, 0), - {}, -}; - -static struct sensor_template trip_attrs[] = { - RW_SENSOR_TEMPLATE("power1_average_min", show_val, set_trip, 7), - RW_SENSOR_TEMPLATE("power1_average_max", show_val, set_trip, 8), - {}, -}; - -static struct sensor_template misc_attrs[] = { - RO_SENSOR_TEMPLATE("name", show_name, 0), - RO_SENSOR_TEMPLATE("power1_model_number", show_str, 0), - RO_SENSOR_TEMPLATE("power1_oem_info", show_str, 2), - RO_SENSOR_TEMPLATE("power1_serial_number", show_str, 1), - {}, -}; - -#undef RO_SENSOR_TEMPLATE -#undef RW_SENSOR_TEMPLATE - /* Read power domain data */ static void remove_domain_devices(struct acpi_power_meter_resource *resource) { @@ -642,109 +292,434 @@ static int read_domain_devices(struct acpi_power_meter_resource *resource) return res; } -/* Registration and deregistration */ -static int register_attrs(struct acpi_power_meter_resource *resource, - struct sensor_template *attrs) +static int set_trip(struct acpi_power_meter_resource *resource, u16 trip_idx, + unsigned long trip) { - struct device *dev = &resource->acpi_dev->dev; - struct sensor_device_attribute *sensors = - &resource->sensors[resource->num_sensors]; - int res = 0; + unsigned long trip_bk; + int ret; - while (attrs->label) { - sensors->dev_attr.attr.name = attrs->label; - sensors->dev_attr.attr.mode = 0444; - sensors->dev_attr.show = attrs->show; - sensors->index = attrs->index; + trip = DIV_ROUND_CLOSEST(trip, 1000); + trip_bk = resource->trip[trip_idx]; - if (attrs->set) { - sensors->dev_attr.attr.mode |= 0200; - sensors->dev_attr.store = attrs->set; - } + resource->trip[trip_idx] = trip; + ret = set_acpi_trip(resource); + if (ret) { + dev_err(&resource->acpi_dev->dev, "set %s failed.\n", + (trip_idx == POWER_METER_TRIP_AVERAGE_MIN_IDX) ? + "power1_average_min" : "power1_average_max"); + resource->trip[trip_idx] = trip_bk; + } - sysfs_attr_init(&sensors->dev_attr.attr); - res = device_create_file(dev, &sensors->dev_attr); - if (res) { - sensors->dev_attr.attr.name = NULL; - goto error; - } - sensors++; - resource->num_sensors++; - attrs++; + return ret; +} + +static int set_cap(struct acpi_power_meter_resource *resource, + unsigned long cap) +{ + union acpi_object arg0 = { ACPI_TYPE_INTEGER }; + struct acpi_object_list args = { 1, &arg0 }; + unsigned long long data; + acpi_status status; + + cap = DIV_ROUND_CLOSEST(cap, 1000); + if (cap > resource->caps.max_cap || cap < resource->caps.min_cap) + return -EINVAL; + + arg0.integer.value = cap; + status = acpi_evaluate_integer(resource->acpi_dev->handle, "_SHL", + &args, &data); + if (ACPI_FAILURE(status)) { + acpi_evaluation_failure_warn(resource->acpi_dev->handle, "_SHL", + status); + return -EINVAL; } + resource->cap = cap; -error: - return res; + /* _SHL returns 0 on success, nonzero otherwise */ + if (data) + return -EINVAL; + + return 0; } -static void remove_attrs(struct acpi_power_meter_resource *resource) +static int set_avg_interval(struct acpi_power_meter_resource *resource, + unsigned long val) { - int i; + union acpi_object arg0 = { ACPI_TYPE_INTEGER }; + struct acpi_object_list args = { 1, &arg0 }; + unsigned long long data; + acpi_status status; - for (i = 0; i < resource->num_sensors; i++) { - if (!resource->sensors[i].dev_attr.attr.name) - continue; - device_remove_file(&resource->acpi_dev->dev, - &resource->sensors[i].dev_attr); + if (val > resource->caps.max_avg_interval || + val < resource->caps.min_avg_interval) + return -EINVAL; + + arg0.integer.value = val; + status = acpi_evaluate_integer(resource->acpi_dev->handle, "_PAI", + &args, &data); + if (ACPI_FAILURE(status)) { + acpi_evaluation_failure_warn(resource->acpi_dev->handle, "_PAI", + status); + return -EINVAL; } + resource->avg_interval = val; - remove_domain_devices(resource); + /* _PAI returns 0 on success, nonzero otherwise */ + if (data) + return -EINVAL; - resource->num_sensors = 0; + return 0; } -static int setup_attrs(struct acpi_power_meter_resource *resource) +static int get_power_alarm_state(struct acpi_power_meter_resource *resource, + long *val) { - int res = 0; + int ret; - /* _PMD method is optional. */ - res = read_domain_devices(resource); - if (res && res != -ENODEV) - return res; + ret = update_meter(resource); + if (ret) + return ret; - if (resource->caps.flags & POWER_METER_CAN_MEASURE) { - res = register_attrs(resource, meter_attrs); - if (res) - goto error; + /* need to update cap if not to support the notification. */ + if (!(resource->caps.flags & POWER_METER_CAN_NOTIFY)) { + ret = update_cap(resource); + if (ret) + return ret; + resource->power_alarm = resource->power > resource->cap; + *val = resource->power_alarm; + } else { + *val = resource->power_alarm || resource->power > resource->cap; + resource->power_alarm = resource->power > resource->cap; } - if (resource->caps.flags & POWER_METER_CAN_CAP) { - if (!can_cap_in_hardware()) { - dev_warn(&resource->acpi_dev->dev, - "Ignoring unsafe software power cap!\n"); - goto skip_unsafe_cap; + return 0; +} + +static umode_t power_meter_is_visible(const void *data, + enum hwmon_sensor_types type, + u32 attr, int channel) +{ + const struct acpi_power_meter_resource *res = data; + + if (type != hwmon_power) + return 0; + + switch (attr) { + case hwmon_power_average: + case hwmon_power_average_interval_min: + case hwmon_power_average_interval_max: + if (res->caps.flags & POWER_METER_CAN_MEASURE) + return 0444; + break; + case hwmon_power_average_interval: + if (res->caps.flags & POWER_METER_CAN_MEASURE) + return 0644; + break; + case hwmon_power_cap_min: + case hwmon_power_cap_max: + case hwmon_power_alarm: + if (res->caps.flags & POWER_METER_CAN_CAP && can_cap_in_hardware()) + return 0444; + break; + case hwmon_power_cap: + if (res->caps.flags & POWER_METER_CAN_CAP && can_cap_in_hardware()) { + if (res->caps.configurable_cap) + return 0644; + else + return 0444; } + break; + default: + break; + } + + return 0; +} - if (resource->caps.configurable_cap) - res = register_attrs(resource, rw_cap_attrs); - else - res = register_attrs(resource, ro_cap_attrs); +static int power_meter_read(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long *val) +{ + struct acpi_power_meter_resource *res = dev_get_drvdata(dev); + int ret = 0; - if (res) - goto error; + if (type != hwmon_power) + return -EINVAL; - res = register_attrs(resource, misc_cap_attrs); - if (res) - goto error; + guard(mutex)(&res->lock); + + switch (attr) { + case hwmon_power_average: + ret = update_meter(res); + if (ret) + return ret; + if (res->power == UNKNOWN_POWER) + return -ENODATA; + *val = res->power * 1000; + break; + case hwmon_power_average_interval_min: + *val = res->caps.min_avg_interval; + break; + case hwmon_power_average_interval_max: + *val = res->caps.max_avg_interval; + break; + case hwmon_power_average_interval: + ret = update_avg_interval(res); + if (ret) + return ret; + *val = (res)->avg_interval; + break; + case hwmon_power_cap_min: + *val = res->caps.min_cap * 1000; + break; + case hwmon_power_cap_max: + *val = res->caps.max_cap * 1000; + break; + case hwmon_power_alarm: + ret = get_power_alarm_state(res, val); + if (ret) + return ret; + break; + case hwmon_power_cap: + ret = update_cap(res); + if (ret) + return ret; + *val = res->cap * 1000; + break; + default: + break; } -skip_unsafe_cap: - if (resource->caps.flags & POWER_METER_CAN_TRIP) { - res = register_attrs(resource, trip_attrs); - if (res) - goto error; + return 0; +} + +static int power_meter_write(struct device *dev, enum hwmon_sensor_types type, + u32 attr, int channel, long val) +{ + struct acpi_power_meter_resource *res = dev_get_drvdata(dev); + int ret; + + if (type != hwmon_power) + return -EINVAL; + + guard(mutex)(&res->lock); + switch (attr) { + case hwmon_power_cap: + ret = set_cap(res, val); + break; + case hwmon_power_average_interval: + ret = set_avg_interval(res, val); + break; + default: + ret = -EOPNOTSUPP; } - res = register_attrs(resource, misc_attrs); - if (res) - goto error; + return ret; +} - return res; -error: - remove_attrs(resource); - return res; +static const struct hwmon_channel_info * const power_meter_info[] = { + HWMON_CHANNEL_INFO(power, HWMON_P_AVERAGE | + HWMON_P_AVERAGE_INTERVAL | HWMON_P_AVERAGE_INTERVAL_MIN | + HWMON_P_AVERAGE_INTERVAL_MAX | HWMON_P_CAP | HWMON_P_CAP_MIN | + HWMON_P_CAP_MAX | HWMON_P_ALARM), + NULL +}; + +static const struct hwmon_ops power_meter_ops = { + .is_visible = power_meter_is_visible, + .read = power_meter_read, + .write = power_meter_write, +}; + +static const struct hwmon_chip_info power_meter_chip_info = { + .ops = &power_meter_ops, + .info = power_meter_info, +}; + +static ssize_t power1_average_max_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct acpi_power_meter_resource *res = dev_get_drvdata(dev); + unsigned long trip; + int ret; + + ret = kstrtoul(buf, 10, &trip); + if (ret) + return ret; + + mutex_lock(&res->lock); + ret = set_trip(res, POWER_METER_TRIP_AVERAGE_MAX_IDX, trip); + mutex_unlock(&res->lock); + + return ret == 0 ? count : ret; +} + +static ssize_t power1_average_min_store(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + struct acpi_power_meter_resource *res = dev_get_drvdata(dev); + unsigned long trip; + int ret; + + ret = kstrtoul(buf, 10, &trip); + if (ret) + return ret; + + mutex_lock(&res->lock); + ret = set_trip(res, POWER_METER_TRIP_AVERAGE_MIN_IDX, trip); + mutex_unlock(&res->lock); + + return ret == 0 ? count : ret; +} + +static ssize_t power1_average_min_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct acpi_power_meter_resource *res = dev_get_drvdata(dev); + + if (res->trip[POWER_METER_TRIP_AVERAGE_MIN_IDX] < 0) + return sysfs_emit(buf, "unknown\n"); + + return sysfs_emit(buf, "%lld\n", + res->trip[POWER_METER_TRIP_AVERAGE_MIN_IDX] * 1000); +} + +static ssize_t power1_average_max_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct acpi_power_meter_resource *res = dev_get_drvdata(dev); + + if (res->trip[POWER_METER_TRIP_AVERAGE_MAX_IDX] < 0) + return sysfs_emit(buf, "unknown\n"); + + return sysfs_emit(buf, "%lld\n", + res->trip[POWER_METER_TRIP_AVERAGE_MAX_IDX] * 1000); +} + +static ssize_t power1_cap_hyst_show(struct device *dev, + struct device_attribute *attr, char *buf) +{ + struct acpi_power_meter_resource *res = dev_get_drvdata(dev); + + if (res->caps.hysteresis == UNKNOWN_HYSTERESIS) + return sysfs_emit(buf, "unknown\n"); + + return sysfs_emit(buf, "%llu\n", res->caps.hysteresis * 1000); +} + +static ssize_t power1_accuracy_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct acpi_power_meter_resource *res = dev_get_drvdata(dev); + unsigned int acc = res->caps.accuracy; + + return sysfs_emit(buf, "%u.%u%%\n", acc / 1000, acc % 1000); +} + +static ssize_t power1_is_battery_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct acpi_power_meter_resource *res = dev_get_drvdata(dev); + + return sysfs_emit(buf, "%u\n", + res->caps.flags & POWER_METER_IS_BATTERY ? 1 : 0); +} + +static ssize_t power1_model_number_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct acpi_power_meter_resource *res = dev_get_drvdata(dev); + + return sysfs_emit(buf, "%s\n", res->model_number); +} + +static ssize_t power1_oem_info_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct acpi_power_meter_resource *res = dev_get_drvdata(dev); + + return sysfs_emit(buf, "%s\n", res->oem_info); +} + +static ssize_t power1_serial_number_show(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct acpi_power_meter_resource *res = dev_get_drvdata(dev); + + return sysfs_emit(buf, "%s\n", res->serial_number); } +/* depend on POWER_METER_CAN_TRIP */ +static DEVICE_ATTR_RW(power1_average_max); +static DEVICE_ATTR_RW(power1_average_min); + +/* depend on POWER_METER_CAN_CAP */ +static DEVICE_ATTR_RO(power1_cap_hyst); + +/* depend on POWER_METER_CAN_MEASURE */ +static DEVICE_ATTR_RO(power1_accuracy); +static DEVICE_ATTR_RO(power1_is_battery); + +static DEVICE_ATTR_RO(power1_model_number); +static DEVICE_ATTR_RO(power1_oem_info); +static DEVICE_ATTR_RO(power1_serial_number); + +static umode_t power_extra_is_visible(struct kobject *kobj, + struct attribute *attr, int idx) +{ + struct device *dev = kobj_to_dev(kobj); + struct acpi_power_meter_resource *res = dev_get_drvdata(dev); + + if (attr == &dev_attr_power1_is_battery.attr || + attr == &dev_attr_power1_accuracy.attr) { + if ((res->caps.flags & POWER_METER_CAN_MEASURE) == 0) + return 0; + } + + if (attr == &dev_attr_power1_cap_hyst.attr) { + if ((res->caps.flags & POWER_METER_CAN_CAP) == 0) { + return 0; + } else if (!can_cap_in_hardware()) { + dev_warn(&res->acpi_dev->dev, + "Ignoring unsafe software power cap!\n"); + return 0; + } + } + + if (attr == &dev_attr_power1_average_max.attr || + attr == &dev_attr_power1_average_min.attr) { + if ((res->caps.flags & POWER_METER_CAN_TRIP) == 0) + return 0; + } + + return attr->mode; +} + +static struct attribute *power_extra_attrs[] = { + &dev_attr_power1_average_max.attr, + &dev_attr_power1_average_min.attr, + &dev_attr_power1_cap_hyst.attr, + &dev_attr_power1_accuracy.attr, + &dev_attr_power1_is_battery.attr, + &dev_attr_power1_model_number.attr, + &dev_attr_power1_oem_info.attr, + &dev_attr_power1_serial_number.attr, + NULL +}; + +static const struct attribute_group power_extra_group = { + .attrs = power_extra_attrs, + .is_visible = power_extra_is_visible, +}; + +__ATTRIBUTE_GROUPS(power_extra); + static void free_capabilities(struct acpi_power_meter_resource *resource) { acpi_string *str; @@ -853,13 +828,23 @@ static void acpi_power_meter_notify(struct acpi_device *device, u32 event) case METER_NOTIFY_CONFIG: mutex_lock(&resource->lock); free_capabilities(resource); + remove_domain_devices(resource); + hwmon_device_unregister(resource->hwmon_dev); res = read_capabilities(resource); - mutex_unlock(&resource->lock); if (res) - break; - - remove_attrs(resource); - setup_attrs(resource); + dev_err_once(&device->dev, "read capabilities failed.\n"); + res = read_domain_devices(resource); + if (res && res != -ENODEV) + dev_err_once(&device->dev, "read domain devices failed.\n"); + resource->hwmon_dev = + hwmon_device_register_with_info(&device->dev, + ACPI_POWER_METER_NAME, + resource, + &power_meter_chip_info, + power_extra_groups); + if (IS_ERR(resource->hwmon_dev)) + dev_err_once(&device->dev, "register hwmon device failed.\n"); + mutex_unlock(&resource->lock); break; case METER_NOTIFY_TRIP: sysfs_notify(&device->dev.kobj, NULL, POWER_AVERAGE_NAME); @@ -933,11 +918,16 @@ static int acpi_power_meter_add(struct acpi_device *device) resource->trip[0] = -1; resource->trip[1] = -1; - res = setup_attrs(resource); - if (res) + /* _PMD method is optional. */ + res = read_domain_devices(resource); + if (res && res != -ENODEV) goto exit_free_capability; - resource->hwmon_dev = hwmon_device_register(&device->dev); + resource->hwmon_dev = + hwmon_device_register_with_info(&device->dev, + ACPI_POWER_METER_NAME, resource, + &power_meter_chip_info, + power_extra_groups); if (IS_ERR(resource->hwmon_dev)) { res = PTR_ERR(resource->hwmon_dev); goto exit_remove; @@ -947,7 +937,7 @@ static int acpi_power_meter_add(struct acpi_device *device) goto exit; exit_remove: - remove_attrs(resource); + remove_domain_devices(resource); exit_free_capability: free_capabilities(resource); exit_free: @@ -966,7 +956,7 @@ static void acpi_power_meter_remove(struct acpi_device *device) resource = acpi_driver_data(device); hwmon_device_unregister(resource->hwmon_dev); - remove_attrs(resource); + remove_domain_devices(resource); free_capabilities(resource); kfree(resource); -- Gitee From 259ab23fb5badd814f9cc19354a343a88b7416da Mon Sep 17 00:00:00 2001 From: Huisong Li Date: Tue, 4 Mar 2025 15:46:40 +0800 Subject: [PATCH 11/11] hwmon: Fix the missing of 'average' word in hwmon_power_attr_templates ANBZ: #28799 commit 232427772fc123942d110c18269cfbdf40edae18 upstream The string "power%d_interval_max" and "power%d_interval_min" in the hwmon_power_attr_templates[] are corresponding to the sysfs interface name of hwmon_power_average_interval_max and hwmon_power_average_interval_min. But the 'average' word is missing in two strings. Fortunately, there is no driver to use it yet. Signed-off-by: Huisong Li Link: https://lore.kernel.org/r/20250304074640.2770353-1-lihuisong@huawei.com Signed-off-by: Guenter Roeck Signed-off-by: Wanhan He --- drivers/hwmon/hwmon.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c index c7dd3f5b2bd5..a35690a2d8a7 100644 --- a/drivers/hwmon/hwmon.c +++ b/drivers/hwmon/hwmon.c @@ -538,8 +538,8 @@ static const char * const hwmon_power_attr_templates[] = { [hwmon_power_enable] = "power%d_enable", [hwmon_power_average] = "power%d_average", [hwmon_power_average_interval] = "power%d_average_interval", - [hwmon_power_average_interval_max] = "power%d_interval_max", - [hwmon_power_average_interval_min] = "power%d_interval_min", + [hwmon_power_average_interval_max] = "power%d_average_interval_max", + [hwmon_power_average_interval_min] = "power%d_average_interval_min", [hwmon_power_average_highest] = "power%d_average_highest", [hwmon_power_average_lowest] = "power%d_average_lowest", [hwmon_power_average_max] = "power%d_average_max", -- Gitee