platform-driver-x86.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/2] Add sysfs files to plat device based driver
@ 2025-08-06  6:38 Suma Hegde
  2025-08-06  6:38 ` [PATCH 1/2] platform/x86/amd/hsmp: Rename the HSMP_BIN_ATTR_GRP macro Suma Hegde
  2025-08-06  6:38 ` [PATCH 2/2] platform/x86/amd/hsmp: plat: Add sysfs files to display HSMP telemetry Suma Hegde
  0 siblings, 2 replies; 5+ messages in thread
From: Suma Hegde @ 2025-08-06  6:38 UTC (permalink / raw)
  To: platform-driver-x86; +Cc: ilpo.jarvinen, hdegoede, Suma Hegde

This series has two patches.

* The first one renames the macro to make it a generic name. This is
  in preparation to the second patch.

* The second patch adds sysfs files to the platform device based driver.
  Basic system telemetry is reported through sysfs files similar to ACPI
  based driver in 511a4a5ea2b6f1d4e0c719f27db6b627b2b52e49.

This series is rebased on review-ilpo-next + below two patches
https://lore.kernel.org/platform-driver-x86/20250804101551.89866-1-suma.hegde@amd.com/T/#u
https://lore.kernel.org/platform-driver-x86/20250805091911.483825-1-suma.hegde@amd.com/T/#u

Suma Hegde (2):
  platform/x86/amd/hsmp: Rename the HSMP_BIN_ATTR_GRP macro
  platform/x86/amd/hsmp: plat: Add sysfs files to display HSMP telemetry

 Documentation/arch/x86/amd_hsmp.rst   |   4 +-
 drivers/platform/x86/amd/hsmp/acpi.c  |  33 +--
 drivers/platform/x86/amd/hsmp/plat.c  | 388 +++++++++++++++++++++++++-
 drivers/platform/x86/amd/hsmp/sysfs.h |  48 ++++
 4 files changed, 433 insertions(+), 40 deletions(-)
 create mode 100644 drivers/platform/x86/amd/hsmp/sysfs.h

-- 
2.25.1


^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] platform/x86/amd/hsmp: Rename the HSMP_BIN_ATTR_GRP macro
  2025-08-06  6:38 [PATCH 0/2] Add sysfs files to plat device based driver Suma Hegde
@ 2025-08-06  6:38 ` Suma Hegde
  2025-08-06  6:38 ` [PATCH 2/2] platform/x86/amd/hsmp: plat: Add sysfs files to display HSMP telemetry Suma Hegde
  1 sibling, 0 replies; 5+ messages in thread
From: Suma Hegde @ 2025-08-06  6:38 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: ilpo.jarvinen, hdegoede, Suma Hegde, Naveen Krishna Chatradhi

Rename HSMP_BIN_ATTR_GRP to HSMP_ATTR_GRP as its not limited to binary
attributes.

An upcoming patch will also add dev attribute list along with binary
attributes.

Signed-off-by: Suma Hegde <suma.hegde@amd.com>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
---
 drivers/platform/x86/amd/hsmp/plat.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
index f8aa844d33e4..10e8f98ea12c 100644
--- a/drivers/platform/x86/amd/hsmp/plat.c
+++ b/drivers/platform/x86/amd/hsmp/plat.c
@@ -110,21 +110,21 @@ HSMP_BIN_ATTR(5, *sock5_attr_list);
 HSMP_BIN_ATTR(6, *sock6_attr_list);
 HSMP_BIN_ATTR(7, *sock7_attr_list);
 
-#define HSMP_BIN_ATTR_GRP(index, _list, _name)			\
+#define HSMP_ATTR_GRP(index, _list, _name)			\
 static const struct attribute_group sock##index##_attr_grp = {	\
 	.bin_attrs_new = _list,					\
 	.is_bin_visible = hsmp_is_sock_attr_visible,		\
 	.name = #_name,						\
 }
 
-HSMP_BIN_ATTR_GRP(0, sock0_attr_list, socket0);
-HSMP_BIN_ATTR_GRP(1, sock1_attr_list, socket1);
-HSMP_BIN_ATTR_GRP(2, sock2_attr_list, socket2);
-HSMP_BIN_ATTR_GRP(3, sock3_attr_list, socket3);
-HSMP_BIN_ATTR_GRP(4, sock4_attr_list, socket4);
-HSMP_BIN_ATTR_GRP(5, sock5_attr_list, socket5);
-HSMP_BIN_ATTR_GRP(6, sock6_attr_list, socket6);
-HSMP_BIN_ATTR_GRP(7, sock7_attr_list, socket7);
+HSMP_ATTR_GRP(0, sock0_attr_list, socket0);
+HSMP_ATTR_GRP(1, sock1_attr_list, socket1);
+HSMP_ATTR_GRP(2, sock2_attr_list, socket2);
+HSMP_ATTR_GRP(3, sock3_attr_list, socket3);
+HSMP_ATTR_GRP(4, sock4_attr_list, socket4);
+HSMP_ATTR_GRP(5, sock5_attr_list, socket5);
+HSMP_ATTR_GRP(6, sock6_attr_list, socket6);
+HSMP_ATTR_GRP(7, sock7_attr_list, socket7);
 
 static const struct attribute_group *hsmp_groups[] = {
 	&sock0_attr_grp,
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] platform/x86/amd/hsmp: plat: Add sysfs files to display HSMP telemetry
  2025-08-06  6:38 [PATCH 0/2] Add sysfs files to plat device based driver Suma Hegde
  2025-08-06  6:38 ` [PATCH 1/2] platform/x86/amd/hsmp: Rename the HSMP_BIN_ATTR_GRP macro Suma Hegde
@ 2025-08-06  6:38 ` Suma Hegde
  2025-08-19 11:42   ` Ilpo Järvinen
  1 sibling, 1 reply; 5+ messages in thread
From: Suma Hegde @ 2025-08-06  6:38 UTC (permalink / raw)
  To: platform-driver-x86
  Cc: ilpo.jarvinen, hdegoede, Suma Hegde, Naveen Krishna Chatradhi

This patch adds sysfs files to platform device based driver.

Following sysfs files are added similar to those in the ACPI based driver.
* c0_residency_input
* prochot_status
* smu_fw_version
* protocol_version
* ddr_max_bw(GB/s)
* ddr_utilised_bw_input(GB/s)
* ddr_utilised_bw_perc_input(%)
* mclk_input(MHz)
* fclk_input(MHz)
* clk_fmax(MHz)
* clk_fmin(MHz)
* cclk_freq_limit_input(MHz)
* pwr_current_active_freq_limit(MHz)
* pwr_current_active_freq_limit_source

Signed-off-by: Suma Hegde <suma.hegde@amd.com>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
---
 Documentation/arch/x86/amd_hsmp.rst   |   4 +-
 drivers/platform/x86/amd/hsmp/acpi.c  |  33 +--
 drivers/platform/x86/amd/hsmp/plat.c  | 388 +++++++++++++++++++++++++-
 drivers/platform/x86/amd/hsmp/sysfs.h |  48 ++++
 4 files changed, 433 insertions(+), 40 deletions(-)
 create mode 100644 drivers/platform/x86/amd/hsmp/sysfs.h

diff --git a/Documentation/arch/x86/amd_hsmp.rst b/Documentation/arch/x86/amd_hsmp.rst
index a094f55c10b0..6dd9948d8c21 100644
--- a/Documentation/arch/x86/amd_hsmp.rst
+++ b/Documentation/arch/x86/amd_hsmp.rst
@@ -73,7 +73,9 @@ The same is defined in the amd_hsmp.h header.
 
 2. HSMP telemetry sysfs files
 
-Following sysfs files are available at /sys/devices/platform/AMDI0097:0X/.
+Following sysfs files are available at /sys/devices/platform/AMDI0097:0X/ for
+ACPI based driver and /sys/devices/platform/amd_hsmp/socketX/ for platform
+device based driver.
 
 * c0_residency_input: Percentage of cores in C0 state.
 * prochot_status: Reports 1 if the processor is at thermal threshold value,
diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index 19f0ca7958b6..f6434cf07f6b 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -13,12 +13,12 @@
 
 #include <linux/acpi.h>
 #include <linux/array_size.h>
-#include <linux/bits.h>
 #include <linux/bitfield.h>
 #include <linux/device.h>
 #include <linux/dev_printk.h>
 #include <linux/ioport.h>
 #include <linux/kstrtox.h>
+#include <linux/limits.h>
 #include <linux/module.h>
 #include <linux/platform_device.h>
 #include <linux/sysfs.h>
@@ -29,6 +29,7 @@
 #include <asm/amd/node.h>
 
 #include "hsmp.h"
+#include "sysfs.h"
 
 #define DRIVER_NAME		"hsmp_acpi"
 
@@ -39,11 +40,6 @@
 
 static struct hsmp_plat_device *hsmp_pdev;
 
-struct hsmp_sys_attr {
-	struct device_attribute dattr;
-	u32 msg_id;
-};
-
 static int amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
 			      u32 *value, bool write)
 {
@@ -257,8 +253,6 @@ static umode_t hsmp_is_sock_dev_attr_visible(struct kobject *kobj,
 	return attr->mode;
 }
 
-#define to_hsmp_sys_attr(_attr) container_of(_attr, struct hsmp_sys_attr, dattr)
-
 static ssize_t hsmp_msg_resp32_show(struct device *dev, struct device_attribute *attr,
 				    char *buf)
 {
@@ -274,17 +268,6 @@ static ssize_t hsmp_msg_resp32_show(struct device *dev, struct device_attribute
 	return sysfs_emit(buf, "%u\n", data);
 }
 
-#define DDR_MAX_BW_MASK		GENMASK(31, 20)
-#define DDR_UTIL_BW_MASK	GENMASK(19, 8)
-#define DDR_UTIL_BW_PERC_MASK	GENMASK(7, 0)
-#define FW_VER_MAJOR_MASK	GENMASK(23, 16)
-#define FW_VER_MINOR_MASK	GENMASK(15, 8)
-#define FW_VER_DEBUG_MASK	GENMASK(7, 0)
-#define FMAX_MASK		GENMASK(31, 16)
-#define FMIN_MASK		GENMASK(15, 0)
-#define FREQ_LIMIT_MASK		GENMASK(31, 16)
-#define FREQ_SRC_IND_MASK	GENMASK(15, 0)
-
 static ssize_t hsmp_ddr_max_bw_show(struct device *dev, struct device_attribute *attr,
 				    char *buf)
 {
@@ -423,17 +406,6 @@ static ssize_t hsmp_freq_limit_show(struct device *dev, struct device_attribute
 	return sysfs_emit(buf, "%lu\n", FIELD_GET(FREQ_LIMIT_MASK, data));
 }
 
-static const char * const freqlimit_srcnames[] = {
-	"cHTC-Active",
-	"PROCHOT",
-	"TDC limit",
-	"PPT Limit",
-	"OPN Max",
-	"Reliability Limit",
-	"APML Agent",
-	"HSMP Agent",
-};
-
 static ssize_t hsmp_freq_limit_source_show(struct device *dev, struct device_attribute *attr,
 					   char *buf)
 {
@@ -521,6 +493,7 @@ static const struct bin_attribute *hsmp_attr_list[] = {
 #define HSMP_DEV_ATTR(_name, _msg_id, _show, _mode)	\
 static struct hsmp_sys_attr hattr_##_name = {		\
 	.dattr = __ATTR(_name, _mode, _show, NULL),	\
+	.sock_ind = U16_MAX,				\
 	.msg_id = _msg_id,				\
 }
 
diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
index 10e8f98ea12c..3a0171ee4a80 100644
--- a/drivers/platform/x86/amd/hsmp/plat.c
+++ b/drivers/platform/x86/amd/hsmp/plat.c
@@ -12,7 +12,9 @@
 #include <asm/amd/hsmp.h>
 
 #include <linux/acpi.h>
+#include <linux/bitfield.h>
 #include <linux/build_bug.h>
+#include <linux/container_of.h>
 #include <linux/device.h>
 #include <linux/dev_printk.h>
 #include <linux/kconfig.h>
@@ -24,6 +26,7 @@
 #include <asm/amd/node.h>
 
 #include "hsmp.h"
+#include "sysfs.h"
 
 #define DRIVER_NAME		"amd_hsmp"
 
@@ -78,6 +81,186 @@ static umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
 	return 0;
 }
 
+static umode_t hsmp_is_sock_dev_attr_visible(struct kobject *kobj,
+					     struct attribute *attr, int id)
+{
+	struct device_attribute *dattr = container_of(attr, struct device_attribute, attr);
+	struct hsmp_sys_attr *hattr = container_of(dattr, struct hsmp_sys_attr, dattr);
+
+	if (id == 0 && hattr->sock_ind >= hsmp_pdev->num_sockets)
+		return SYSFS_GROUP_INVISIBLE;
+
+	return attr->mode;
+}
+
+static ssize_t hsmp_msg_resp32_show(struct device *dev, struct device_attribute *attr,
+				    char *buf)
+{
+	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
+	u32 data;
+	int ret;
+
+	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%u\n", data);
+}
+
+static ssize_t hsmp_ddr_max_bw_show(struct device *dev, struct device_attribute *attr,
+				    char *buf)
+{
+	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
+	u32 data;
+	int ret;
+
+	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%lu\n", FIELD_GET(DDR_MAX_BW_MASK, data));
+}
+
+static ssize_t hsmp_ddr_util_bw_show(struct device *dev, struct device_attribute *attr,
+				     char *buf)
+{
+	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
+	u32 data;
+	int ret;
+
+	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%lu\n", FIELD_GET(DDR_UTIL_BW_MASK, data));
+}
+
+static ssize_t hsmp_ddr_util_bw_perc_show(struct device *dev, struct device_attribute *attr,
+					  char *buf)
+{
+	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
+	u32 data;
+	int ret;
+
+	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%lu\n", FIELD_GET(DDR_UTIL_BW_PERC_MASK, data));
+}
+
+static ssize_t hsmp_msg_fw_ver_show(struct device *dev, struct device_attribute *attr,
+				    char *buf)
+{
+	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
+	u32 data;
+	int ret;
+
+	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%lu.%lu.%lu\n",
+			  FIELD_GET(FW_VER_MAJOR_MASK, data),
+			  FIELD_GET(FW_VER_MINOR_MASK, data),
+			  FIELD_GET(FW_VER_DEBUG_MASK, data));
+}
+
+static ssize_t hsmp_fclk_show(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
+	u32 data[2];
+	int ret;
+
+	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, data, 2);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%u\n", data[0]);
+}
+
+static ssize_t hsmp_mclk_show(struct device *dev, struct device_attribute *attr,
+			      char *buf)
+{
+	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
+	u32 data[2];
+	int ret;
+
+	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, data, 2);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%u\n", data[1]);
+}
+
+static ssize_t hsmp_clk_fmax_show(struct device *dev, struct device_attribute *attr,
+				  char *buf)
+{
+	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
+	u32 data;
+	int ret;
+
+	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%lu\n", FIELD_GET(FMAX_MASK, data));
+}
+
+static ssize_t hsmp_clk_fmin_show(struct device *dev, struct device_attribute *attr,
+				  char *buf)
+{
+	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
+	u32 data;
+	int ret;
+
+	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%lu\n", FIELD_GET(FMIN_MASK, data));
+}
+
+static ssize_t hsmp_freq_limit_show(struct device *dev, struct device_attribute *attr,
+				    char *buf)
+{
+	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
+	u32 data;
+	int ret;
+
+	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
+	if (ret)
+		return ret;
+
+	return sysfs_emit(buf, "%lu\n", FIELD_GET(FREQ_LIMIT_MASK, data));
+}
+
+static ssize_t hsmp_freq_limit_source_show(struct device *dev, struct device_attribute *attr,
+					   char *buf)
+{
+	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
+	unsigned int index;
+	int len = 0;
+	u16 src_ind;
+	u32 data;
+	int ret;
+
+	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
+	if (ret)
+		return ret;
+
+	src_ind = FIELD_GET(FREQ_SRC_IND_MASK, data);
+	for (index = 0; index < ARRAY_SIZE(freqlimit_srcnames); index++) {
+		if (!src_ind)
+			break;
+		if (src_ind & 1)
+			len += sysfs_emit_at(buf, len, "%s\n", freqlimit_srcnames[index]);
+		src_ind >>= 1;
+	}
+	return len;
+}
+
 /*
  * AMD supports maximum of 8 sockets in a system.
  * Static array of 8 + 1(for NULL) elements is created below
@@ -110,21 +293,208 @@ HSMP_BIN_ATTR(5, *sock5_attr_list);
 HSMP_BIN_ATTR(6, *sock6_attr_list);
 HSMP_BIN_ATTR(7, *sock7_attr_list);
 
-#define HSMP_ATTR_GRP(index, _list, _name)			\
+#define HSMP_ATTR_GRP(index, _list, _name, _dlist)		\
 static const struct attribute_group sock##index##_attr_grp = {	\
 	.bin_attrs_new = _list,					\
+	.attrs = _dlist,					\
 	.is_bin_visible = hsmp_is_sock_attr_visible,		\
+	.is_visible = hsmp_is_sock_dev_attr_visible,		\
 	.name = #_name,						\
 }
 
-HSMP_ATTR_GRP(0, sock0_attr_list, socket0);
-HSMP_ATTR_GRP(1, sock1_attr_list, socket1);
-HSMP_ATTR_GRP(2, sock2_attr_list, socket2);
-HSMP_ATTR_GRP(3, sock3_attr_list, socket3);
-HSMP_ATTR_GRP(4, sock4_attr_list, socket4);
-HSMP_ATTR_GRP(5, sock5_attr_list, socket5);
-HSMP_ATTR_GRP(6, sock6_attr_list, socket6);
-HSMP_ATTR_GRP(7, sock7_attr_list, socket7);
+#define HSMP_DEV_ATTR(_name, _msg_id, _show, _mode, _sock_ind)	\
+static struct hsmp_sys_attr hattr_##_name##_sock_ind = {	\
+	.dattr = __ATTR(_name, _mode, _show, NULL),		\
+	.sock_ind = _sock_ind,					\
+	.msg_id = _msg_id,					\
+}
+
+HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 0);
+HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 1);
+HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 2);
+HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 3);
+HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 4);
+HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 5);
+HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 6);
+HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 7);
+
+HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 0);
+HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 1);
+HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 2);
+HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 3);
+HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 4);
+HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 5);
+HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 6);
+HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 7);
+
+HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 0);
+HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 1);
+HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 2);
+HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 3);
+HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 4);
+HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 5);
+HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 6);
+HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 7);
+
+HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 0);
+HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 1);
+HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 2);
+HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 3);
+HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 4);
+HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 5);
+HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 6);
+HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 7);
+
+HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 0);
+HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 1);
+HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 2);
+HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 3);
+HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 4);
+HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 5);
+HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 6);
+HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 7);
+
+HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 0);
+HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 1);
+HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 2);
+HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 3);
+HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 4);
+HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 5);
+HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 6);
+HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 7);
+
+HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
+	      0444, 0);
+HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
+	      0444, 1);
+HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
+	      0444, 2);
+HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
+	      0444, 3);
+HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
+	      0444, 4);
+HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
+	      0444, 5);
+HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
+	      0444, 6);
+HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
+	      0444, 7);
+
+HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 0);
+HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 1);
+HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 2);
+HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 3);
+HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 4);
+HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 5);
+HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 6);
+HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 7);
+
+HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 0);
+HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 1);
+HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 2);
+HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 3);
+HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 4);
+HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 5);
+HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 6);
+HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 7);
+
+HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 0);
+HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 1);
+HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 2);
+HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 3);
+HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 4);
+HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 5);
+HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 6);
+HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 7);
+
+HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 0);
+HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 1);
+HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 2);
+HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 3);
+HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 4);
+HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 5);
+HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 6);
+HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 7);
+
+HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 0);
+HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 1);
+HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 2);
+HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 3);
+HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 4);
+HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 5);
+HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 6);
+HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 7);
+
+HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
+	      0444, 0);
+HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
+	      0444, 1);
+HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
+	      0444, 2);
+HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
+	      0444, 3);
+HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
+	      0444, 4);
+HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
+	      0444, 5);
+HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
+	      0444, 6);
+HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
+	      0444, 7);
+
+HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
+	      hsmp_freq_limit_source_show, 0444, 0);
+HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
+	      hsmp_freq_limit_source_show, 0444, 1);
+HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
+	      hsmp_freq_limit_source_show, 0444, 2);
+HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
+	      hsmp_freq_limit_source_show, 0444, 3);
+HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
+	      hsmp_freq_limit_source_show, 0444, 4);
+HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
+	      hsmp_freq_limit_source_show, 0444, 5);
+HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
+	      hsmp_freq_limit_source_show, 0444, 6);
+HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
+	      hsmp_freq_limit_source_show, 0444, 7);
+
+#define HSMP_DEV_ATTR_LIST(_name, _sock_ind)					\
+static struct attribute _name[] = {						\
+	&hattr_c0_residency_input##_sock_ind.dattr.attr,			\
+	&hattr_prochot_status##_sock_ind.dattr.attr,				\
+	&hattr_smu_fw_version##_sock_ind.dattr.attr,				\
+	&hattr_protocol_version##_sock_ind.dattr.attr,				\
+	&hattr_cclk_freq_limit_input##_sock_ind.dattr.attr,			\
+	&hattr_ddr_max_bw##_sock_ind.dattr.attr,				\
+	&hattr_ddr_utilised_bw_input##_sock_ind.dattr.attr,			\
+	&hattr_ddr_utilised_bw_perc_input##_sock_ind.dattr.attr,		\
+	&hattr_fclk_input##_sock_ind.dattr.attr,				\
+	&hattr_mclk_input##_sock_ind.dattr.attr,				\
+	&hattr_clk_fmax##_sock_ind.dattr.attr,					\
+	&hattr_clk_fmin##_sock_ind.dattr.attr,					\
+	&hattr_pwr_current_active_freq_limit##_sock_ind.dattr.attr,		\
+	&hattr_pwr_current_active_freq_limit_source##_sock_ind.dattr.attr,	\
+	NULL									\
+}
+
+HSMP_DEV_ATTR_LIST(*sock0_dev_attr_list, 0);
+HSMP_DEV_ATTR_LIST(*sock1_dev_attr_list, 1);
+HSMP_DEV_ATTR_LIST(*sock2_dev_attr_list, 2);
+HSMP_DEV_ATTR_LIST(*sock3_dev_attr_list, 3);
+HSMP_DEV_ATTR_LIST(*sock4_dev_attr_list, 4);
+HSMP_DEV_ATTR_LIST(*sock5_dev_attr_list, 5);
+HSMP_DEV_ATTR_LIST(*sock6_dev_attr_list, 6);
+HSMP_DEV_ATTR_LIST(*sock7_dev_attr_list, 7);
+
+HSMP_ATTR_GRP(0, sock0_attr_list, socket0, sock0_dev_attr_list);
+HSMP_ATTR_GRP(1, sock1_attr_list, socket1, sock1_dev_attr_list);
+HSMP_ATTR_GRP(2, sock2_attr_list, socket2, sock2_dev_attr_list);
+HSMP_ATTR_GRP(3, sock3_attr_list, socket3, sock3_dev_attr_list);
+HSMP_ATTR_GRP(4, sock4_attr_list, socket4, sock4_dev_attr_list);
+HSMP_ATTR_GRP(5, sock5_attr_list, socket5, sock5_dev_attr_list);
+HSMP_ATTR_GRP(6, sock6_attr_list, socket6, sock6_dev_attr_list);
+HSMP_ATTR_GRP(7, sock7_attr_list, socket7, sock7_dev_attr_list);
 
 static const struct attribute_group *hsmp_groups[] = {
 	&sock0_attr_grp,
diff --git a/drivers/platform/x86/amd/hsmp/sysfs.h b/drivers/platform/x86/amd/hsmp/sysfs.h
new file mode 100644
index 000000000000..c4cd7e71e404
--- /dev/null
+++ b/drivers/platform/x86/amd/hsmp/sysfs.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * AMD HSMP Platform Driver
+ * Copyright (c) 2025, AMD.
+ * All Rights Reserved.
+ *
+ * Header file for HSMP sysfs interface
+ */
+
+#ifndef HSMP_SYSFS_H
+#define HSMP_SYSFS_H
+
+#include <linux/bits.h>
+#include <linux/container_of.h>
+#include <linux/device.h>
+#include <linux/types.h>
+
+struct hsmp_sys_attr {
+	struct device_attribute dattr;
+	u16 sock_ind;
+	u32 msg_id;
+};
+
+static char * const freqlimit_srcnames[] = {
+	"cHTC-Active",
+	"PROCHOT",
+	"TDC limit",
+	"PPT Limit",
+	"OPN Max",
+	"Reliability Limit",
+	"APML Agent",
+	"HSMP Agent",
+};
+
+#define DDR_MAX_BW_MASK		GENMASK(31, 20)
+#define DDR_UTIL_BW_MASK	GENMASK(19, 8)
+#define DDR_UTIL_BW_PERC_MASK	GENMASK(7, 0)
+#define FW_VER_MAJOR_MASK	GENMASK(23, 16)
+#define FW_VER_MINOR_MASK	GENMASK(15, 8)
+#define FW_VER_DEBUG_MASK	GENMASK(7, 0)
+#define FMAX_MASK		GENMASK(31, 16)
+#define FMIN_MASK		GENMASK(15, 0)
+#define FREQ_LIMIT_MASK		GENMASK(31, 16)
+#define FREQ_SRC_IND_MASK	GENMASK(15, 0)
+
+#define to_hsmp_sys_attr(_dev_attr) \
+	container_of(_dev_attr, struct hsmp_sys_attr, dattr)
+#endif /* HSMP_SYSFS_H */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] platform/x86/amd/hsmp: plat: Add sysfs files to display HSMP telemetry
  2025-08-06  6:38 ` [PATCH 2/2] platform/x86/amd/hsmp: plat: Add sysfs files to display HSMP telemetry Suma Hegde
@ 2025-08-19 11:42   ` Ilpo Järvinen
  2025-08-24 11:59     ` Suma Hegde
  0 siblings, 1 reply; 5+ messages in thread
From: Ilpo Järvinen @ 2025-08-19 11:42 UTC (permalink / raw)
  To: Suma Hegde; +Cc: platform-driver-x86, Hans de Goede, Naveen Krishna Chatradhi

On Wed, 6 Aug 2025, Suma Hegde wrote:

> This patch adds sysfs files to platform device based driver.
> 
> Following sysfs files are added similar to those in the ACPI based driver.
> * c0_residency_input
> * prochot_status
> * smu_fw_version
> * protocol_version
> * ddr_max_bw(GB/s)
> * ddr_utilised_bw_input(GB/s)
> * ddr_utilised_bw_perc_input(%)
> * mclk_input(MHz)
> * fclk_input(MHz)
> * clk_fmax(MHz)
> * clk_fmin(MHz)
> * cclk_freq_limit_input(MHz)
> * pwr_current_active_freq_limit(MHz)
> * pwr_current_active_freq_limit_source
> 
> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> ---
>  Documentation/arch/x86/amd_hsmp.rst   |   4 +-
>  drivers/platform/x86/amd/hsmp/acpi.c  |  33 +--
>  drivers/platform/x86/amd/hsmp/plat.c  | 388 +++++++++++++++++++++++++-
>  drivers/platform/x86/amd/hsmp/sysfs.h |  48 ++++
>  4 files changed, 433 insertions(+), 40 deletions(-)
>  create mode 100644 drivers/platform/x86/amd/hsmp/sysfs.h
> 
> diff --git a/Documentation/arch/x86/amd_hsmp.rst b/Documentation/arch/x86/amd_hsmp.rst
> index a094f55c10b0..6dd9948d8c21 100644
> --- a/Documentation/arch/x86/amd_hsmp.rst
> +++ b/Documentation/arch/x86/amd_hsmp.rst
> @@ -73,7 +73,9 @@ The same is defined in the amd_hsmp.h header.
>  
>  2. HSMP telemetry sysfs files
>  
> -Following sysfs files are available at /sys/devices/platform/AMDI0097:0X/.
> +Following sysfs files are available at /sys/devices/platform/AMDI0097:0X/ for
> +ACPI based driver and /sys/devices/platform/amd_hsmp/socketX/ for platform
> +device based driver.
>  
>  * c0_residency_input: Percentage of cores in C0 state.
>  * prochot_status: Reports 1 if the processor is at thermal threshold value,
> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
> index 19f0ca7958b6..f6434cf07f6b 100644
> --- a/drivers/platform/x86/amd/hsmp/acpi.c
> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
> @@ -13,12 +13,12 @@
>  
>  #include <linux/acpi.h>
>  #include <linux/array_size.h>
> -#include <linux/bits.h>
>  #include <linux/bitfield.h>
>  #include <linux/device.h>
>  #include <linux/dev_printk.h>
>  #include <linux/ioport.h>
>  #include <linux/kstrtox.h>
> +#include <linux/limits.h>
>  #include <linux/module.h>
>  #include <linux/platform_device.h>
>  #include <linux/sysfs.h>
> @@ -29,6 +29,7 @@
>  #include <asm/amd/node.h>
>  
>  #include "hsmp.h"
> +#include "sysfs.h"
>  
>  #define DRIVER_NAME		"hsmp_acpi"
>  
> @@ -39,11 +40,6 @@
>  
>  static struct hsmp_plat_device *hsmp_pdev;
>  
> -struct hsmp_sys_attr {
> -	struct device_attribute dattr;
> -	u32 msg_id;
> -};
> -
>  static int amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
>  			      u32 *value, bool write)
>  {
> @@ -257,8 +253,6 @@ static umode_t hsmp_is_sock_dev_attr_visible(struct kobject *kobj,
>  	return attr->mode;
>  }
>  
> -#define to_hsmp_sys_attr(_attr) container_of(_attr, struct hsmp_sys_attr, dattr)
> -
>  static ssize_t hsmp_msg_resp32_show(struct device *dev, struct device_attribute *attr,
>  				    char *buf)
>  {
> @@ -274,17 +268,6 @@ static ssize_t hsmp_msg_resp32_show(struct device *dev, struct device_attribute
>  	return sysfs_emit(buf, "%u\n", data);
>  }
>  
> -#define DDR_MAX_BW_MASK		GENMASK(31, 20)
> -#define DDR_UTIL_BW_MASK	GENMASK(19, 8)
> -#define DDR_UTIL_BW_PERC_MASK	GENMASK(7, 0)
> -#define FW_VER_MAJOR_MASK	GENMASK(23, 16)
> -#define FW_VER_MINOR_MASK	GENMASK(15, 8)
> -#define FW_VER_DEBUG_MASK	GENMASK(7, 0)
> -#define FMAX_MASK		GENMASK(31, 16)
> -#define FMIN_MASK		GENMASK(15, 0)
> -#define FREQ_LIMIT_MASK		GENMASK(31, 16)
> -#define FREQ_SRC_IND_MASK	GENMASK(15, 0)
> -
>  static ssize_t hsmp_ddr_max_bw_show(struct device *dev, struct device_attribute *attr,
>  				    char *buf)
>  {
> @@ -423,17 +406,6 @@ static ssize_t hsmp_freq_limit_show(struct device *dev, struct device_attribute
>  	return sysfs_emit(buf, "%lu\n", FIELD_GET(FREQ_LIMIT_MASK, data));
>  }
>  
> -static const char * const freqlimit_srcnames[] = {
> -	"cHTC-Active",
> -	"PROCHOT",
> -	"TDC limit",
> -	"PPT Limit",
> -	"OPN Max",
> -	"Reliability Limit",
> -	"APML Agent",
> -	"HSMP Agent",
> -};

Please put these moves into own patch.

>  static ssize_t hsmp_freq_limit_source_show(struct device *dev, struct device_attribute *attr,
>  					   char *buf)
>  {
> @@ -521,6 +493,7 @@ static const struct bin_attribute *hsmp_attr_list[] = {
>  #define HSMP_DEV_ATTR(_name, _msg_id, _show, _mode)	\
>  static struct hsmp_sys_attr hattr_##_name = {		\
>  	.dattr = __ATTR(_name, _mode, _show, NULL),	\
> +	.sock_ind = U16_MAX,				\
>  	.msg_id = _msg_id,				\
>  }
>  
> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
> index 10e8f98ea12c..3a0171ee4a80 100644
> --- a/drivers/platform/x86/amd/hsmp/plat.c
> +++ b/drivers/platform/x86/amd/hsmp/plat.c
> @@ -12,7 +12,9 @@
>  #include <asm/amd/hsmp.h>
>  
>  #include <linux/acpi.h>
> +#include <linux/bitfield.h>
>  #include <linux/build_bug.h>
> +#include <linux/container_of.h>
>  #include <linux/device.h>
>  #include <linux/dev_printk.h>
>  #include <linux/kconfig.h>
> @@ -24,6 +26,7 @@
>  #include <asm/amd/node.h>
>  
>  #include "hsmp.h"
> +#include "sysfs.h"
>  
>  #define DRIVER_NAME		"amd_hsmp"
>  
> @@ -78,6 +81,186 @@ static umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
>  	return 0;
>  }
>  
> +static umode_t hsmp_is_sock_dev_attr_visible(struct kobject *kobj,
> +					     struct attribute *attr, int id)
> +{
> +	struct device_attribute *dattr = container_of(attr, struct device_attribute, attr);
> +	struct hsmp_sys_attr *hattr = container_of(dattr, struct hsmp_sys_attr, dattr);
> +
> +	if (id == 0 && hattr->sock_ind >= hsmp_pdev->num_sockets)
> +		return SYSFS_GROUP_INVISIBLE;
> +
> +	return attr->mode;
> +}
> +
> +static ssize_t hsmp_msg_resp32_show(struct device *dev, struct device_attribute *attr,
> +				    char *buf)
> +{
> +	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
> +	u32 data;
> +	int ret;
> +
> +	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%u\n", data);
> +}
> +
> +static ssize_t hsmp_ddr_max_bw_show(struct device *dev, struct device_attribute *attr,
> +				    char *buf)
> +{
> +	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
> +	u32 data;
> +	int ret;
> +
> +	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%lu\n", FIELD_GET(DDR_MAX_BW_MASK, data));
> +}
> +
> +static ssize_t hsmp_ddr_util_bw_show(struct device *dev, struct device_attribute *attr,
> +				     char *buf)
> +{
> +	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
> +	u32 data;
> +	int ret;
> +
> +	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%lu\n", FIELD_GET(DDR_UTIL_BW_MASK, data));
> +}
> +
> +static ssize_t hsmp_ddr_util_bw_perc_show(struct device *dev, struct device_attribute *attr,
> +					  char *buf)
> +{
> +	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
> +	u32 data;
> +	int ret;
> +
> +	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%lu\n", FIELD_GET(DDR_UTIL_BW_PERC_MASK, data));
> +}
> +
> +static ssize_t hsmp_msg_fw_ver_show(struct device *dev, struct device_attribute *attr,
> +				    char *buf)
> +{
> +	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
> +	u32 data;
> +	int ret;
> +
> +	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%lu.%lu.%lu\n",
> +			  FIELD_GET(FW_VER_MAJOR_MASK, data),
> +			  FIELD_GET(FW_VER_MINOR_MASK, data),
> +			  FIELD_GET(FW_VER_DEBUG_MASK, data));
> +}
> +
> +static ssize_t hsmp_fclk_show(struct device *dev, struct device_attribute *attr,
> +			      char *buf)
> +{
> +	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
> +	u32 data[2];
> +	int ret;
> +
> +	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, data, 2);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%u\n", data[0]);
> +}
> +
> +static ssize_t hsmp_mclk_show(struct device *dev, struct device_attribute *attr,
> +			      char *buf)
> +{
> +	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
> +	u32 data[2];
> +	int ret;
> +
> +	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, data, 2);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%u\n", data[1]);
> +}
> +
> +static ssize_t hsmp_clk_fmax_show(struct device *dev, struct device_attribute *attr,
> +				  char *buf)
> +{
> +	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
> +	u32 data;
> +	int ret;
> +
> +	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%lu\n", FIELD_GET(FMAX_MASK, data));
> +}
> +
> +static ssize_t hsmp_clk_fmin_show(struct device *dev, struct device_attribute *attr,
> +				  char *buf)
> +{
> +	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
> +	u32 data;
> +	int ret;
> +
> +	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%lu\n", FIELD_GET(FMIN_MASK, data));
> +}
> +
> +static ssize_t hsmp_freq_limit_show(struct device *dev, struct device_attribute *attr,
> +				    char *buf)
> +{
> +	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
> +	u32 data;
> +	int ret;
> +
> +	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
> +	if (ret)
> +		return ret;
> +
> +	return sysfs_emit(buf, "%lu\n", FIELD_GET(FREQ_LIMIT_MASK, data));
> +}
> +
> +static ssize_t hsmp_freq_limit_source_show(struct device *dev, struct device_attribute *attr,
> +					   char *buf)
> +{
> +	struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
> +	unsigned int index;
> +	int len = 0;
> +	u16 src_ind;
> +	u32 data;
> +	int ret;
> +
> +	ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
> +	if (ret)
> +		return ret;
> +
> +	src_ind = FIELD_GET(FREQ_SRC_IND_MASK, data);
> +	for (index = 0; index < ARRAY_SIZE(freqlimit_srcnames); index++) {
> +		if (!src_ind)
> +			break;
> +		if (src_ind & 1)
> +			len += sysfs_emit_at(buf, len, "%s\n", freqlimit_srcnames[index]);
> +		src_ind >>= 1;
> +	}
> +	return len;
> +}
> +
>  /*
>   * AMD supports maximum of 8 sockets in a system.
>   * Static array of 8 + 1(for NULL) elements is created below
> @@ -110,21 +293,208 @@ HSMP_BIN_ATTR(5, *sock5_attr_list);
>  HSMP_BIN_ATTR(6, *sock6_attr_list);
>  HSMP_BIN_ATTR(7, *sock7_attr_list);
>  
> -#define HSMP_ATTR_GRP(index, _list, _name)			\
> +#define HSMP_ATTR_GRP(index, _list, _name, _dlist)		\
>  static const struct attribute_group sock##index##_attr_grp = {	\
>  	.bin_attrs_new = _list,					\
> +	.attrs = _dlist,					\
>  	.is_bin_visible = hsmp_is_sock_attr_visible,		\
> +	.is_visible = hsmp_is_sock_dev_attr_visible,		\

Please describe the change properly on the general level in the changelog 
as the necessity of these additional members are not obvious from the very 
terse wording of the changelog.

>  	.name = #_name,						\
>  }
>  
> -HSMP_ATTR_GRP(0, sock0_attr_list, socket0);
> -HSMP_ATTR_GRP(1, sock1_attr_list, socket1);
> -HSMP_ATTR_GRP(2, sock2_attr_list, socket2);
> -HSMP_ATTR_GRP(3, sock3_attr_list, socket3);
> -HSMP_ATTR_GRP(4, sock4_attr_list, socket4);
> -HSMP_ATTR_GRP(5, sock5_attr_list, socket5);
> -HSMP_ATTR_GRP(6, sock6_attr_list, socket6);
> -HSMP_ATTR_GRP(7, sock7_attr_list, socket7);
> +#define HSMP_DEV_ATTR(_name, _msg_id, _show, _mode, _sock_ind)	\
> +static struct hsmp_sys_attr hattr_##_name##_sock_ind = {	\
> +	.dattr = __ATTR(_name, _mode, _show, NULL),		\
> +	.sock_ind = _sock_ind,					\
> +	.msg_id = _msg_id,					\
> +}
> +
> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 0);
> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 1);
> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 2);
> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 3);
> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 4);
> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 5);
> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 6);
> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 7);
> +
> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 0);
> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 1);
> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 2);
> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 3);
> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 4);
> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 5);
> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 6);
> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 7);
> +
> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 0);
> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 1);
> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 2);
> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 3);
> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 4);
> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 5);
> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 6);
> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 7);
> +
> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 0);
> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 1);
> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 2);
> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 3);
> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 4);
> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 5);
> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 6);
> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 7);
> +
> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 0);
> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 1);
> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 2);
> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 3);
> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 4);
> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 5);
> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 6);
> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 7);
> +
> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 0);
> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 1);
> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 2);
> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 3);
> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 4);
> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 5);
> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 6);
> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 7);
> +
> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
> +	      0444, 0);
> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
> +	      0444, 1);
> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
> +	      0444, 2);
> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
> +	      0444, 3);
> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
> +	      0444, 4);
> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
> +	      0444, 5);
> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
> +	      0444, 6);
> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
> +	      0444, 7);
> +
> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 0);
> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 1);
> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 2);
> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 3);
> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 4);
> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 5);
> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 6);
> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 7);
> +
> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 0);
> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 1);
> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 2);
> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 3);
> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 4);
> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 5);
> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 6);
> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 7);
> +
> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 0);
> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 1);
> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 2);
> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 3);
> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 4);
> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 5);
> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 6);
> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 7);
> +
> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 0);
> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 1);
> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 2);
> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 3);
> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 4);
> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 5);
> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 6);
> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 7);
> +
> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 0);
> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 1);
> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 2);
> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 3);
> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 4);
> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 5);
> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 6);
> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 7);
> +
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
> +	      0444, 0);
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
> +	      0444, 1);
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
> +	      0444, 2);
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
> +	      0444, 3);
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
> +	      0444, 4);
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
> +	      0444, 5);
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
> +	      0444, 6);
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
> +	      0444, 7);
> +
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
> +	      hsmp_freq_limit_source_show, 0444, 0);
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
> +	      hsmp_freq_limit_source_show, 0444, 1);
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
> +	      hsmp_freq_limit_source_show, 0444, 2);
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
> +	      hsmp_freq_limit_source_show, 0444, 3);
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
> +	      hsmp_freq_limit_source_show, 0444, 4);
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
> +	      hsmp_freq_limit_source_show, 0444, 5);
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
> +	      hsmp_freq_limit_source_show, 0444, 6);
> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
> +	      hsmp_freq_limit_source_show, 0444, 7);

Any reason why you chose this way over having all those done by a macro 
for values 0-7?

> +#define HSMP_DEV_ATTR_LIST(_name, _sock_ind)					\
> +static struct attribute _name[] = {						\
> +	&hattr_c0_residency_input##_sock_ind.dattr.attr,			\
> +	&hattr_prochot_status##_sock_ind.dattr.attr,				\
> +	&hattr_smu_fw_version##_sock_ind.dattr.attr,				\
> +	&hattr_protocol_version##_sock_ind.dattr.attr,				\
> +	&hattr_cclk_freq_limit_input##_sock_ind.dattr.attr,			\
> +	&hattr_ddr_max_bw##_sock_ind.dattr.attr,				\
> +	&hattr_ddr_utilised_bw_input##_sock_ind.dattr.attr,			\
> +	&hattr_ddr_utilised_bw_perc_input##_sock_ind.dattr.attr,		\
> +	&hattr_fclk_input##_sock_ind.dattr.attr,				\
> +	&hattr_mclk_input##_sock_ind.dattr.attr,				\
> +	&hattr_clk_fmax##_sock_ind.dattr.attr,					\
> +	&hattr_clk_fmin##_sock_ind.dattr.attr,					\
> +	&hattr_pwr_current_active_freq_limit##_sock_ind.dattr.attr,		\
> +	&hattr_pwr_current_active_freq_limit_source##_sock_ind.dattr.attr,	\
> +	NULL									\
> +}
> +
> +HSMP_DEV_ATTR_LIST(*sock0_dev_attr_list, 0);
> +HSMP_DEV_ATTR_LIST(*sock1_dev_attr_list, 1);
> +HSMP_DEV_ATTR_LIST(*sock2_dev_attr_list, 2);
> +HSMP_DEV_ATTR_LIST(*sock3_dev_attr_list, 3);
> +HSMP_DEV_ATTR_LIST(*sock4_dev_attr_list, 4);
> +HSMP_DEV_ATTR_LIST(*sock5_dev_attr_list, 5);
> +HSMP_DEV_ATTR_LIST(*sock6_dev_attr_list, 6);
> +HSMP_DEV_ATTR_LIST(*sock7_dev_attr_list, 7);
> +
> +HSMP_ATTR_GRP(0, sock0_attr_list, socket0, sock0_dev_attr_list);
> +HSMP_ATTR_GRP(1, sock1_attr_list, socket1, sock1_dev_attr_list);
> +HSMP_ATTR_GRP(2, sock2_attr_list, socket2, sock2_dev_attr_list);
> +HSMP_ATTR_GRP(3, sock3_attr_list, socket3, sock3_dev_attr_list);
> +HSMP_ATTR_GRP(4, sock4_attr_list, socket4, sock4_dev_attr_list);
> +HSMP_ATTR_GRP(5, sock5_attr_list, socket5, sock5_dev_attr_list);
> +HSMP_ATTR_GRP(6, sock6_attr_list, socket6, sock6_dev_attr_list);
> +HSMP_ATTR_GRP(7, sock7_attr_list, socket7, sock7_dev_attr_list);
>  
>  static const struct attribute_group *hsmp_groups[] = {
>  	&sock0_attr_grp,
> diff --git a/drivers/platform/x86/amd/hsmp/sysfs.h b/drivers/platform/x86/amd/hsmp/sysfs.h
> new file mode 100644
> index 000000000000..c4cd7e71e404
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hsmp/sysfs.h
> @@ -0,0 +1,48 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * AMD HSMP Platform Driver
> + * Copyright (c) 2025, AMD.
> + * All Rights Reserved.
> + *
> + * Header file for HSMP sysfs interface
> + */
> +
> +#ifndef HSMP_SYSFS_H
> +#define HSMP_SYSFS_H
> +
> +#include <linux/bits.h>
> +#include <linux/container_of.h>
> +#include <linux/device.h>
> +#include <linux/types.h>
> +
> +struct hsmp_sys_attr {
> +	struct device_attribute dattr;
> +	u16 sock_ind;
> +	u32 msg_id;
> +};
> +
> +static char * const freqlimit_srcnames[] = {

Please don't put this into a header as it's duplicated to all compilation 
units.

Where one const went?

> +	"cHTC-Active",
> +	"PROCHOT",
> +	"TDC limit",
> +	"PPT Limit",
> +	"OPN Max",
> +	"Reliability Limit",
> +	"APML Agent",
> +	"HSMP Agent",
> +};
> +
> +#define DDR_MAX_BW_MASK		GENMASK(31, 20)
> +#define DDR_UTIL_BW_MASK	GENMASK(19, 8)
> +#define DDR_UTIL_BW_PERC_MASK	GENMASK(7, 0)
> +#define FW_VER_MAJOR_MASK	GENMASK(23, 16)
> +#define FW_VER_MINOR_MASK	GENMASK(15, 8)
> +#define FW_VER_DEBUG_MASK	GENMASK(7, 0)
> +#define FMAX_MASK		GENMASK(31, 16)
> +#define FMIN_MASK		GENMASK(15, 0)
> +#define FREQ_LIMIT_MASK		GENMASK(31, 16)
> +#define FREQ_SRC_IND_MASK	GENMASK(15, 0)
> +
> +#define to_hsmp_sys_attr(_dev_attr) \
> +	container_of(_dev_attr, struct hsmp_sys_attr, dattr)
> +#endif /* HSMP_SYSFS_H */
> 

-- 
 i.


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH 2/2] platform/x86/amd/hsmp: plat: Add sysfs files to display HSMP telemetry
  2025-08-19 11:42   ` Ilpo Järvinen
@ 2025-08-24 11:59     ` Suma Hegde
  0 siblings, 0 replies; 5+ messages in thread
From: Suma Hegde @ 2025-08-24 11:59 UTC (permalink / raw)
  To: Ilpo Järvinen
  Cc: platform-driver-x86, Hans de Goede, Naveen Krishna Chatradhi

Hi Ilpo,


On 8/19/2025 5:12 PM, Ilpo Järvinen wrote:
> Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
>
>
> On Wed, 6 Aug 2025, Suma Hegde wrote:
>
>> This patch adds sysfs files to platform device based driver.
>>
>> Following sysfs files are added similar to those in the ACPI based driver.
>> * c0_residency_input
>> * prochot_status
>> * smu_fw_version
>> * protocol_version
>> * ddr_max_bw(GB/s)
>> * ddr_utilised_bw_input(GB/s)
>> * ddr_utilised_bw_perc_input(%)
>> * mclk_input(MHz)
>> * fclk_input(MHz)
>> * clk_fmax(MHz)
>> * clk_fmin(MHz)
>> * cclk_freq_limit_input(MHz)
>> * pwr_current_active_freq_limit(MHz)
>> * pwr_current_active_freq_limit_source
>>
>> Signed-off-by: Suma Hegde <suma.hegde@amd.com>
>> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
>> ---
>>   Documentation/arch/x86/amd_hsmp.rst   |   4 +-
>>   drivers/platform/x86/amd/hsmp/acpi.c  |  33 +--
>>   drivers/platform/x86/amd/hsmp/plat.c  | 388 +++++++++++++++++++++++++-
>>   drivers/platform/x86/amd/hsmp/sysfs.h |  48 ++++
>>   4 files changed, 433 insertions(+), 40 deletions(-)
>>   create mode 100644 drivers/platform/x86/amd/hsmp/sysfs.h
>>
>> diff --git a/Documentation/arch/x86/amd_hsmp.rst b/Documentation/arch/x86/amd_hsmp.rst
>> index a094f55c10b0..6dd9948d8c21 100644
>> --- a/Documentation/arch/x86/amd_hsmp.rst
>> +++ b/Documentation/arch/x86/amd_hsmp.rst
>> @@ -73,7 +73,9 @@ The same is defined in the amd_hsmp.h header.
>>
>>   2. HSMP telemetry sysfs files
>>
>> -Following sysfs files are available at /sys/devices/platform/AMDI0097:0X/.
>> +Following sysfs files are available at /sys/devices/platform/AMDI0097:0X/ for
>> +ACPI based driver and /sys/devices/platform/amd_hsmp/socketX/ for platform
>> +device based driver.
>>
>>   * c0_residency_input: Percentage of cores in C0 state.
>>   * prochot_status: Reports 1 if the processor is at thermal threshold value,
>> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
>> index 19f0ca7958b6..f6434cf07f6b 100644
>> --- a/drivers/platform/x86/amd/hsmp/acpi.c
>> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
>> @@ -13,12 +13,12 @@
>>
>>   #include <linux/acpi.h>
>>   #include <linux/array_size.h>
>> -#include <linux/bits.h>
>>   #include <linux/bitfield.h>
>>   #include <linux/device.h>
>>   #include <linux/dev_printk.h>
>>   #include <linux/ioport.h>
>>   #include <linux/kstrtox.h>
>> +#include <linux/limits.h>
>>   #include <linux/module.h>
>>   #include <linux/platform_device.h>
>>   #include <linux/sysfs.h>
>> @@ -29,6 +29,7 @@
>>   #include <asm/amd/node.h>
>>
>>   #include "hsmp.h"
>> +#include "sysfs.h"
>>
>>   #define DRIVER_NAME          "hsmp_acpi"
>>
>> @@ -39,11 +40,6 @@
>>
>>   static struct hsmp_plat_device *hsmp_pdev;
>>
>> -struct hsmp_sys_attr {
>> -     struct device_attribute dattr;
>> -     u32 msg_id;
>> -};
>> -
>>   static int amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
>>                              u32 *value, bool write)
>>   {
>> @@ -257,8 +253,6 @@ static umode_t hsmp_is_sock_dev_attr_visible(struct kobject *kobj,
>>        return attr->mode;
>>   }
>>
>> -#define to_hsmp_sys_attr(_attr) container_of(_attr, struct hsmp_sys_attr, dattr)
>> -
>>   static ssize_t hsmp_msg_resp32_show(struct device *dev, struct device_attribute *attr,
>>                                    char *buf)
>>   {
>> @@ -274,17 +268,6 @@ static ssize_t hsmp_msg_resp32_show(struct device *dev, struct device_attribute
>>        return sysfs_emit(buf, "%u\n", data);
>>   }
>>
>> -#define DDR_MAX_BW_MASK              GENMASK(31, 20)
>> -#define DDR_UTIL_BW_MASK     GENMASK(19, 8)
>> -#define DDR_UTIL_BW_PERC_MASK        GENMASK(7, 0)
>> -#define FW_VER_MAJOR_MASK    GENMASK(23, 16)
>> -#define FW_VER_MINOR_MASK    GENMASK(15, 8)
>> -#define FW_VER_DEBUG_MASK    GENMASK(7, 0)
>> -#define FMAX_MASK            GENMASK(31, 16)
>> -#define FMIN_MASK            GENMASK(15, 0)
>> -#define FREQ_LIMIT_MASK              GENMASK(31, 16)
>> -#define FREQ_SRC_IND_MASK    GENMASK(15, 0)
>> -
>>   static ssize_t hsmp_ddr_max_bw_show(struct device *dev, struct device_attribute *attr,
>>                                    char *buf)
>>   {
>> @@ -423,17 +406,6 @@ static ssize_t hsmp_freq_limit_show(struct device *dev, struct device_attribute
>>        return sysfs_emit(buf, "%lu\n", FIELD_GET(FREQ_LIMIT_MASK, data));
>>   }
>>
>> -static const char * const freqlimit_srcnames[] = {
>> -     "cHTC-Active",
>> -     "PROCHOT",
>> -     "TDC limit",
>> -     "PPT Limit",
>> -     "OPN Max",
>> -     "Reliability Limit",
>> -     "APML Agent",
>> -     "HSMP Agent",
>> -};
> Please put these moves into own patch.
>
>>   static ssize_t hsmp_freq_limit_source_show(struct device *dev, struct device_attribute *attr,
>>                                           char *buf)
>>   {
>> @@ -521,6 +493,7 @@ static const struct bin_attribute *hsmp_attr_list[] = {
>>   #define HSMP_DEV_ATTR(_name, _msg_id, _show, _mode)  \
>>   static struct hsmp_sys_attr hattr_##_name = {                \
>>        .dattr = __ATTR(_name, _mode, _show, NULL),     \
>> +     .sock_ind = U16_MAX,                            \
>>        .msg_id = _msg_id,                              \
>>   }
>>
>> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
>> index 10e8f98ea12c..3a0171ee4a80 100644
>> --- a/drivers/platform/x86/amd/hsmp/plat.c
>> +++ b/drivers/platform/x86/amd/hsmp/plat.c
>> @@ -12,7 +12,9 @@
>>   #include <asm/amd/hsmp.h>
>>
>>   #include <linux/acpi.h>
>> +#include <linux/bitfield.h>
>>   #include <linux/build_bug.h>
>> +#include <linux/container_of.h>
>>   #include <linux/device.h>
>>   #include <linux/dev_printk.h>
>>   #include <linux/kconfig.h>
>> @@ -24,6 +26,7 @@
>>   #include <asm/amd/node.h>
>>
>>   #include "hsmp.h"
>> +#include "sysfs.h"
>>
>>   #define DRIVER_NAME          "amd_hsmp"
>>
>> @@ -78,6 +81,186 @@ static umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
>>        return 0;
>>   }
>>
>> +static umode_t hsmp_is_sock_dev_attr_visible(struct kobject *kobj,
>> +                                          struct attribute *attr, int id)
>> +{
>> +     struct device_attribute *dattr = container_of(attr, struct device_attribute, attr);
>> +     struct hsmp_sys_attr *hattr = container_of(dattr, struct hsmp_sys_attr, dattr);
>> +
>> +     if (id == 0 && hattr->sock_ind >= hsmp_pdev->num_sockets)
>> +             return SYSFS_GROUP_INVISIBLE;
>> +
>> +     return attr->mode;
>> +}
>> +
>> +static ssize_t hsmp_msg_resp32_show(struct device *dev, struct device_attribute *attr,
>> +                                 char *buf)
>> +{
>> +     struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
>> +     u32 data;
>> +     int ret;
>> +
>> +     ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return sysfs_emit(buf, "%u\n", data);
>> +}
>> +
>> +static ssize_t hsmp_ddr_max_bw_show(struct device *dev, struct device_attribute *attr,
>> +                                 char *buf)
>> +{
>> +     struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
>> +     u32 data;
>> +     int ret;
>> +
>> +     ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return sysfs_emit(buf, "%lu\n", FIELD_GET(DDR_MAX_BW_MASK, data));
>> +}
>> +
>> +static ssize_t hsmp_ddr_util_bw_show(struct device *dev, struct device_attribute *attr,
>> +                                  char *buf)
>> +{
>> +     struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
>> +     u32 data;
>> +     int ret;
>> +
>> +     ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return sysfs_emit(buf, "%lu\n", FIELD_GET(DDR_UTIL_BW_MASK, data));
>> +}
>> +
>> +static ssize_t hsmp_ddr_util_bw_perc_show(struct device *dev, struct device_attribute *attr,
>> +                                       char *buf)
>> +{
>> +     struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
>> +     u32 data;
>> +     int ret;
>> +
>> +     ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return sysfs_emit(buf, "%lu\n", FIELD_GET(DDR_UTIL_BW_PERC_MASK, data));
>> +}
>> +
>> +static ssize_t hsmp_msg_fw_ver_show(struct device *dev, struct device_attribute *attr,
>> +                                 char *buf)
>> +{
>> +     struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
>> +     u32 data;
>> +     int ret;
>> +
>> +     ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return sysfs_emit(buf, "%lu.%lu.%lu\n",
>> +                       FIELD_GET(FW_VER_MAJOR_MASK, data),
>> +                       FIELD_GET(FW_VER_MINOR_MASK, data),
>> +                       FIELD_GET(FW_VER_DEBUG_MASK, data));
>> +}
>> +
>> +static ssize_t hsmp_fclk_show(struct device *dev, struct device_attribute *attr,
>> +                           char *buf)
>> +{
>> +     struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
>> +     u32 data[2];
>> +     int ret;
>> +
>> +     ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, data, 2);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return sysfs_emit(buf, "%u\n", data[0]);
>> +}
>> +
>> +static ssize_t hsmp_mclk_show(struct device *dev, struct device_attribute *attr,
>> +                           char *buf)
>> +{
>> +     struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
>> +     u32 data[2];
>> +     int ret;
>> +
>> +     ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, data, 2);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return sysfs_emit(buf, "%u\n", data[1]);
>> +}
>> +
>> +static ssize_t hsmp_clk_fmax_show(struct device *dev, struct device_attribute *attr,
>> +                               char *buf)
>> +{
>> +     struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
>> +     u32 data;
>> +     int ret;
>> +
>> +     ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return sysfs_emit(buf, "%lu\n", FIELD_GET(FMAX_MASK, data));
>> +}
>> +
>> +static ssize_t hsmp_clk_fmin_show(struct device *dev, struct device_attribute *attr,
>> +                               char *buf)
>> +{
>> +     struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
>> +     u32 data;
>> +     int ret;
>> +
>> +     ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return sysfs_emit(buf, "%lu\n", FIELD_GET(FMIN_MASK, data));
>> +}
>> +
>> +static ssize_t hsmp_freq_limit_show(struct device *dev, struct device_attribute *attr,
>> +                                 char *buf)
>> +{
>> +     struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
>> +     u32 data;
>> +     int ret;
>> +
>> +     ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
>> +     if (ret)
>> +             return ret;
>> +
>> +     return sysfs_emit(buf, "%lu\n", FIELD_GET(FREQ_LIMIT_MASK, data));
>> +}
>> +
>> +static ssize_t hsmp_freq_limit_source_show(struct device *dev, struct device_attribute *attr,
>> +                                        char *buf)
>> +{
>> +     struct hsmp_sys_attr *hattr = to_hsmp_sys_attr(attr);
>> +     unsigned int index;
>> +     int len = 0;
>> +     u16 src_ind;
>> +     u32 data;
>> +     int ret;
>> +
>> +     ret = hsmp_msg_get_nargs(hattr->sock_ind, hattr->msg_id, &data, 1);
>> +     if (ret)
>> +             return ret;
>> +
>> +     src_ind = FIELD_GET(FREQ_SRC_IND_MASK, data);
>> +     for (index = 0; index < ARRAY_SIZE(freqlimit_srcnames); index++) {
>> +             if (!src_ind)
>> +                     break;
>> +             if (src_ind & 1)
>> +                     len += sysfs_emit_at(buf, len, "%s\n", freqlimit_srcnames[index]);
>> +             src_ind >>= 1;
>> +     }
>> +     return len;
>> +}
>> +
>>   /*
>>    * AMD supports maximum of 8 sockets in a system.
>>    * Static array of 8 + 1(for NULL) elements is created below
>> @@ -110,21 +293,208 @@ HSMP_BIN_ATTR(5, *sock5_attr_list);
>>   HSMP_BIN_ATTR(6, *sock6_attr_list);
>>   HSMP_BIN_ATTR(7, *sock7_attr_list);
>>
>> -#define HSMP_ATTR_GRP(index, _list, _name)                   \
>> +#define HSMP_ATTR_GRP(index, _list, _name, _dlist)           \
>>   static const struct attribute_group sock##index##_attr_grp = {       \
>>        .bin_attrs_new = _list,                                 \
>> +     .attrs = _dlist,                                        \
>>        .is_bin_visible = hsmp_is_sock_attr_visible,            \
>> +     .is_visible = hsmp_is_sock_dev_attr_visible,            \
> Please describe the change properly on the general level in the changelog
> as the necessity of these additional members are not obvious from the very
> terse wording of the changelog.
>
>>        .name = #_name,                                         \
>>   }
>>
>> -HSMP_ATTR_GRP(0, sock0_attr_list, socket0);
>> -HSMP_ATTR_GRP(1, sock1_attr_list, socket1);
>> -HSMP_ATTR_GRP(2, sock2_attr_list, socket2);
>> -HSMP_ATTR_GRP(3, sock3_attr_list, socket3);
>> -HSMP_ATTR_GRP(4, sock4_attr_list, socket4);
>> -HSMP_ATTR_GRP(5, sock5_attr_list, socket5);
>> -HSMP_ATTR_GRP(6, sock6_attr_list, socket6);
>> -HSMP_ATTR_GRP(7, sock7_attr_list, socket7);
>> +#define HSMP_DEV_ATTR(_name, _msg_id, _show, _mode, _sock_ind)       \
>> +static struct hsmp_sys_attr hattr_##_name##_sock_ind = {     \
>> +     .dattr = __ATTR(_name, _mode, _show, NULL),             \
>> +     .sock_ind = _sock_ind,                                  \
>> +     .msg_id = _msg_id,                                      \
>> +}
>> +
>> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 0);
>> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 1);
>> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 2);
>> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 3);
>> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 4);
>> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 5);
>> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 6);
>> +HSMP_DEV_ATTR(c0_residency_input, HSMP_GET_C0_PERCENT, hsmp_msg_resp32_show, 0444, 7);
>> +
>> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 0);
>> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 1);
>> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 2);
>> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 3);
>> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 4);
>> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 5);
>> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 6);
>> +HSMP_DEV_ATTR(smu_fw_version, HSMP_GET_SMU_VER, hsmp_msg_fw_ver_show, 0444, 7);
>> +
>> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 0);
>> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 1);
>> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 2);
>> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 3);
>> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 4);
>> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 5);
>> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 6);
>> +HSMP_DEV_ATTR(protocol_version, HSMP_GET_PROTO_VER, hsmp_msg_resp32_show, 0444, 7);
>> +
>> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 0);
>> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 1);
>> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 2);
>> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 3);
>> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 4);
>> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 5);
>> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 6);
>> +HSMP_DEV_ATTR(prochot_status, HSMP_GET_PROC_HOT, hsmp_msg_resp32_show, 0444, 7);
>> +
>> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 0);
>> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 1);
>> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 2);
>> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 3);
>> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 4);
>> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 5);
>> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 6);
>> +HSMP_DEV_ATTR(ddr_max_bw, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_max_bw_show, 0444, 7);
>> +
>> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 0);
>> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 1);
>> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 2);
>> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 3);
>> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 4);
>> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 5);
>> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 6);
>> +HSMP_DEV_ATTR(ddr_utilised_bw_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_show, 0444, 7);
>> +
>> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
>> +           0444, 0);
>> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
>> +           0444, 1);
>> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
>> +           0444, 2);
>> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
>> +           0444, 3);
>> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
>> +           0444, 4);
>> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
>> +           0444, 5);
>> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
>> +           0444, 6);
>> +HSMP_DEV_ATTR(ddr_utilised_bw_perc_input, HSMP_GET_DDR_BANDWIDTH, hsmp_ddr_util_bw_perc_show,
>> +           0444, 7);
>> +
>> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 0);
>> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 1);
>> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 2);
>> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 3);
>> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 4);
>> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 5);
>> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 6);
>> +HSMP_DEV_ATTR(cclk_freq_limit_input, HSMP_GET_CCLK_THROTTLE_LIMIT, hsmp_msg_resp32_show, 0444, 7);
>> +
>> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 0);
>> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 1);
>> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 2);
>> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 3);
>> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 4);
>> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 5);
>> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 6);
>> +HSMP_DEV_ATTR(fclk_input, HSMP_GET_FCLK_MCLK, hsmp_fclk_show, 0444, 7);
>> +
>> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 0);
>> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 1);
>> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 2);
>> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 3);
>> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 4);
>> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 5);
>> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 6);
>> +HSMP_DEV_ATTR(mclk_input, HSMP_GET_FCLK_MCLK, hsmp_mclk_show, 0444, 7);
>> +
>> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 0);
>> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 1);
>> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 2);
>> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 3);
>> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 4);
>> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 5);
>> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 6);
>> +HSMP_DEV_ATTR(clk_fmax, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmax_show, 0444, 7);
>> +
>> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 0);
>> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 1);
>> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 2);
>> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 3);
>> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 4);
>> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 5);
>> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 6);
>> +HSMP_DEV_ATTR(clk_fmin, HSMP_GET_SOCKET_FMAX_FMIN, hsmp_clk_fmin_show, 0444, 7);
>> +
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
>> +           0444, 0);
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
>> +           0444, 1);
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
>> +           0444, 2);
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
>> +           0444, 3);
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
>> +           0444, 4);
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
>> +           0444, 5);
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
>> +           0444, 6);
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit, HSMP_GET_SOCKET_FREQ_LIMIT, hsmp_freq_limit_show,
>> +           0444, 7);
>> +
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
>> +           hsmp_freq_limit_source_show, 0444, 0);
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
>> +           hsmp_freq_limit_source_show, 0444, 1);
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
>> +           hsmp_freq_limit_source_show, 0444, 2);
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
>> +           hsmp_freq_limit_source_show, 0444, 3);
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
>> +           hsmp_freq_limit_source_show, 0444, 4);
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
>> +           hsmp_freq_limit_source_show, 0444, 5);
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
>> +           hsmp_freq_limit_source_show, 0444, 6);
>> +HSMP_DEV_ATTR(pwr_current_active_freq_limit_source, HSMP_GET_SOCKET_FREQ_LIMIT,
>> +           hsmp_freq_limit_source_show, 0444, 7);
> Any reason why you chose this way over having all those done by a macro
> for values 0-7?


I encountered  checkpatch "CHECK" s (CHECK: Macro argument reuse '_name' 
- possible side-effects?)

when I added the following code:

#define HSMP_DEV_ATTR_FOR_SOCK_ALL(_name, _msgid, _func, _mode) {       \
         HSMP_DEV_ATTR(_name, _msgid, _func, _mode, 0)                   \
         HSMP_DEV_ATTR(_name, _msgid, _func, _mode, 1)                   \
         HSMP_DEV_ATTR(_name, _msgid, _func, _mode, 2)                   \
         HSMP_DEV_ATTR(_name, _msgid, _func, _mode, 3)                   \
         HSMP_DEV_ATTR(_name, _msgid, _func, _mode, 4)                   \
         HSMP_DEV_ATTR(_name, _msgid, _func, _mode, 5)                   \
         HSMP_DEV_ATTR(_name, _msgid, _func, _mode, 6)                   \
         HSMP_DEV_ATTR(_name, _msgid, _func, _mode, 7)                   \
}

  As a result, I decided not to include this macro.


>> +#define HSMP_DEV_ATTR_LIST(_name, _sock_ind)                                 \
>> +static struct attribute _name[] = {                                          \
>> +     &hattr_c0_residency_input##_sock_ind.dattr.attr,                        \
>> +     &hattr_prochot_status##_sock_ind.dattr.attr,                            \
>> +     &hattr_smu_fw_version##_sock_ind.dattr.attr,                            \
>> +     &hattr_protocol_version##_sock_ind.dattr.attr,                          \
>> +     &hattr_cclk_freq_limit_input##_sock_ind.dattr.attr,                     \
>> +     &hattr_ddr_max_bw##_sock_ind.dattr.attr,                                \
>> +     &hattr_ddr_utilised_bw_input##_sock_ind.dattr.attr,                     \
>> +     &hattr_ddr_utilised_bw_perc_input##_sock_ind.dattr.attr,                \
>> +     &hattr_fclk_input##_sock_ind.dattr.attr,                                \
>> +     &hattr_mclk_input##_sock_ind.dattr.attr,                                \
>> +     &hattr_clk_fmax##_sock_ind.dattr.attr,                                  \
>> +     &hattr_clk_fmin##_sock_ind.dattr.attr,                                  \
>> +     &hattr_pwr_current_active_freq_limit##_sock_ind.dattr.attr,             \
>> +     &hattr_pwr_current_active_freq_limit_source##_sock_ind.dattr.attr,      \
>> +     NULL                                                                    \
>> +}
>> +
>> +HSMP_DEV_ATTR_LIST(*sock0_dev_attr_list, 0);
>> +HSMP_DEV_ATTR_LIST(*sock1_dev_attr_list, 1);
>> +HSMP_DEV_ATTR_LIST(*sock2_dev_attr_list, 2);
>> +HSMP_DEV_ATTR_LIST(*sock3_dev_attr_list, 3);
>> +HSMP_DEV_ATTR_LIST(*sock4_dev_attr_list, 4);
>> +HSMP_DEV_ATTR_LIST(*sock5_dev_attr_list, 5);
>> +HSMP_DEV_ATTR_LIST(*sock6_dev_attr_list, 6);
>> +HSMP_DEV_ATTR_LIST(*sock7_dev_attr_list, 7);
>> +
>> +HSMP_ATTR_GRP(0, sock0_attr_list, socket0, sock0_dev_attr_list);
>> +HSMP_ATTR_GRP(1, sock1_attr_list, socket1, sock1_dev_attr_list);
>> +HSMP_ATTR_GRP(2, sock2_attr_list, socket2, sock2_dev_attr_list);
>> +HSMP_ATTR_GRP(3, sock3_attr_list, socket3, sock3_dev_attr_list);
>> +HSMP_ATTR_GRP(4, sock4_attr_list, socket4, sock4_dev_attr_list);
>> +HSMP_ATTR_GRP(5, sock5_attr_list, socket5, sock5_dev_attr_list);
>> +HSMP_ATTR_GRP(6, sock6_attr_list, socket6, sock6_dev_attr_list);
>> +HSMP_ATTR_GRP(7, sock7_attr_list, socket7, sock7_dev_attr_list);
>>
>>   static const struct attribute_group *hsmp_groups[] = {
>>        &sock0_attr_grp,
>> diff --git a/drivers/platform/x86/amd/hsmp/sysfs.h b/drivers/platform/x86/amd/hsmp/sysfs.h
>> new file mode 100644
>> index 000000000000..c4cd7e71e404
>> --- /dev/null
>> +++ b/drivers/platform/x86/amd/hsmp/sysfs.h
>> @@ -0,0 +1,48 @@
>> +/* SPDX-License-Identifier: GPL-2.0 */
>> +/*
>> + * AMD HSMP Platform Driver
>> + * Copyright (c) 2025, AMD.
>> + * All Rights Reserved.
>> + *
>> + * Header file for HSMP sysfs interface
>> + */
>> +
>> +#ifndef HSMP_SYSFS_H
>> +#define HSMP_SYSFS_H
>> +
>> +#include <linux/bits.h>
>> +#include <linux/container_of.h>
>> +#include <linux/device.h>
>> +#include <linux/types.h>
>> +
>> +struct hsmp_sys_attr {
>> +     struct device_attribute dattr;
>> +     u16 sock_ind;
>> +     u32 msg_id;
>> +};
>> +
>> +static char * const freqlimit_srcnames[] = {
> Please don't put this into a header as it's duplicated to all compilation
> units.
>
> Where one const went?


I will address this and other comments in next version.


>
>> +     "cHTC-Active",
>> +     "PROCHOT",
>> +     "TDC limit",
>> +     "PPT Limit",
>> +     "OPN Max",
>> +     "Reliability Limit",
>> +     "APML Agent",
>> +     "HSMP Agent",
>> +};
>> +
>> +#define DDR_MAX_BW_MASK              GENMASK(31, 20)
>> +#define DDR_UTIL_BW_MASK     GENMASK(19, 8)
>> +#define DDR_UTIL_BW_PERC_MASK        GENMASK(7, 0)
>> +#define FW_VER_MAJOR_MASK    GENMASK(23, 16)
>> +#define FW_VER_MINOR_MASK    GENMASK(15, 8)
>> +#define FW_VER_DEBUG_MASK    GENMASK(7, 0)
>> +#define FMAX_MASK            GENMASK(31, 16)
>> +#define FMIN_MASK            GENMASK(15, 0)
>> +#define FREQ_LIMIT_MASK              GENMASK(31, 16)
>> +#define FREQ_SRC_IND_MASK    GENMASK(15, 0)
>> +
>> +#define to_hsmp_sys_attr(_dev_attr) \
>> +     container_of(_dev_attr, struct hsmp_sys_attr, dattr)
>> +#endif /* HSMP_SYSFS_H */
>>
> --
>   i.


Thanks and Regards,

Suma

>

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2025-08-24 11:59 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-06  6:38 [PATCH 0/2] Add sysfs files to plat device based driver Suma Hegde
2025-08-06  6:38 ` [PATCH 1/2] platform/x86/amd/hsmp: Rename the HSMP_BIN_ATTR_GRP macro Suma Hegde
2025-08-06  6:38 ` [PATCH 2/2] platform/x86/amd/hsmp: plat: Add sysfs files to display HSMP telemetry Suma Hegde
2025-08-19 11:42   ` Ilpo Järvinen
2025-08-24 11:59     ` Suma Hegde

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).