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

[Hans de Goede <hdegoede@redhat.com>]

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