| Message ID | 20240627053958.2533860-9-suma.hegde@amd.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | platform/x86/amd/hsmp: Split ACPI and plat device driver | expand |
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@amd.com> > Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com> > --- > 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)
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@amd.com> >> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com> >> --- >> 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
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 + 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; +} + static inline bool is_f1a_m0h(void) { if (boot_cpu_data.x86 == 0x1A && boot_cpu_data.x86_model <= 0x0F)