On Thu, 27 Jun 2024, Suma Hegde wrote:
> Separate the probes for ACPI and platform device drivers.
> Provide a Kconfig option to select either the
> ACPI or the platform device based driver.
>
> Signed-off-by: Suma Hegde <suma.hegde@xxxxxxx>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@xxxxxxx>
> ---
> arch/x86/include/asm/amd_hsmp.h | 2 +-
> drivers/platform/x86/amd/hsmp/Kconfig | 25 ++++-
> drivers/platform/x86/amd/hsmp/Makefile | 10 +-
> drivers/platform/x86/amd/hsmp/acpi.c | 114 ++++++++++++++++++++++-
> drivers/platform/x86/amd/hsmp/hsmp.c | 25 ++---
> drivers/platform/x86/amd/hsmp/hsmp.h | 8 +-
> drivers/platform/x86/amd/hsmp/plat.c | 122 +++++++------------------
> 7 files changed, 183 insertions(+), 123 deletions(-)
>
> diff --git a/arch/x86/include/asm/amd_hsmp.h b/arch/x86/include/asm/amd_hsmp.h
> index 03c2ce3edaf5..ada14e55f9f4 100644
> --- a/arch/x86/include/asm/amd_hsmp.h
> +++ b/arch/x86/include/asm/amd_hsmp.h
> @@ -5,7 +5,7 @@
>
> #include <uapi/asm/amd_hsmp.h>
>
> -#if IS_ENABLED(CONFIG_AMD_HSMP)
> +#if IS_ENABLED(CONFIG_AMD_HSMP) || IS_ENABLED(CONFIG_AMD_HSMP_ACPI)
> int hsmp_send_message(struct hsmp_message *msg);
> #else
> static inline int hsmp_send_message(struct hsmp_message *msg)
> diff --git a/drivers/platform/x86/amd/hsmp/Kconfig b/drivers/platform/x86/amd/hsmp/Kconfig
> index b55d4ed9bceb..1cb10d2aac77 100644
> --- a/drivers/platform/x86/amd/hsmp/Kconfig
> +++ b/drivers/platform/x86/amd/hsmp/Kconfig
> @@ -3,9 +3,30 @@
> # AMD HSMP Driver
> #
>
> +menu "AMD Host System Management Port driver"
> + depends on AMD_NB
> +
> +config AMD_HSMP_ACPI
> + tristate "AMD HSMP ACPI driver"
> + depends on ACPI
> + help
> + The driver provides a way for user space tools to monitor and manage
> + system management functionality on EPYC server CPUs from AMD.
> +
> + Host System Management Port (HSMP) interface is a mailbox interface
> + between the x86 core and the System Management Unit (SMU) firmware.
> +
> + This driver supports ACPI based probing.
> +
> + You may enable this, if your platform bios provides an ACPI object
> + as described in the documentation.
> +
> + If you choose to compile this driver as a module the module will be
> + called amd_hsmp.
> +
> config AMD_HSMP
> tristate "AMD HSMP Driver"
> - depends on AMD_NB && X86_64 && ACPI
> + depends on !(AMD_HSMP_ACPI || AMD_HSMP_ACPI=m)
depends on AMD_HSMP_ACPI=n is simpler.
> help
> The driver provides a way for user space tools to monitor and manage
> system management functionality on EPYC server CPUs from AMD.
> @@ -15,3 +36,5 @@ config AMD_HSMP
>
> If you choose to compile this driver as a module the module will be
> called amd_hsmp.
> +
> +endmenu
> diff --git a/drivers/platform/x86/amd/hsmp/Makefile b/drivers/platform/x86/amd/hsmp/Makefile
> index 0cc92865c0a2..53ebc462b0f9 100644
> --- a/drivers/platform/x86/amd/hsmp/Makefile
> +++ b/drivers/platform/x86/amd/hsmp/Makefile
> @@ -4,5 +4,11 @@
> # AMD HSMP Driver
> #
>
> -obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
> -amd_hsmp-objs := hsmp.o plat.o acpi.o
> +ifneq ($(CONFIG_AMD_HSMP), )
> +obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
> +amd_hsmp-objs = hsmp.o plat.o
> +endif
> +ifneq ($(CONFIG_AMD_HSMP_ACPI), )
> +obj-$(CONFIG_AMD_HSMP_ACPI) += amd_hsmp.o
> +amd_hsmp-objs = hsmp.o acpi.o
> +endif
You should add a third symbol for the core that is selected to make
the Makefile side less convoluted.
The build system might work with something as simple as this (I'm not sure
if ...-m postfix works for composite objects, ...-y is documented to
work):
obj-$(CONFIG_xx) += amd_hsmp.o
amd_hsmp-$(CONFIG_xx) = hsmp.o
amd_hsmp-$(CONFIG_AMD_HSMP) += plat.o
amd_hsmp-$(CONFIG_AMD_HSMP_ACPI) += acpi.o
> diff --git a/drivers/platform/x86/amd/hsmp/acpi.c b/drivers/platform/x86/amd/hsmp/acpi.c
> index 90bfa1ddadbf..0307f4e7176d 100644
> --- a/drivers/platform/x86/amd/hsmp/acpi.c
> +++ b/drivers/platform/x86/amd/hsmp/acpi.c
> @@ -12,21 +12,34 @@
> #include "hsmp.h"
>
> #include <linux/acpi.h>
> +#include <asm/amd_nb.h>
> +#include <linux/platform_device.h>
> +
> +#define DRIVER_NAME "amd_hsmp"
> +#define DRIVER_VERSION "2.3"
> +#define ACPI_HSMP_DEVICE_HID "AMDI0097"
>
> /* These are the strings specified in ACPI table */
> #define MSG_IDOFF_STR "MsgIdOffset"
> #define MSG_ARGOFF_STR "MsgArgOffset"
> #define MSG_RESPOFF_STR "MsgRspOffset"
>
> -void amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
> - u32 *value, bool write)
> +static int amd_hsmp_acpi_rdwr(struct hsmp_socket *sock, u32 offset,
> + u32 *value, bool write)
> {
> if (write)
> iowrite32(*value, sock->virt_base_addr + offset);
> else
> *value = ioread32(sock->virt_base_addr + offset);
> + return 0;
> }
>
> +static const struct file_operations hsmp_fops = {
> + .owner = THIS_MODULE,
> + .unlocked_ioctl = hsmp_ioctl,
> + .compat_ioctl = hsmp_ioctl,
> +};
> +
> /* This is the UUID used for HSMP */
> static const guid_t acpi_hsmp_uuid = GUID_INIT(0xb74d619d, 0x5707, 0x48bd,
> 0xa6, 0x9f, 0x4e, 0xa2,
> @@ -189,7 +202,7 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
>
> sock->sock_ind = sock_ind;
> sock->dev = dev;
> - plat_dev.is_acpi_device = true;
> + sock->amd_hsmp_rdwr = amd_hsmp_acpi_rdwr;
>
> sema_init(&sock->hsmp_sem, 1);
>
> @@ -202,7 +215,7 @@ static int hsmp_parse_acpi_table(struct device *dev, u16 sock_ind)
> return hsmp_read_acpi_dsd(sock);
> }
>
> -int hsmp_create_acpi_sysfs_if(struct device *dev)
> +static int hsmp_create_acpi_sysfs_if(struct device *dev)
> {
> struct attribute_group *attr_grp;
> u16 sock_ind;
> @@ -225,7 +238,7 @@ int hsmp_create_acpi_sysfs_if(struct device *dev)
> return devm_device_add_group(dev, attr_grp);
> }
>
> -int init_acpi(struct device *dev)
> +static int init_acpi(struct device *dev)
> {
> u16 sock_ind;
> int ret;
> @@ -259,3 +272,94 @@ int init_acpi(struct device *dev)
>
> return ret;
> }
> +
> +static const struct acpi_device_id amd_hsmp_acpi_ids[] = {
> + {ACPI_HSMP_DEVICE_HID, 0},
> + {}
> +};
> +MODULE_DEVICE_TABLE(acpi, amd_hsmp_acpi_ids);
> +
> +static bool check_acpi_support(struct device *dev)
> +{
> + struct acpi_device *adev = ACPI_COMPANION(dev);
> +
> + if (adev && !acpi_match_device_ids(adev, amd_hsmp_acpi_ids))
> + return true;
> +
> + return false;
> +}
> +
> +static int hsmp_acpi_probe(struct platform_device *pdev)
> +{
> + int ret;
> +
> + if (!plat_dev.is_probed) {
> + plat_dev.num_sockets = amd_nb_num();
> + if (plat_dev.num_sockets == 0 || plat_dev.num_sockets > MAX_AMD_SOCKETS)
> + return -ENODEV;
> +
> + plat_dev.sock = devm_kcalloc(&pdev->dev, plat_dev.num_sockets,
> + sizeof(*plat_dev.sock),
> + GFP_KERNEL);
> + if (!plat_dev.sock)
> + return -ENOMEM;
> + }
> +
> + if (!check_acpi_support(&pdev->dev)) {
> + dev_err(&pdev->dev, "Not ACPI device?\n");
> + return -ENODEV;
> + }
> +
> + ret = init_acpi(&pdev->dev);
> + if (ret) {
> + dev_err(&pdev->dev, "Failed to initialize HSMP interface.\n");
> + return ret;
> + }
> +
> + ret = hsmp_create_acpi_sysfs_if(&pdev->dev);
> + if (ret)
> + dev_err(&pdev->dev, "Failed to create HSMP sysfs interface\n");
> +
> + if (!plat_dev.is_probed) {
> + plat_dev.hsmp_device.name = HSMP_CDEV_NAME;
> + plat_dev.hsmp_device.minor = MISC_DYNAMIC_MINOR;
> + plat_dev.hsmp_device.fops = &hsmp_fops;
> + plat_dev.hsmp_device.parent = &pdev->dev;
> + plat_dev.hsmp_device.nodename = HSMP_DEVNODE_NAME;
> + plat_dev.hsmp_device.mode = 0644;
> +
> + ret = misc_register(&plat_dev.hsmp_device);
> + if (ret)
> + return ret;
> + plat_dev.is_probed = true;
> + }
> +
> + return 0;
> +}
> +
> +static void hsmp_acpi_remove(struct platform_device *pdev)
> +{
> + /*
> + * We register only one misc_device even on multi socket system.
multi-socket
> + * So, deregister should happen only once.
> + */
> + if (plat_dev.is_probed) {
> + misc_deregister(&plat_dev.hsmp_device);
> + plat_dev.is_probed = false;
> + }
> +}
> +
> +static struct platform_driver amd_hsmp_driver = {
> + .probe = hsmp_acpi_probe,
> + .remove_new = hsmp_acpi_remove,
> + .driver = {
> + .name = DRIVER_NAME,
> + .acpi_match_table = amd_hsmp_acpi_ids,
> + },
> +};
> +
> +module_platform_driver(amd_hsmp_driver);
> +
> +MODULE_DESCRIPTION("AMD HSMP Platform Interface Driver");
> +MODULE_VERSION(DRIVER_VERSION);
> +MODULE_LICENSE("GPL v2");
--
i.
