[PATCH] platform/x86: dell: Fix ALIENWARE_WMI dependencies

[PATCH] platform/x86: dell: Fix ALIENWARE_WMI dependencies

[Kurt Borja]

If ACPI_PLATFORM_PROFILE is selected by ALIENWARE_WMI_WMAX, the former
is forced to be at least =m, because the latter is a bool.

This allows the following config:

	CONFIG_ALIENWARE_WMI=y
	CONFIG_ACPI_PLATFORM_PROFILE=m

which causes a linking error, because ALIENWARE_WMI_WMAX is only used to
conditionally link alienware-wmi-wmax.c to the alienware-wmi kernel
object, which might not be a module.

Move the ACPI_PLATFORM_PROFILE reverse dependency to ALIENWARE_WMI to
fix this issue.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202503051819.bQ9P70Og-lkp@intel.com/
Fixes: b1b8fcf6e677 ("platform/x86: Update alienware-wmi config entries")
Signed-off-by: Kurt Borja <kuurtb@gmail.com>
---
 drivers/platform/x86/dell/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
index f8a0dffcaab7c3b423472c5b9093011334a698c8..87c2ceb12d48d610c85fa739abbc4f0d88e33ec8 100644
--- a/drivers/platform/x86/dell/Kconfig
+++ b/drivers/platform/x86/dell/Kconfig
@@ -22,6 +22,7 @@ config ALIENWARE_WMI
 	depends on DMI
 	depends on LEDS_CLASS
 	depends on NEW_LEDS
+	select ACPI_PLATFORM_PROFILE
 	help
 	 This is a driver for controlling Alienware WMI driven features.
 
@@ -43,7 +44,6 @@ config ALIENWARE_WMI_WMAX
 	bool "Alienware WMAX WMI device driver"
 	default y
 	depends on ALIENWARE_WMI
-	select ACPI_PLATFORM_PROFILE
 	help
 	 Alienware WMI driver with AlienFX LED, HDMI, amplifier, deep sleep and
 	 AWCC thermal control capabilities.

---
base-commit: e57eabe2fb044950e6ffdfe01803895043dec0b7
change-id: 20250309-dell-kconfig-fix-4d3daedaedbf

Best regards,
-- 
 ~ Kurt

Re: [PATCH] platform/x86: dell: Fix ALIENWARE_WMI dependencies

[Ilpo Järvinen]

On Sun, 9 Mar 2025, Kurt Borja wrote:

> If ACPI_PLATFORM_PROFILE is selected by ALIENWARE_WMI_WMAX, the former
> is forced to be at least =m, because the latter is a bool.
> 
> This allows the following config:
> 
> 	CONFIG_ALIENWARE_WMI=y
> 	CONFIG_ACPI_PLATFORM_PROFILE=m

Hi,

selecting from =y should not result in =m for the other symbol. This is 
a bug in Kconfig infrastructure.

I ran across this a few years back and even had a test case to prove the 
select bug but back then the original problem eventually was solved in a 
different way which no longer hit the problem. I never could figure out
how to fix the kconfig logic though without breaking something and it 
ended up into low priority bin and never got solved.

Sadly, it seems I've lost the test case patch that exhibits the bug 
somewhere... I'll try to look for it from my archived files.

> which causes a linking error, because ALIENWARE_WMI_WMAX is only used to
> conditionally link alienware-wmi-wmax.c to the alienware-wmi kernel
> object, which might not be a module.
> 
> Move the ACPI_PLATFORM_PROFILE reverse dependency to ALIENWARE_WMI to
> fix this issue.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202503051819.bQ9P70Og-lkp@intel.com/
> Fixes: b1b8fcf6e677 ("platform/x86: Update alienware-wmi config entries")
> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
> ---
>  drivers/platform/x86/dell/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
> index f8a0dffcaab7c3b423472c5b9093011334a698c8..87c2ceb12d48d610c85fa739abbc4f0d88e33ec8 100644
> --- a/drivers/platform/x86/dell/Kconfig
> +++ b/drivers/platform/x86/dell/Kconfig
> @@ -22,6 +22,7 @@ config ALIENWARE_WMI
>  	depends on DMI
>  	depends on LEDS_CLASS
>  	depends on NEW_LEDS
> +	select ACPI_PLATFORM_PROFILE
>  	help
>  	 This is a driver for controlling Alienware WMI driven features.
>  
> @@ -43,7 +44,6 @@ config ALIENWARE_WMI_WMAX
>  	bool "Alienware WMAX WMI device driver"
>  	default y
>  	depends on ALIENWARE_WMI
> -	select ACPI_PLATFORM_PROFILE
>  	help
>  	 Alienware WMI driver with AlienFX LED, HDMI, amplifier, deep sleep and
>  	 AWCC thermal control capabilities.
> 
> ---
> base-commit: e57eabe2fb044950e6ffdfe01803895043dec0b7
> change-id: 20250309-dell-kconfig-fix-4d3daedaedbf
> 
> Best regards,
> 

-- 
 i.

Re: [PATCH] platform/x86: dell: Fix ALIENWARE_WMI dependencies

[Kurt Borja]

Hi Ilpo,

On Mon Mar 10, 2025 at 9:29 AM -05, Ilpo Järvinen wrote:
> On Sun, 9 Mar 2025, Kurt Borja wrote:
>
>> If ACPI_PLATFORM_PROFILE is selected by ALIENWARE_WMI_WMAX, the former
>> is forced to be at least =m, because the latter is a bool.
>> 
>> This allows the following config:
>> 
>> 	CONFIG_ALIENWARE_WMI=y
>> 	CONFIG_ACPI_PLATFORM_PROFILE=m
>
> Hi,
>
> selecting from =y should not result in =m for the other symbol. This is 
> a bug in Kconfig infrastructure.
>
> I ran across this a few years back and even had a test case to prove the 
> select bug but back then the original problem eventually was solved in a 
> different way which no longer hit the problem. I never could figure out
> how to fix the kconfig logic though without breaking something and it 
> ended up into low priority bin and never got solved.
>
> Sadly, it seems I've lost the test case patch that exhibits the bug 
> somewhere... I'll try to look for it from my archived files.

That's funny.

I thought this was a Kconfig quirk, that resulted from the following
hierarchy:

Type		0	1	2
=============== ======= ======= =======
Bool		n	y
Tristate	n	m	y

So a <bool> selecting the <tristate> would force it to be at least =m.

The same thing happens with depend, where a dependecy would be fulfilled
for a <bool> if a <tristate> was at least =m. That's why in the kernel
robot report the linking error was also due to the HWMON dependency.

Anyway, this patch could serve as a workaround if you feel it's
necessary. I'm going to put the HWMON dependecy in the ALIENWARE_WMI
symbol for my other series.

-- 
 ~ Kurt

Re: [PATCH] platform/x86: dell: Fix ALIENWARE_WMI dependencies

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2829 bytes --]

Hi Kbuild/config people,

Could you please take a look at this select problem.

I attempted to solve this kconfig select logic problem a few years back 
but IIRC, my solution leaked memory or had some other problem I could not 
find solution to (and now the code has changed enough I couldn't even get 
my buggy solution easily forward-ported so I just dropped the solution and 
left just the test case into the patch).

As the Kconfig problem back then got resolved through other means, I never 
mentioned this back then but seems the same problem likely happens here
in some other form (bool selecting tristate that ends up getting only =m).

On Mon, 10 Mar 2025, Kurt Borja wrote:
> On Mon Mar 10, 2025 at 9:29 AM -05, Ilpo Järvinen wrote:
> > On Sun, 9 Mar 2025, Kurt Borja wrote:
> >
> >> If ACPI_PLATFORM_PROFILE is selected by ALIENWARE_WMI_WMAX, the former
> >> is forced to be at least =m, because the latter is a bool.
> >> 
> >> This allows the following config:
> >> 
> >> 	CONFIG_ALIENWARE_WMI=y
> >> 	CONFIG_ACPI_PLATFORM_PROFILE=m
> >
> > Hi,
> >
> > selecting from =y should not result in =m for the other symbol. This is 
> > a bug in Kconfig infrastructure.
> >
> > I ran across this a few years back and even had a test case to prove the 
> > select bug but back then the original problem eventually was solved in a 
> > different way which no longer hit the problem. I never could figure out
> > how to fix the kconfig logic though without breaking something and it 
> > ended up into low priority bin and never got solved.
> >
> > Sadly, it seems I've lost the test case patch that exhibits the bug 
> > somewhere... I'll try to look for it from my archived files.
> 
> That's funny.
> 
> I thought this was a Kconfig quirk, that resulted from the following
> hierarchy:
> 
> Type		0	1	2
> =============== ======= ======= =======
> Bool		n	y

I think y should be 2 in both cases so select should cause the selected 
symbol to becomes =y but there's a logic problem deep in select logic in 
the kconfig code.

I've attached the multi-select based reproducer test case. make 
testconfig runs kconfig selftests and results in CONFIG_A3=m despite 
CONFIG_C3=y selecting it.

Please do not apply the test case before the problem is fixed.

> Tristate	n	m	y
> 
> So a <bool> selecting the <tristate> would force it to be at least =m.
> 
> The same thing happens with depend, where a dependecy would be fulfilled
> for a <bool> if a <tristate> was at least =m. That's why in the kernel
> robot report the linking error was also due to the HWMON dependency.
> 
> Anyway, this patch could serve as a workaround if you feel it's
> necessary. I'm going to put the HWMON dependecy in the ALIENWARE_WMI
> symbol for my other series.
> 
> 

-- 
 i.

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-diff; name=select_bug.patch, Size: 3160 bytes --]

From: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Subject: [PATCH] kconfig: Test that the largest of selects is taken

DO NOT APPLY WITHOUT FIXING KCONFIG CODE FIRST!!!

With dual select, the largest selection should be taken. Test with
indepedent selectors and ones which depends on each other.

Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>

---
 scripts/kconfig/tests/dual_select/Kconfig         | 40 +++++++++++++++++++++++
 scripts/kconfig/tests/dual_select/__init__.py     |  8 +++++
 scripts/kconfig/tests/dual_select/defconfig       |  7 ++++
 scripts/kconfig/tests/dual_select/expected_config | 10 ++++++
 4 files changed, 65 insertions(+)
 create mode 100644 scripts/kconfig/tests/dual_select/Kconfig
 create mode 100644 scripts/kconfig/tests/dual_select/__init__.py
 create mode 100644 scripts/kconfig/tests/dual_select/defconfig
 create mode 100644 scripts/kconfig/tests/dual_select/expected_config

diff --git a/scripts/kconfig/tests/dual_select/Kconfig b/scripts/kconfig/tests/dual_select/Kconfig
new file mode 100644
index 000000000000..776ddc4e8bf9
--- /dev/null
+++ b/scripts/kconfig/tests/dual_select/Kconfig
@@ -0,0 +1,40 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config MODULES
+	bool "Enable loadable module support"
+	modules
+	default y
+
+config A1
+	tristate "A1"
+# indepedendent tristate selects tristate
+config B1
+	tristate "B1"
+	select A1
+# bool selects lib
+config C1
+	bool "C1"
+	select A1
+
+config A2
+	tristate "A2"
+config B2
+	tristate "B2"
+	select A2
+# tristate depends on tristate and selects tristate
+config C2
+	tristate "C2"
+	depends on B2
+	select A2
+
+
+config A3
+	tristate "A3"
+config B3
+	tristate "B3"
+	select A3
+# bool depends on tristate and selects tristate
+config C3
+	bool "C3"
+	depends on B3
+	select A3
diff --git a/scripts/kconfig/tests/dual_select/__init__.py b/scripts/kconfig/tests/dual_select/__init__.py
new file mode 100644
index 000000000000..613d801014e8
--- /dev/null
+++ b/scripts/kconfig/tests/dual_select/__init__.py
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+"""
+Test dual select should select the largest selection.
+"""
+
+def test(conf):
+    assert conf.defconfig('defconfig') == 0
+    assert conf.config_contains('expected_config')
diff --git a/scripts/kconfig/tests/dual_select/defconfig b/scripts/kconfig/tests/dual_select/defconfig
new file mode 100644
index 000000000000..fa0475fa74ab
--- /dev/null
+++ b/scripts/kconfig/tests/dual_select/defconfig
@@ -0,0 +1,7 @@
+CONFIG_MODULES=y
+CONFIG_B1=m
+CONFIG_C1=y
+CONFIG_B2=y
+CONFIG_C2=m
+CONFIG_B3=m
+CONFIG_C3=y
diff --git a/scripts/kconfig/tests/dual_select/expected_config b/scripts/kconfig/tests/dual_select/expected_config
new file mode 100644
index 000000000000..1c06356d8b1c
--- /dev/null
+++ b/scripts/kconfig/tests/dual_select/expected_config
@@ -0,0 +1,10 @@
+CONFIG_MODULES=y
+CONFIG_A1=y
+CONFIG_B1=m
+CONFIG_C1=y
+CONFIG_A2=y
+CONFIG_B2=y
+CONFIG_C2=m
+CONFIG_A3=y
+CONFIG_B3=m
+CONFIG_C3=y

-- 
tg: (80e54e84911a..) kconfig/dual-select-case (depends on: master)