Hi Hans,
On 10/6/2024 3:39 AM, Hans de Goede wrote:
Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding.
Hi Suma,
On 3-Oct-24 1:15 PM, Suma Hegde wrote:
Separate the probes for HSMP ACPI and platform device drivers.
Provide a Kconfig option to choose between ACPI or the platform device
based driver. The common code which is the core part of the HSMP driver
maintained at hsmp.c is guarded by AMD_HSMP config and is selected by
these two driver configs. This will be built into separate hsmp_common.ko
module and acpi as hsmp_acpi and plat as amd_hsmp respectively.
Also add "|| COMPILE_TEST" clause in Kconfig to get build coverage for
HSMP.
Signed-off-by: Suma Hegde <suma.hegde@xxxxxxx>
Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@xxxxxxx>
---
Changes since v8:
Removed unused hsmp_fops structure definition.
| Reported-by: kernel test robot <lkp@xxxxxxxxx>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410021345.okdVjjGy-lkp@xxxxxxxxx/
Changes since v7:
1. Commit description is updated.
2. Makefile is updated to create 3 modules, hsmp_common, hsmp_acpi, amd_hsmp.
3. hsmp.c is modified to make it as a module and functions used by acpi.c and plat.c
are made as export symbols (hsmp_test(), hsmp_cache_proto_ver() etc).
4. "depends on AMD_HSMP_ACPI=n" is removed from Kconfig.
5. Documentation is updated to reflect new changes.
6. amd_hsmp.h is modified to remove IS_ENABLED(CONFIG_AMD_HSMP_ACPI).
Thank you for the new version, the way the split is done now looks
good to me.
One small remark below (I have not done a full review only checked
the Kconfig / Makefile bits) :
<snip>
diff --git a/drivers/platform/x86/amd/hsmp/Kconfig b/drivers/platform/x86/amd/hsmp/Kconfig
index b55d4ed9bceb..dc64b5b1165c 100644
--- a/drivers/platform/x86/amd/hsmp/Kconfig
+++ b/drivers/platform/x86/amd/hsmp/Kconfig
@@ -4,14 +4,44 @@
#
config AMD_HSMP
- tristate "AMD HSMP Driver"
- depends on AMD_NB && X86_64 && ACPI
+ tristate
+
+menu "AMD HSMP Driver"
+ depends on AMD_NB || COMPILE_TEST
+
+config AMD_HSMP_ACPI
+ tristate "AMD HSMP ACPI device driver"
+ depends on ACPI
+ select AMD_HSMP
help
+ Host System Management Port (HSMP) interface is a mailbox interface
+ between the x86 core and the System Management Unit (SMU) firmware.
The driver provides a way for user space tools to monitor and manage
- system management functionality on EPYC server CPUs from AMD.
+ system management functionality on EPYC and MI300A server CPUs
+ from AMD.
+
+ This option supports ACPI based probing.
+ You may enable this, if your platform BIOS provides an ACPI object
+ as described in amd_hsmp.rst document.
+
+ If you choose to compile this driver as a module the module will be
+ called amd_hsmp.
This last line should be: "called hsmp_acpi.".
Please pardon me for this error. I will address it in v10 after your
full review.
+config AMD_HSMP_PLAT
+ tristate "AMD HSMP platform device driver"
+ select AMD_HSMP
+ help
Host System Management Port (HSMP) interface is a mailbox interface
between the x86 core and the System Management Unit (SMU) firmware.
+ The driver provides a way for user space tools to monitor and manage
+ system management functionality on EPYC and MI300A server CPUs
+ from AMD.
+
+ This option supports platform device based probing.
+ You may enable this, if your platform BIOS does not provide
+ HSMP ACPI object.
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..3175d8885e87 100644
--- a/drivers/platform/x86/amd/hsmp/Makefile
+++ b/drivers/platform/x86/amd/hsmp/Makefile
@@ -4,5 +4,9 @@
# AMD HSMP Driver
#
-obj-$(CONFIG_AMD_HSMP) += amd_hsmp.o
-amd_hsmp-objs := hsmp.o plat.o acpi.o
+obj-$(CONFIG_AMD_HSMP) += hsmp_common.o
+hsmp_common-objs := hsmp.o
+obj-$(CONFIG_AMD_HSMP_PLAT) += amd_hsmp.o
+amd_hsmp-objs := plat.o
+obj-$(CONFIG_AMD_HSMP_ACPI) += hsmp_acpi.o
+hsmp_acpi-objs := acpi.o
Regards,
Hans
Thanks and Regards,
Suma
