Re: [PATCH 08/10] platform/x86/amd/hsmp: Move read and is_visible to respective files

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Mario,

Thank you for your review. I will address these review comments in v2.

On 6/28/2024 1:18 AM, Mario Limonciello wrote:
On 6/27/2024 00:39, Suma Hegde wrote:
The .read() and .is_visibile() needs to be handled differently in acpi and

is_visible()

platform drivers, due to the way the sysfs files are created.

This is in preparation to using .dev_groups instead of dynamic sysfs
creation. The sysfs at this point is not functional, it will be enabled in
the next patch.

Signed-off-by: Suma Hegde <suma.hegde@xxxxxxx>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@xxxxxxx>
---
  drivers/platform/x86/amd/hsmp/acpi.c | 41 ++++++++++++++++++++
  drivers/platform/x86/amd/hsmp/hsmp.c | 37 ------------------
  drivers/platform/x86/amd/hsmp/plat.c | 57 ++++++++++++++++++++++++++++
  3 files changed, 98 insertions(+), 37 deletions(-)

diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
index 0307f4e7176d..1ea17aa296c7 100644
--- a/drivers/platform/x86/amd/hsmp/acpi.c
+++ b/drivers/platform/x86/amd/hsmp/acpi.c
@@ -12,6 +12,7 @@
  #include "hsmp.h"
    #include <linux/acpi.h>
+#include <asm/amd_hsmp.h>
  #include <asm/amd_nb.h>
  #include <linux/platform_device.h>
  @@ -206,6 +207,8 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
        sema_init(&sock->hsmp_sem, 1);
  +    dev_set_drvdata(dev, sock);
+
      /* Read MP1 base address from CRS method */
      ret = hsmp_read_acpi_crs(sock);
      if (ret)
@@ -238,6 +241,44 @@ static int hsmp_create_acpi_sysfs_if(struct device *dev)
      return devm_device_add_group(dev, attr_grp);
  }
  +ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj,
+                 struct bin_attribute *bin_attr, char *buf,
+                 loff_t off, size_t count)
+{
+    struct device *dev = container_of(kobj, struct device, kobj);
+    struct hsmp_socket *sock = dev_get_drvdata(dev);
+    struct hsmp_message msg = { 0 };
+    int ret;
+
+    if (!sock)
+        return -EINVAL;
+
+    /* Do not support lseek(), reads entire metric table */
+    if (count < bin_attr->size) {
+        dev_err(sock->dev, "Wrong buffer size\n");
+        return -EINVAL;
+    }
+
+    msg.msg_id      = HSMP_GET_METRIC_TABLE;
+    msg.sock_ind    = sock->sock_ind;
+
+    ret = hsmp_send_message(&msg);
+    if (ret)
+        return ret;
+    memcpy_fromio(buf, sock->metric_tbl_addr, bin_attr->size);
+
+    return bin_attr->size;
+}
+
+umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
+                  struct bin_attribute *battr, int id)
+{
+    if (plat_dev.proto_ver == HSMP_PROTO_VER6)
+        return battr->attr.mode;
+    else

Since your only path in the "if" returns this else is redundant.

+        return 0;
+}
+
  static int init_acpi(struct device *dev)
  {
      u16 sock_ind;
diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
index 4bf598021f4a..c199a0ff457d 100644
--- a/drivers/platform/x86/amd/hsmp/hsmp.c
+++ b/drivers/platform/x86/amd/hsmp/hsmp.c
@@ -273,34 +273,6 @@ long hsmp_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
      return 0;
  }
  -ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj,
-                 struct bin_attribute *bin_attr, char *buf,
-                 loff_t off, size_t count)
-{
-    struct hsmp_socket *sock = bin_attr->private;
-    struct hsmp_message msg = { 0 };
-    int ret;
-
-    if (!sock)
-        return -EINVAL;
-
-    /* Do not support lseek(), reads entire metric table */
-    if (count < bin_attr->size) {
-        dev_err(sock->dev, "Wrong buffer size\n");
-        return -EINVAL;
-    }
-
-    msg.msg_id    = HSMP_GET_METRIC_TABLE;
-    msg.sock_ind    = sock->sock_ind;
-
-    ret = hsmp_send_message(&msg);
-    if (ret)
-        return ret;
-    memcpy_fromio(buf, sock->metric_tbl_addr, bin_attr->size);
-
-    return bin_attr->size;
-}
-
  static int hsmp_get_tbl_dram_base(u16 sock_ind)
  {
      struct hsmp_socket *sock = &plat_dev.sock[sock_ind];
@@ -334,15 +306,6 @@ static int hsmp_get_tbl_dram_base(u16 sock_ind)
      return 0;
  }
  -umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
-                  struct bin_attribute *battr, int id)
-{
-    if (plat_dev.proto_ver == HSMP_PROTO_VER6)
-        return battr->attr.mode;
-    else
-        return 0;
-}
-
  static int hsmp_init_metric_tbl_bin_attr(struct bin_attribute **hattrs, u16 sock_ind)
  {
      struct bin_attribute *hattr = &plat_dev.sock[sock_ind].hsmp_attr;
diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
index 62423581d839..57aa64b18e0d 100644
--- a/drivers/platform/x86/amd/hsmp/plat.c
+++ b/drivers/platform/x86/amd/hsmp/plat.c
@@ -11,6 +11,7 @@
    #include "hsmp.h"
  +#include <asm/amd_hsmp.h>
  #include <asm/amd_nb.h>
  #include <linux/module.h>
  #include <linux/pci.h>
@@ -88,6 +89,62 @@ static int hsmp_create_non_acpi_sysfs_if(struct device *dev)
      return device_add_groups(dev, hsmp_attr_grps);
  }
  +ssize_t hsmp_metric_tbl_read(struct file *filp, struct kobject *kobj,
+                 struct bin_attribute *bin_attr, char *buf,
+                 loff_t off, size_t count)
+{
+    struct hsmp_message msg = { 0 };
+    struct hsmp_socket *sock;
+    u8 sock_ind;
+    int ret;
+
+    ret = kstrtou8(bin_attr->private, 10, &sock_ind);
+    if (ret)
+        return ret;
+
+    if (sock_ind >= plat_dev.num_sockets)
+        return -EINVAL;
+
+    sock = &plat_dev.sock[sock_ind];
+    if (!sock)
+        return -EINVAL;
+
+    /* Do not support lseek(), reads entire metric table */
+    if (count < bin_attr->size) {
+        dev_err(sock->dev, "Wrong buffer size\n");
+        return -EINVAL;
+    }
+
+    msg.msg_id    = HSMP_GET_METRIC_TABLE;
+    msg.sock_ind    = sock_ind;
+
+    ret = hsmp_send_message(&msg);
+    if (ret)
+        return ret;
+    memcpy_fromio(buf, sock->metric_tbl_addr, bin_attr->size);
+
+    return bin_attr->size;
+}
+
+umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
+                  struct bin_attribute *battr, int id)
+{
+    u8 sock_ind;
+    int ret;
+
+    ret = kstrtou8(battr->private, 10, &sock_ind);
+    if (ret)
+        return ret;
+
+    if (id == 0 && sock_ind >= plat_dev.num_sockets)
+        return SYSFS_GROUP_INVISIBLE;
+
+    if (plat_dev.proto_ver == HSMP_PROTO_VER6)
+        return battr->attr.mode;
+    else
+        return 0;

Since your only path in the "if" returns this else is redundant.

+}
+
  static inline bool is_f1a_m0h(void)
  {
      if (boot_cpu_data.x86 == 0x1A && boot_cpu_data.x86_model <= 0x0F)

Thanks and Regards,

Suma





[Index of Archives]     [Linux Kernel Development]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux