On Tue, 20 Aug 2024, Suma Hegde wrote:
> Move platform device part to plat.c.
These move patches should also state why we're doing this. Something along
the lines of:
An upcoming change splits HSMP driver into ACPI and platform device
variants. Prepare for the split by moving ...
Think of somebody using git annotate/blame on this code in this driver and
landing on a line originating from this change.
> No functinality/logical changes.
> Common code which can be used by ACPI and platform device
> remains in hsmp.c.
> ACPI code in hsmp.c will be moved to acpi.c in next patch.
>
> Signed-off-by: Suma Hegde <suma.hegde@xxxxxxx>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@xxxxxxx>
> ---
Confirmed this should make no change to the logic
(with:
diff -u <(grep '^[-]' 05.patch | cut -b 2-) <(grep '^[+]' 05.patch | cut -b 2-)
).
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@xxxxxxxxxxxxxxx>
--
i.
> Changes since v3:
> 1. module init, exit code, probe, remove functions are kept back in hsmp.c
> not added to plat.c in this patch, as per review comment.
>
> Changes since v2:
> None
>
> Changes since v1:
> 1. Include new header file device.h in plat.c
> 2. Arrange headers in alphabetical order
> 3. Add an empty line between asm/ and linux/ headers
>
> drivers/platform/x86/amd/hsmp/Makefile | 2 +-
> drivers/platform/x86/amd/hsmp/hsmp.c | 138 ++-----------------------
> drivers/platform/x86/amd/hsmp/hsmp.h | 14 +++
> drivers/platform/x86/amd/hsmp/plat.c | 136 ++++++++++++++++++++++++
> 4 files changed, 161 insertions(+), 129 deletions(-)
> create mode 100644 drivers/platform/x86/amd/hsmp/plat.c
>
> diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile
> index fda64906a5e8..fb8ba04b2f0d 100644
> --- a/drivers/platform/x86/amd/hsmp/Makefile
> +++ b/drivers/platform/x86/amd/hsmp/Makefile
> @@ -5,4 +5,4 @@
> #
>
> obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
> -amd_hsmp-objs := hsmp.o
> +amd_hsmp-objs := hsmp.o plat.o
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.c b/drivers/platform/x86/amd/hsmp/hsmp.c
> index 9ed1fc0aeb00..6da7c6189020 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.c
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.c
> @@ -16,7 +16,6 @@
> #include <linux/delay.h>
> #include <linux/device.h>
> #include <linux/module.h>
> -#include <linux/pci.h>
> #include <linux/platform_device.h>
> #include <linux/semaphore.h>
> #include <linux/sysfs.h>
> @@ -40,45 +39,12 @@
> #define HSMP_WR true
> #define HSMP_RD false
>
> -/*
> - * To access specific HSMP mailbox register, s/w writes the SMN address of HSMP mailbox
> - * register into the SMN_INDEX register, and reads/writes the SMN_DATA reg.
> - * Below are required SMN address for HSMP Mailbox register offsets in SMU address space
> - */
> -#define SMN_HSMP_BASE 0x3B00000
> -#define SMN_HSMP_MSG_ID 0x0010534
> -#define SMN_HSMP_MSG_ID_F1A_M0H 0x0010934
> -#define SMN_HSMP_MSG_RESP 0x0010980
> -#define SMN_HSMP_MSG_DATA 0x00109E0
> -
> -#define HSMP_INDEX_REG 0xc4
> -#define HSMP_DATA_REG 0xc8
> -
> /* These are the strings specified in ACPI table */
> #define MSG_IDOFF_STR "MsgIdOffset"
> #define MSG_ARGOFF_STR "MsgArgOffset"
> #define MSG_RESPOFF_STR "MsgRspOffset"
>
> -static struct hsmp_plat_device plat_dev;
> -
> -static int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset,
> - u32 *value, bool write)
> -{
> - int ret;
> -
> - if (!sock->root)
> - return -ENODEV;
> -
> - ret = pci_write_config_dword(sock->root, HSMP_INDEX_REG,
> - sock->mbinfo.base_addr + offset);
> - if (ret)
> - return ret;
> -
> - ret = (write ? pci_write_config_dword(sock->root, HSMP_DATA_REG, *value)
> - : pci_read_config_dword(sock->root, HSMP_DATA_REG, value));
> -
> - return ret;
> -}
> +struct hsmp_plat_device plat_dev;
>
> static int amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
> u32 *value, bool write)
> @@ -248,7 +214,7 @@ int hsmp_send_message(struct hsmp_message *msg)
> }
> EXPORT_SYMBOL_GPL(hsmp_send_message);
>
> -static int hsmp_test(u16 sock_ind, u32 value)
> +int hsmp_test(u16 sock_ind, u32 value)
> {
> struct hsmp_message msg = { 0 };
> int ret;
> @@ -516,9 +482,9 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
> return hsmp_read_acpi_dsd(sock);
> }
>
> -static 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)
> +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 };
> @@ -577,8 +543,8 @@ static int hsmp_get_tbl_dram_base(u16 sock_ind)
> return 0;
> }
>
> -static umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
> - struct bin_attribute *battr, int id)
> +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;
> @@ -607,8 +573,8 @@ static int hsmp_init_metric_tbl_bin_attr(struct bin_attribute **hattrs, u16 sock
> /* One bin sysfs for metrics table */
> #define NUM_HSMP_ATTRS 1
>
> -static int hsmp_create_attr_list(struct attribute_group *attr_grp,
> - struct device *dev, u16 sock_ind)
> +int hsmp_create_attr_list(struct attribute_group *attr_grp,
> + struct device *dev, u16 sock_ind)
> {
> struct bin_attribute **hsmp_bin_attrs;
>
> @@ -624,36 +590,6 @@ static int hsmp_create_attr_list(struct attribute_group *attr_grp,
> return hsmp_init_metric_tbl_bin_attr(hsmp_bin_attrs, sock_ind);
> }
>
> -static int hsmp_create_non_acpi_sysfs_if(struct device *dev)
> -{
> - const struct attribute_group **hsmp_attr_grps;
> - struct attribute_group *attr_grp;
> - u16 i;
> -
> - hsmp_attr_grps = devm_kcalloc(dev, plat_dev.num_sockets + 1,
> - sizeof(*hsmp_attr_grps),
> - GFP_KERNEL);
> - if (!hsmp_attr_grps)
> - return -ENOMEM;
> -
> - /* Create a sysfs directory for each socket */
> - for (i = 0; i < plat_dev.num_sockets; i++) {
> - attr_grp = devm_kzalloc(dev, sizeof(struct attribute_group),
> - GFP_KERNEL);
> - if (!attr_grp)
> - return -ENOMEM;
> -
> - snprintf(plat_dev.sock[i].name, HSMP_ATTR_GRP_NAME_SIZE, "socket%u", (u8)i);
> - attr_grp->name = plat_dev.sock[i].name;
> - attr_grp->is_bin_visible = hsmp_is_sock_attr_visible;
> - hsmp_attr_grps[i] = attr_grp;
> -
> - hsmp_create_attr_list(attr_grp, dev, i);
> - }
> -
> - return device_add_groups(dev, hsmp_attr_grps);
> -}
> -
> static int hsmp_create_acpi_sysfs_if(struct device *dev)
> {
> struct attribute_group *attr_grp;
> @@ -677,7 +613,7 @@ static int hsmp_create_acpi_sysfs_if(struct device *dev)
> return devm_device_add_group(dev, attr_grp);
> }
>
> -static int hsmp_cache_proto_ver(u16 sock_ind)
> +int hsmp_cache_proto_ver(u16 sock_ind)
> {
> struct hsmp_message msg = { 0 };
> int ret;
> @@ -693,60 +629,6 @@ static int hsmp_cache_proto_ver(u16 sock_ind)
> return ret;
> }
>
> -static inline bool is_f1a_m0h(void)
> -{
> - if (boot_cpu_data.x86 == 0x1A && boot_cpu_data.x86_model <= 0x0F)
> - return true;
> -
> - return false;
> -}
> -
> -static int init_platform_device(struct device *dev)
> -{
> - struct hsmp_socket *sock;
> - int ret, i;
> -
> - for (i = 0; i < plat_dev.num_sockets; i++) {
> - if (!node_to_amd_nb(i))
> - return -ENODEV;
> - sock = &plat_dev.sock[i];
> - sock->root = node_to_amd_nb(i)->root;
> - sock->sock_ind = i;
> - sock->dev = dev;
> - sock->mbinfo.base_addr = SMN_HSMP_BASE;
> - sock->amd_hsmp_rdwr = amd_hsmp_pci_rdwr;
> -
> - /*
> - * This is a transitional change from non-ACPI to ACPI, only
> - * family 0x1A, model 0x00 platform is supported for both ACPI and non-ACPI.
> - */
> - if (is_f1a_m0h())
> - sock->mbinfo.msg_id_off = SMN_HSMP_MSG_ID_F1A_M0H;
> - else
> - sock->mbinfo.msg_id_off = SMN_HSMP_MSG_ID;
> -
> - sock->mbinfo.msg_resp_off = SMN_HSMP_MSG_RESP;
> - sock->mbinfo.msg_arg_off = SMN_HSMP_MSG_DATA;
> - sema_init(&sock->hsmp_sem, 1);
> -
> - /* Test the hsmp interface on each socket */
> - ret = hsmp_test(i, 0xDEADBEEF);
> - if (ret) {
> - dev_err(dev, "HSMP test message failed on Fam:%x model:%x\n",
> - boot_cpu_data.x86, boot_cpu_data.x86_model);
> - dev_err(dev, "Is HSMP disabled in BIOS ?\n");
> - return ret;
> - }
> - ret = hsmp_cache_proto_ver(i);
> - if (ret) {
> - dev_err(dev, "Failed to read HSMP protocol version\n");
> - return ret;
> - }
> - }
> -
> - return 0;
> -}
> -
> static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
> {ACPI_HSMP_DEVICE_HID, 0},
> {}
> diff --git a/drivers/platform/x86/amd/hsmp/hsmp.h b/drivers/platform/x86/amd/hsmp/hsmp.h
> index d54b9681d514..d59a9efb4799 100644
> --- a/drivers/platform/x86/amd/hsmp/hsmp.h
> +++ b/drivers/platform/x86/amd/hsmp/hsmp.h
> @@ -55,4 +55,18 @@ struct hsmp_plat_device {
> bool is_acpi_device;
> bool is_probed;
> };
> +
> +extern struct hsmp_plat_device plat_dev;
> +
> +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);
> +int hsmp_create_non_acpi_sysfs_if(struct device *dev);
> +int hsmp_cache_proto_ver(u16 sock_ind);
> +umode_t hsmp_is_sock_attr_visible(struct kobject *kobj,
> + struct bin_attribute *battr, int id);
> +int hsmp_create_attr_list(struct attribute_group *attr_grp,
> + struct device *dev, u16 sock_ind);
> +int hsmp_test(u16 sock_ind, u32 value);
> +int init_platform_device(struct device *dev);
> #endif /* HSMP_H */
> diff --git a/drivers/platform/x86/amd/hsmp/plat.c b/drivers/platform/x86/amd/hsmp/plat.c
> new file mode 100644
> index 000000000000..85a104859acd
> --- /dev/null
> +++ b/drivers/platform/x86/amd/hsmp/plat.c
> @@ -0,0 +1,136 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * AMD HSMP Platform Driver
> + * Copyright (c) 2024, AMD.
> + * All Rights Reserved.
> + *
> + * This file provides platform device implementations.
> + */
> +
> +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> +
> +#include <asm/amd_nb.h>
> +
> +#include <linux/device.h>
> +#include <linux/pci.h>
> +#include <linux/sysfs.h>
> +
> +#include "hsmp.h"
> +
> +/*
> + * To access specific HSMP mailbox register, s/w writes the SMN address of HSMP mailbox
> + * register into the SMN_INDEX register, and reads/writes the SMN_DATA reg.
> + * Below are required SMN address for HSMP Mailbox register offsets in SMU address space
> + */
> +#define SMN_HSMP_BASE 0x3B00000
> +#define SMN_HSMP_MSG_ID 0x0010534
> +#define SMN_HSMP_MSG_ID_F1A_M0H 0x0010934
> +#define SMN_HSMP_MSG_RESP 0x0010980
> +#define SMN_HSMP_MSG_DATA 0x00109E0
> +
> +#define HSMP_INDEX_REG 0xc4
> +#define HSMP_DATA_REG 0xc8
> +
> +static int amd_hsmp_pci_rdwr(struct hsmp_socket *sock, u32 offset,
> + u32 *value, bool write)
> +{
> + int ret;
> +
> + if (!sock->root)
> + return -ENODEV;
> +
> + ret = pci_write_config_dword(sock->root, HSMP_INDEX_REG,
> + sock->mbinfo.base_addr + offset);
> + if (ret)
> + return ret;
> +
> + ret = (write ? pci_write_config_dword(sock->root, HSMP_DATA_REG, *value)
> + : pci_read_config_dword(sock->root, HSMP_DATA_REG, value));
> +
> + return ret;
> +}
> +
> +int hsmp_create_non_acpi_sysfs_if(struct device *dev)
> +{
> + const struct attribute_group **hsmp_attr_grps;
> + struct attribute_group *attr_grp;
> + u16 i;
> +
> + hsmp_attr_grps = devm_kcalloc(dev, plat_dev.num_sockets + 1,
> + sizeof(*hsmp_attr_grps),
> + GFP_KERNEL);
> + if (!hsmp_attr_grps)
> + return -ENOMEM;
> +
> + /* Create a sysfs directory for each socket */
> + for (i = 0; i < plat_dev.num_sockets; i++) {
> + attr_grp = devm_kzalloc(dev, sizeof(struct attribute_group),
> + GFP_KERNEL);
> + if (!attr_grp)
> + return -ENOMEM;
> +
> + snprintf(plat_dev.sock[i].name, HSMP_ATTR_GRP_NAME_SIZE, "socket%u", (u8)i);
> + attr_grp->name = plat_dev.sock[i].name;
> + attr_grp->is_bin_visible = hsmp_is_sock_attr_visible;
> + hsmp_attr_grps[i] = attr_grp;
> +
> + hsmp_create_attr_list(attr_grp, dev, i);
> + }
> +
> + return device_add_groups(dev, hsmp_attr_grps);
> +}
> +
> +static inline bool is_f1a_m0h(void)
> +{
> + if (boot_cpu_data.x86 == 0x1A && boot_cpu_data.x86_model <= 0x0F)
> + return true;
> +
> + return false;
> +}
> +
> +int init_platform_device(struct device *dev)
> +{
> + struct hsmp_socket *sock;
> + int ret, i;
> +
> + for (i = 0; i < plat_dev.num_sockets; i++) {
> + if (!node_to_amd_nb(i))
> + return -ENODEV;
> + sock = &plat_dev.sock[i];
> + sock->root = node_to_amd_nb(i)->root;
> + sock->sock_ind = i;
> + sock->dev = dev;
> + sock->mbinfo.base_addr = SMN_HSMP_BASE;
> + sock->amd_hsmp_rdwr = amd_hsmp_pci_rdwr;
> +
> + /*
> + * This is a transitional change from non-ACPI to ACPI, only
> + * family 0x1A, model 0x00 platform is supported for both ACPI and non-ACPI.
> + */
> + if (is_f1a_m0h())
> + sock->mbinfo.msg_id_off = SMN_HSMP_MSG_ID_F1A_M0H;
> + else
> + sock->mbinfo.msg_id_off = SMN_HSMP_MSG_ID;
> +
> + sock->mbinfo.msg_resp_off = SMN_HSMP_MSG_RESP;
> + sock->mbinfo.msg_arg_off = SMN_HSMP_MSG_DATA;
> + sema_init(&sock->hsmp_sem, 1);
> +
> + /* Test the hsmp interface on each socket */
> + ret = hsmp_test(i, 0xDEADBEEF);
> + if (ret) {
> + dev_err(dev, "HSMP test message failed on Fam:%x model:%x\n",
> + boot_cpu_data.x86, boot_cpu_data.x86_model);
> + dev_err(dev, "Is HSMP disabled in BIOS ?\n");
> + return ret;
> + }
> +
> + ret = hsmp_cache_proto_ver(i);
> + if (ret) {
> + dev_err(dev, "Failed to read HSMP protocol version\n");
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
>
