Re: [v2 07/11] platform/x86/amd/hsmp: Create mutually exclusive ACPI and plat drivers

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

On Thu, 11 Jul 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@amd.com>
> Reviewed-by: Naveen Krishna Chatradhi <naveenkrishna.chatradhi@amd.com>
> ---
> Changes since v1:
> Rename "plat_dev" to "hsmp_pdev"
> 
>  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   | 119 ++++++++++++++++++++++--
>  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, 186 insertions(+), 125 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..15888a89581a 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=n
>  	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

I still don't agree with this approach. You can add another config for the 
hsmp core and select it, and it will work with the existing configs when 
they're properly run through make oldconfig or olddefconfig before 
building.

-- 
 i.