[PATCH] platform/x86/amd: Don't allow HSMP=y and PMC to be enabled together

[PATCH] platform/x86/amd: Don't allow HSMP=y and PMC to be enabled together

[Mario Limonciello]

If the HSMP driver is compiled into the kernel it can cause problems
on systems that support PMC since it probes hardware it shouldn't.

Prevent users from compiling HSMP as built-in when PMC is enabled.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2414
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3285
Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
---
 drivers/platform/x86/amd/pmc/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/platform/x86/amd/pmc/Kconfig b/drivers/platform/x86/amd/pmc/Kconfig
index 883c0a95ac0c..d734f6698f97 100644
--- a/drivers/platform/x86/amd/pmc/Kconfig
+++ b/drivers/platform/x86/amd/pmc/Kconfig
@@ -7,6 +7,7 @@ config AMD_PMC
 	tristate "AMD SoC PMC driver"
 	depends on ACPI && PCI && RTC_CLASS && AMD_NB
 	depends on SUSPEND
+	depends on (AMD_HSMP = n) || (AMD_HSMP = m)
 	select SERIO
 	help
 	  The driver provides support for AMD Power Management Controller
-- 
2.34.1

Re: [PATCH] platform/x86/amd: Don't allow HSMP=y and PMC to be enabled together

[Kuppuswamy Sathyanarayanan]

On 4/10/24 7:10 AM, Mario Limonciello wrote:
> If the HSMP driver is compiled into the kernel it can cause problems
> on systems that support PMC since it probes hardware it shouldn't.
>
> Prevent users from compiling HSMP as built-in when PMC is enabled.
>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2414
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3285
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
> ---
>  drivers/platform/x86/amd/pmc/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/platform/x86/amd/pmc/Kconfig b/drivers/platform/x86/amd/pmc/Kconfig
> index 883c0a95ac0c..d734f6698f97 100644
> --- a/drivers/platform/x86/amd/pmc/Kconfig
> +++ b/drivers/platform/x86/amd/pmc/Kconfig
> @@ -7,6 +7,7 @@ config AMD_PMC
>  	tristate "AMD SoC PMC driver"
>  	depends on ACPI && PCI && RTC_CLASS && AMD_NB
>  	depends on SUSPEND
> +	depends on (AMD_HSMP = n) || (AMD_HSMP = m)

Can't you use depends on AMD_HSMP!=y ?

>  	select SERIO
>  	help
>  	  The driver provides support for AMD Power Management Controller

-- 
Sathyanarayanan Kuppuswamy
Linux Kernel Developer

Re: [PATCH] platform/x86/amd: Don't allow HSMP=y and PMC to be enabled together

[Hans de Goede]

Hi Mario,

On 4/10/24 4:10 PM, Mario Limonciello wrote:
> If the HSMP driver is compiled into the kernel it can cause problems
> on systems that support PMC since it probes hardware it shouldn't.
> 
> Prevent users from compiling HSMP as built-in when PMC is enabled.
> 
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2414
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/3285
> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>

Your commit message says "Prevent users from compiling HSMP as
built-in when PMC is enabled."

But what you are doing is preventing building PMC at all when
HSMP is builtin. Not sure if that is actually what you want,
but if it is then the commit msg needs some work.

Also I'm wondering how this actually fixes anything. Users
can still enable both as a module and then load both modules
at the same time?

So wouldn't it be better for both drivers to somehow see
the other driver is bound and then exit from probe() early on?

Typically how this works is that there is a "struct device"
representing the hw-block and then only one device driver
can be bound at the same time.

IIRC the PMC is somewhat special and does not really have
a "struct device" automatically instantiated for it by
the kernel. But there could still be some shared common
code which instantiates a platform_device for the pmc and
then both drivers could become platform drivers and the user
can switch between the 2 even if both are builtin by unbinding
one platform-driver through sysfs and then binding the other
through sysfs.

Regards,

Hans




> ---
>  drivers/platform/x86/amd/pmc/Kconfig | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/platform/x86/amd/pmc/Kconfig b/drivers/platform/x86/amd/pmc/Kconfig
> index 883c0a95ac0c..d734f6698f97 100644
> --- a/drivers/platform/x86/amd/pmc/Kconfig
> +++ b/drivers/platform/x86/amd/pmc/Kconfig
> @@ -7,6 +7,7 @@ config AMD_PMC
>  	tristate "AMD SoC PMC driver"
>  	depends on ACPI && PCI && RTC_CLASS && AMD_NB
>  	depends on SUSPEND
> +	depends on (AMD_HSMP = n) || (AMD_HSMP = m)
>  	select SERIO
>  	help
>  	  The driver provides support for AMD Power Management Controller