[PATCH v2 1/3] acpi: make EC support compile-time conditional

[PATCH v2 1/3] acpi: make EC support compile-time conditional

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The embedded controller code is mainly used on x86 laptops and cannot
work without PC style I/O port access.

Make this a user-visible configuration option that is default enabled
on x86 but otherwise disabled, and that can never be enabled unless
CONFIG_HAS_IOPORT is also available.

The empty stubs in internal.h help ignore the EC code in configurations
that don't support it. In order to see those stubs, the sbshc code also
has to include this header and drop duplicate declarations.

All the direct callers of ec_read/ec_write already had an x86
dependency and now also need to depend on APCI_EC.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/acpi/Kconfig               | 11 ++++++++++-
 drivers/acpi/Makefile              |  2 +-
 drivers/acpi/internal.h            | 25 +++++++++++++++++++++++++
 drivers/acpi/sbshc.c               |  9 +--------
 drivers/char/Kconfig               |  1 +
 drivers/hwmon/Kconfig              |  3 ++-
 drivers/platform/x86/Kconfig       | 22 ++++++++++++----------
 drivers/platform/x86/dell/Kconfig  |  1 +
 drivers/platform/x86/hp/Kconfig    |  1 +
 drivers/platform/x86/intel/Kconfig |  2 +-
 include/linux/acpi.h               |  8 ++++++--
 11 files changed, 61 insertions(+), 24 deletions(-)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index d67f63d93b2a..d65cd08ba8e1 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -132,8 +132,17 @@ config ACPI_REV_OVERRIDE_POSSIBLE
 	  makes it possible to force the kernel to return "5" as the supported
 	  ACPI revision via the "acpi_rev_override" command line switch.
 
+config ACPI_EC
+	bool "Embedded Controller"
+	depends on HAS_IOPORT
+	default X86
+	help
+	  This driver handles communication with the microcontroller
+	  on many x86 laptops and other machines.
+
 config ACPI_EC_DEBUGFS
 	tristate "EC read/write access through /sys/kernel/debug/ec"
+	depends on ACPI_EC
 	help
 	  Say N to disable Embedded Controller /sys/kernel/debug interface
 
@@ -433,7 +442,7 @@ config ACPI_HOTPLUG_IOAPIC
 
 config ACPI_SBS
 	tristate "Smart Battery System"
-	depends on X86
+	depends on X86 && ACPI_EC
 	select POWER_SUPPLY
 	help
 	  This driver supports the Smart Battery System, another
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 61ca4afe83dc..40208a0f5dfb 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -41,7 +41,7 @@ acpi-y				+= resource.o
 acpi-y				+= acpi_processor.o
 acpi-y				+= processor_core.o
 acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) += processor_pdc.o
-acpi-y				+= ec.o
+acpi-$(CONFIG_ACPI_EC)		+= ec.o
 acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
 acpi-$(CONFIG_PCI)		+= pci_root.o pci_link.o pci_irq.o
 obj-$(CONFIG_ACPI_MCFG)		+= pci_mcfg.o
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index ced7dff9a5db..00910ccd7eda 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -215,6 +215,8 @@ extern struct acpi_ec *first_ec;
 /* External interfaces use first EC only, so remember */
 typedef int (*acpi_ec_query_func) (void *data);
 
+#ifdef CONFIG_ACPI_EC
+
 void acpi_ec_init(void);
 void acpi_ec_ecdt_probe(void);
 void acpi_ec_dsdt_probe(void);
@@ -231,6 +233,29 @@ void acpi_ec_flush_work(void);
 bool acpi_ec_dispatch_gpe(void);
 #endif
 
+#else
+
+static inline void acpi_ec_init(void) {}
+static inline void acpi_ec_ecdt_probe(void) {}
+static inline void acpi_ec_dsdt_probe(void) {}
+static inline void acpi_ec_block_transactions(void) {}
+static inline void acpi_ec_unblock_transactions(void) {}
+static inline int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
+			      acpi_handle handle, acpi_ec_query_func func,
+			      void *data)
+{
+	return -ENXIO;
+}
+static inline void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit) {}
+static inline void acpi_ec_register_opregions(struct acpi_device *adev) {}
+
+static inline void acpi_ec_flush_work(void) {}
+static inline bool acpi_ec_dispatch_gpe(void)
+{
+	return false;
+}
+
+#endif
 
 /*--------------------------------------------------------------------------
                                   Suspend/Resume
diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
index 16f2daaa2c45..2b63cd18cca2 100644
--- a/drivers/acpi/sbshc.c
+++ b/drivers/acpi/sbshc.c
@@ -14,6 +14,7 @@
 #include <linux/module.h>
 #include <linux/interrupt.h>
 #include "sbshc.h"
+#include "internal.h"
 
 #define ACPI_SMB_HC_CLASS	"smbus_host_ctl"
 #define ACPI_SMB_HC_DEVICE_NAME	"ACPI SMBus HC"
@@ -236,12 +237,6 @@ static int smbus_alarm(void *context)
 	return 0;
 }
 
-typedef int (*acpi_ec_query_func) (void *data);
-
-extern int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
-			      acpi_handle handle, acpi_ec_query_func func,
-			      void *data);
-
 static int acpi_smbus_hc_add(struct acpi_device *device)
 {
 	int status;
@@ -278,8 +273,6 @@ static int acpi_smbus_hc_add(struct acpi_device *device)
 	return 0;
 }
 
-extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
-
 static void acpi_smbus_hc_remove(struct acpi_device *device)
 {
 	struct acpi_smb_hc *hc;
diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
index 7c8dd0abcfdf..8fb33c90482f 100644
--- a/drivers/char/Kconfig
+++ b/drivers/char/Kconfig
@@ -238,6 +238,7 @@ config APPLICOM
 config SONYPI
 	tristate "Sony Vaio Programmable I/O Control Device support"
 	depends on X86_32 && PCI && INPUT
+	depends on ACPI_EC || !ACPI
 	help
 	  This driver enables access to the Sony Programmable I/O Control
 	  Device which can be found in many (all ?) Sony Vaio laptops.
diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
index 65ea92529406..25ae0a00ea2c 100644
--- a/drivers/hwmon/Kconfig
+++ b/drivers/hwmon/Kconfig
@@ -1747,7 +1747,7 @@ source "drivers/hwmon/occ/Kconfig"
 
 config SENSORS_OXP
 	tristate "OneXPlayer EC fan control"
-	depends on ACPI
+	depends on ACPI_EC
 	depends on X86
 	help
 		If you say yes here you get support for fan readings and control over
@@ -2586,6 +2586,7 @@ config SENSORS_ASUS_WMI
 config SENSORS_ASUS_EC
 	tristate "ASUS EC Sensors"
 	depends on X86
+	depends on ACPI_EC
 	help
 	  If you say yes here you get support for the ACPI embedded controller
 	  hardware monitoring interface found in ASUS motherboards. The driver
diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index 3875abba5a79..0258dd879d64 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -52,6 +52,7 @@ config WMI_BMOF
 config HUAWEI_WMI
 	tristate "Huawei WMI laptop extras driver"
 	depends on ACPI_BATTERY
+	depends on ACPI_EC
 	depends on ACPI_WMI
 	depends on INPUT
 	select INPUT_SPARSEKMAP
@@ -147,7 +148,7 @@ config YT2_1380
 
 config ACERHDF
 	tristate "Acer Aspire One temperature and fan driver"
-	depends on ACPI && THERMAL
+	depends on ACPI_EC && THERMAL
 	select THERMAL_GOV_BANG_BANG
 	help
 	  This is a driver for Acer Aspire One netbooks. It allows to access
@@ -186,6 +187,7 @@ config ACER_WMI
 	depends on SERIO_I8042
 	depends on INPUT
 	depends on RFKILL || RFKILL = n
+	depends on ACPI_EC
 	depends on ACPI_WMI
 	depends on ACPI_VIDEO || ACPI_VIDEO = n
 	depends on HWMON
@@ -334,7 +336,7 @@ config MERAKI_MX100
 
 config EEEPC_LAPTOP
 	tristate "Eee PC Hotkey Driver"
-	depends on ACPI
+	depends on ACPI_EC
 	depends on INPUT
 	depends on RFKILL || RFKILL = n
 	depends on ACPI_VIDEO || ACPI_VIDEO = n
@@ -503,7 +505,7 @@ config SENSORS_HDAPS
 
 config THINKPAD_ACPI
 	tristate "ThinkPad ACPI Laptop Extras"
-	depends on ACPI
+	depends on ACPI_EC
 	depends on ACPI_BATTERY
 	depends on INPUT
 	depends on RFKILL || RFKILL = n
@@ -682,7 +684,7 @@ config MEEGOPAD_ANX7428
 
 config MSI_EC
 	tristate "MSI EC Extras"
-	depends on ACPI
+	depends on ACPI_EC
 	depends on ACPI_BATTERY
 	help
 	  This driver allows various MSI laptops' functionalities to be
@@ -690,7 +692,7 @@ config MSI_EC
 
 config MSI_LAPTOP
 	tristate "MSI Laptop Extras"
-	depends on ACPI
+	depends on ACPI_EC
 	depends on BACKLIGHT_CLASS_DEVICE
 	depends on ACPI_VIDEO || ACPI_VIDEO = n
 	depends on RFKILL
@@ -796,7 +798,7 @@ config SAMSUNG_LAPTOP
 
 config SAMSUNG_Q10
 	tristate "Samsung Q10 Extras"
-	depends on ACPI
+	depends on ACPI_EC
 	select BACKLIGHT_CLASS_DEVICE
 	help
 	  This driver provides support for backlight control on Samsung Q10
@@ -804,7 +806,7 @@ config SAMSUNG_Q10
 
 config ACPI_TOSHIBA
 	tristate "Toshiba Laptop Extras"
-	depends on ACPI
+	depends on ACPI_EC
 	depends on ACPI_BATTERY
 	depends on ACPI_WMI
 	select LEDS_CLASS
@@ -904,7 +906,7 @@ config ACPI_CMPC
 
 config COMPAL_LAPTOP
 	tristate "Compal (and others) Laptop Extras"
-	depends on ACPI
+	depends on ACPI_EC
 	depends on BACKLIGHT_CLASS_DEVICE
 	depends on ACPI_VIDEO || ACPI_VIDEO = n
 	depends on RFKILL
@@ -949,7 +951,7 @@ config PANASONIC_LAPTOP
 
 config SONY_LAPTOP
 	tristate "Sony Laptop Extras"
-	depends on ACPI
+	depends on ACPI_EC
 	depends on ACPI_VIDEO || ACPI_VIDEO = n
 	depends on BACKLIGHT_CLASS_DEVICE
 	depends on INPUT
@@ -972,7 +974,7 @@ config SONYPI_COMPAT
 
 config SYSTEM76_ACPI
 	tristate "System76 ACPI Driver"
-	depends on ACPI
+	depends on ACPI_EC
 	depends on ACPI_BATTERY
 	depends on HWMON
 	depends on INPUT
diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
index 68a49788a396..dc21227dd66e 100644
--- a/drivers/platform/x86/dell/Kconfig
+++ b/drivers/platform/x86/dell/Kconfig
@@ -194,6 +194,7 @@ config DELL_WMI
 config DELL_WMI_PRIVACY
 	bool "Dell WMI Hardware Privacy Support"
 	depends on DELL_WMI
+	depends on ACPI_EC
 	help
 	  This option adds integration with the "Dell Hardware Privacy"
 	  feature of Dell laptops to the dell-wmi driver.
diff --git a/drivers/platform/x86/hp/Kconfig b/drivers/platform/x86/hp/Kconfig
index d776761cd5fd..dd51491b9bcd 100644
--- a/drivers/platform/x86/hp/Kconfig
+++ b/drivers/platform/x86/hp/Kconfig
@@ -37,6 +37,7 @@ config HP_ACCEL
 config HP_WMI
 	tristate "HP WMI extras"
 	default m
+	depends on ACPI_EC
 	depends on ACPI_WMI
 	depends on INPUT
 	depends on RFKILL || RFKILL = n
diff --git a/drivers/platform/x86/intel/Kconfig b/drivers/platform/x86/intel/Kconfig
index ad50bbabec61..eb698dcb9af9 100644
--- a/drivers/platform/x86/intel/Kconfig
+++ b/drivers/platform/x86/intel/Kconfig
@@ -62,7 +62,7 @@ config INTEL_INT0002_VGPIO
 
 config INTEL_OAKTRAIL
 	tristate "Intel Oaktrail Platform Extras"
-	depends on ACPI
+	depends on ACPI_EC
 	depends on ACPI_VIDEO || ACPI_VIDEO=n
 	depends on RFKILL && BACKLIGHT_CLASS_DEVICE && ACPI
 	help
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 4d5ee84c468b..7dd24acd9ffe 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -1164,8 +1164,6 @@ int acpi_subsys_suspend_noirq(struct device *dev);
 int acpi_subsys_suspend(struct device *dev);
 int acpi_subsys_freeze(struct device *dev);
 int acpi_subsys_poweroff(struct device *dev);
-void acpi_ec_mark_gpe_for_wake(void);
-void acpi_ec_set_gpe_wake_mask(u8 action);
 int acpi_subsys_restore_early(struct device *dev);
 #else
 static inline int acpi_subsys_prepare(struct device *dev) { return 0; }
@@ -1176,6 +1174,12 @@ static inline int acpi_subsys_suspend(struct device *dev) { return 0; }
 static inline int acpi_subsys_freeze(struct device *dev) { return 0; }
 static inline int acpi_subsys_poweroff(struct device *dev) { return 0; }
 static inline int acpi_subsys_restore_early(struct device *dev) { return 0; }
+#endif
+
+#if defined(CONFIG_ACPI_EC) && defined(CONFIG_PM_SLEEP)
+void acpi_ec_mark_gpe_for_wake(void);
+void acpi_ec_set_gpe_wake_mask(u8 action);
+#else
 static inline void acpi_ec_mark_gpe_for_wake(void) {}
 static inline void acpi_ec_set_gpe_wake_mask(u8 action) {}
 #endif
-- 
2.39.5

[PATCH v2 2/3] acpi: allow building without CONFIG_HAS_IOPORT

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

CONFIG_HAS_IOPORT will soon become optional and cause a build time
failure when it is disabled but a driver calls inb()/outb(). At the
moment, all architectures that can support ACPI have port I/O, but this
is not necessarily the case in the future on non-x86 architectures.
The result is a set of errors like:

drivers/acpi/osl.c: In function 'acpi_os_read_port':
include/asm-generic/io.h:542:14: error: call to '_inb' declared with attribute error: inb()) requires CONFIG_HAS_IOPORT

Nothing should actually call these functions in this configuration,
and if it does, the result would be undefined behavior today, possibly
a NULL pointer dereference.

Change the low-level functions to return a proper error code when
HAS_IOPORT is disabled.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/acpi/cppc_acpi.c | 6 ++++--
 drivers/acpi/osl.c       | 8 ++++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index b73b3aa92f3f..326b73ae77a9 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1017,7 +1017,8 @@ static int cpc_read(int cpu, struct cpc_register_resource *reg_res, u64 *val)
 	*val = 0;
 	size = GET_BIT_WIDTH(reg);
 
-	if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
+	if (IS_ENABLED(CONFIG_HAS_IOPORT) &&
+	    reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
 		u32 val_u32;
 		acpi_status status;
 
@@ -1090,7 +1091,8 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
 
 	size = GET_BIT_WIDTH(reg);
 
-	if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
+	if (IS_ENABLED(CONFIG_HAS_IOPORT) &&
+	    reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
 		acpi_status status;
 
 		status = acpi_os_write_port((acpi_io_address)reg->address,
diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c
index 70af3fbbebe5..19342ccfabb9 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -642,6 +642,11 @@ acpi_status acpi_os_read_port(acpi_io_address port, u32 *value, u32 width)
 {
 	u32 dummy;
 
+	if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
+		*value = BIT_MASK(width);
+		return AE_NOT_IMPLEMENTED;
+	}
+
 	if (value)
 		*value = 0;
 	else
@@ -665,6 +670,9 @@ EXPORT_SYMBOL(acpi_os_read_port);
 
 acpi_status acpi_os_write_port(acpi_io_address port, u32 value, u32 width)
 {
+	if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+		return AE_NOT_IMPLEMENTED;
+
 	if (width <= 8) {
 		outb(value, port);
 	} else if (width <= 16) {
-- 
2.39.5

[PATCH v2 3/3] acpi: processor_perflib: extend X86 dependency

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

The majority of the processor_perflib code is only used by cpufreq
drivers on the x86 architecture and makes no sense without the
x86 SMI interactions that rely on I/O port access.

Replace the existing #ifdef checks with one that covers all of the
code that is only used by x86 drivers, saving a little bit
of kernel code size on other architectures.

There is likely more code under CONFIG_ACPI_PROCESSOR that falls
into this category, but changing those would require a larger
rework.

Suggested-by: Rafael J. Wysocki <rafael@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
This is not needed for correctness, only as a small optimization.
---
 drivers/acpi/processor_perflib.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
index 4265814c74f8..9a959a963a79 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -209,6 +209,7 @@ void acpi_processor_ppc_exit(struct cpufreq_policy *policy)
 	}
 }
 
+#ifdef CONFIG_X86
 static int acpi_processor_get_performance_control(struct acpi_processor *pr)
 {
 	int result = 0;
@@ -267,7 +268,6 @@ static int acpi_processor_get_performance_control(struct acpi_processor *pr)
 	return result;
 }
 
-#ifdef CONFIG_X86
 /*
  * Some AMDs have 50MHz frequency multiples, but only provide 100MHz rounding
  * in their ACPI data. Calculate the real values and fix up the _PSS data.
@@ -298,9 +298,6 @@ static void amd_fixup_frequency(struct acpi_processor_px *px, int i)
 			px->core_frequency = (100 * (fid + 8)) >> did;
 	}
 }
-#else
-static void amd_fixup_frequency(struct acpi_processor_px *px, int i) {};
-#endif
 
 static int acpi_processor_get_performance_states(struct acpi_processor *pr)
 {
@@ -440,13 +437,11 @@ int acpi_processor_get_performance_info(struct acpi_processor *pr)
 	 * the BIOS is older than the CPU and does not know its frequencies
 	 */
  update_bios:
-#ifdef CONFIG_X86
 	if (acpi_has_method(pr->handle, "_PPC")) {
 		if(boot_cpu_has(X86_FEATURE_EST))
 			pr_warn(FW_BUG "BIOS needs update for CPU "
 			       "frequency support\n");
 	}
-#endif
 	return result;
 }
 EXPORT_SYMBOL_GPL(acpi_processor_get_performance_info);
@@ -788,3 +783,4 @@ void acpi_processor_unregister_performance(unsigned int cpu)
 	mutex_unlock(&performance_mutex);
 }
 EXPORT_SYMBOL(acpi_processor_unregister_performance);
+#endif
-- 
2.39.5

Re: [PATCH v2 2/3] acpi: allow building without CONFIG_HAS_IOPORT

[Andy Shevchenko]

On Fri, Oct 11, 2024 at 06:18:18AM +0000, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> CONFIG_HAS_IOPORT will soon become optional and cause a build time
> failure when it is disabled but a driver calls inb()/outb(). At the
> moment, all architectures that can support ACPI have port I/O, but this
> is not necessarily the case in the future on non-x86 architectures.
> The result is a set of errors like:
> 
> drivers/acpi/osl.c: In function 'acpi_os_read_port':
> include/asm-generic/io.h:542:14: error: call to '_inb' declared with attribute error: inb()) requires CONFIG_HAS_IOPORT
> 
> Nothing should actually call these functions in this configuration,
> and if it does, the result would be undefined behavior today, possibly
> a NULL pointer dereference.
> 
> Change the low-level functions to return a proper error code when
> HAS_IOPORT is disabled.

...

> +	if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
> +		*value = BIT_MASK(width);
> +		return AE_NOT_IMPLEMENTED;

Perhaps it has already been discussed, but why do we need to file value with
semi-garbage when we know it's invalid anyway?

> +	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] acpi: allow building without CONFIG_HAS_IOPORT

[Arnd Bergmann]

On Fri, Oct 11, 2024, at 09:53, Andy Shevchenko wrote:
> On Fri, Oct 11, 2024 at 06:18:18AM +0000, Arnd Bergmann wrote:
>
> ...
>
>> +	if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
>> +		*value = BIT_MASK(width);
>> +		return AE_NOT_IMPLEMENTED;
>
> Perhaps it has already been discussed, but why do we need to file value with
> semi-garbage when we know it's invalid anyway?

It's not strictly necessary, just precaution for possible callers
that use the resulting data without checking the error code.

The all-ones data is what an x86 PC would see when an I/O
port is read that is not connected to any device.

     Arnd

Re: [PATCH v2 2/3] acpi: allow building without CONFIG_HAS_IOPORT

[Andy Shevchenko]

On Fri, Oct 11, 2024 at 09:59:46AM +0000, Arnd Bergmann wrote:
> On Fri, Oct 11, 2024, at 09:53, Andy Shevchenko wrote:
> > On Fri, Oct 11, 2024 at 06:18:18AM +0000, Arnd Bergmann wrote:

...

> >> +	if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
> >> +		*value = BIT_MASK(width);
> >> +		return AE_NOT_IMPLEMENTED;
> >
> > Perhaps it has already been discussed, but why do we need to file value with
> > semi-garbage when we know it's invalid anyway?
> 
> It's not strictly necessary, just precaution for possible callers
> that use the resulting data without checking the error code.

Do you have any examples of that in the kernel?

> The all-ones data is what an x86 PC would see when an I/O
> port is read that is not connected to any device.

Yes, but it's not what your code does.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] acpi: allow building without CONFIG_HAS_IOPORT

[Rafael J. Wysocki]

On Fri, Oct 11, 2024 at 1:12 PM Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> On Fri, Oct 11, 2024 at 09:59:46AM +0000, Arnd Bergmann wrote:
> > On Fri, Oct 11, 2024, at 09:53, Andy Shevchenko wrote:
> > > On Fri, Oct 11, 2024 at 06:18:18AM +0000, Arnd Bergmann wrote:
>
> ...
>
> > >> +  if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
> > >> +          *value = BIT_MASK(width);
> > >> +          return AE_NOT_IMPLEMENTED;
> > >
> > > Perhaps it has already been discussed, but why do we need to file value with
> > > semi-garbage when we know it's invalid anyway?
> >
> > It's not strictly necessary, just precaution for possible callers
> > that use the resulting data without checking the error code.
>
> Do you have any examples of that in the kernel?

Yes, there are at least 2 cases.  May not be relevant, though.

> > The all-ones data is what an x86 PC would see when an I/O
> > port is read that is not connected to any device.
>
> Yes, but it's not what your code does.

Care to elaborate?

Re: [PATCH v2 2/3] acpi: allow building without CONFIG_HAS_IOPORT

[Arnd Bergmann]

On Fri, Oct 11, 2024, at 11:12, Andy Shevchenko wrote:
> On Fri, Oct 11, 2024 at 09:59:46AM +0000, Arnd Bergmann wrote:
>> On Fri, Oct 11, 2024, at 09:53, Andy Shevchenko wrote:
>> > On Fri, Oct 11, 2024 at 06:18:18AM +0000, Arnd Bergmann wrote:
>> >> +	if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
>> >> +		*value = BIT_MASK(width);
>> >> +		return AE_NOT_IMPLEMENTED;
>> >
>> > Perhaps it has already been discussed, but why do we need to file value with
>> > semi-garbage when we know it's invalid anyway?
>> 
>> It's not strictly necessary, just precaution for possible callers
>> that use the resulting data without checking the error code.
>
> Do you have any examples of that in the kernel?


drivers/acpi/processor_throttling.c:            acpi_os_read_port((acpi_io_address) throttling->status_register.
--
drivers/cpufreq/acpi-cpufreq.c-
drivers/cpufreq/acpi-cpufreq.c: acpi_os_read_port(reg->address, &val, reg->bit_width);

$ git grep ^[^=]*acpi_os_read_port 
drivers/acpi/processor_throttling.c:            acpi_os_read_port(\ (acpi_io_address) throttling->status_register.
drivers/cpufreq/acpi-cpufreq.c: acpi_os_read_port(reg->address, &val, reg->bit_width);

>> The all-ones data is what an x86 PC would see when an I/O
>> port is read that is not connected to any device.
>
> Yes, but it's not what your code does.

My bad, I was confused about what BIT_MASK() does.
I'll change it to "GENMASK(width, 0)", which should
do what I intended.

      Arnd

Re: [PATCH v2 2/3] acpi: allow building without CONFIG_HAS_IOPORT

[Andy Shevchenko]

On Fri, Oct 11, 2024 at 01:28:23PM +0200, Rafael J. Wysocki wrote:
> On Fri, Oct 11, 2024 at 1:12 PM Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
> > On Fri, Oct 11, 2024 at 09:59:46AM +0000, Arnd Bergmann wrote:
> > > On Fri, Oct 11, 2024, at 09:53, Andy Shevchenko wrote:
> > > > On Fri, Oct 11, 2024 at 06:18:18AM +0000, Arnd Bergmann wrote:

...

> > > >> +  if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
> > > >> +          *value = BIT_MASK(width);
> > > >> +          return AE_NOT_IMPLEMENTED;
> > > >
> > > > Perhaps it has already been discussed, but why do we need to file value with
> > > > semi-garbage when we know it's invalid anyway?
> > >
> > > It's not strictly necessary, just precaution for possible callers
> > > that use the resulting data without checking the error code.
> >
> > Do you have any examples of that in the kernel?
> 
> Yes, there are at least 2 cases.  May not be relevant, though.

Btw, may be we even can add the error check to them, dunno...

> > > The all-ones data is what an x86 PC would see when an I/O
> > > port is read that is not connected to any device.
> >
> > Yes, but it's not what your code does.
> 
> Care to elaborate?

Sure, but it seems Arnd already figured out that he set one bit only somewhere
in the returned value, not what he stated in the explanation in this email
thread.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 2/3] acpi: allow building without CONFIG_HAS_IOPORT

[Andy Shevchenko]

On Fri, Oct 11, 2024 at 11:40:05AM +0000, Arnd Bergmann wrote:
> On Fri, Oct 11, 2024, at 11:12, Andy Shevchenko wrote:
> > On Fri, Oct 11, 2024 at 09:59:46AM +0000, Arnd Bergmann wrote:
> >> On Fri, Oct 11, 2024, at 09:53, Andy Shevchenko wrote:
> >> > On Fri, Oct 11, 2024 at 06:18:18AM +0000, Arnd Bergmann wrote:
> >> >> +	if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
> >> >> +		*value = BIT_MASK(width);
> >> >> +		return AE_NOT_IMPLEMENTED;
> >> >
> >> > Perhaps it has already been discussed, but why do we need to file value with
> >> > semi-garbage when we know it's invalid anyway?
> >> 
> >> It's not strictly necessary, just precaution for possible callers
> >> that use the resulting data without checking the error code.
> >
> > Do you have any examples of that in the kernel?
> 
> drivers/acpi/processor_throttling.c:            acpi_os_read_port((acpi_io_address) throttling->status_register.
> --
> drivers/cpufreq/acpi-cpufreq.c-
> drivers/cpufreq/acpi-cpufreq.c: acpi_os_read_port(reg->address, &val, reg->bit_width);
> 
> $ git grep ^[^=]*acpi_os_read_port 
> drivers/acpi/processor_throttling.c:            acpi_os_read_port(\ (acpi_io_address) throttling->status_register.
> drivers/cpufreq/acpi-cpufreq.c: acpi_os_read_port(reg->address, &val, reg->bit_width);

May be we can add checks to them, but dunno...

> >> The all-ones data is what an x86 PC would see when an I/O
> >> port is read that is not connected to any device.
> >
> > Yes, but it's not what your code does.
> 
> My bad, I was confused about what BIT_MASK() does.
> I'll change it to "GENMASK(width, 0)", which should
> do what I intended.

Okay. Maybe also adding a comment that it's usual behaviour in response to
the read from non-existing IO port?

(Or for the curios it's all comes from the Data Bus on hardware being Open
 Drain an hence use of pull-up resistors and when there is no response on
 the bus, the default will be "All 1:s").

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 1/3] acpi: make EC support compile-time conditional

[Guenter Roeck]

On Fri, Oct 11, 2024 at 06:18:17AM +0000, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The embedded controller code is mainly used on x86 laptops and cannot
> work without PC style I/O port access.
> 
> Make this a user-visible configuration option that is default enabled
> on x86 but otherwise disabled, and that can never be enabled unless
> CONFIG_HAS_IOPORT is also available.
> 
> The empty stubs in internal.h help ignore the EC code in configurations
> that don't support it. In order to see those stubs, the sbshc code also
> has to include this header and drop duplicate declarations.
> 
> All the direct callers of ec_read/ec_write already had an x86
> dependency and now also need to depend on APCI_EC.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/acpi/Kconfig               | 11 ++++++++++-
>  drivers/acpi/Makefile              |  2 +-
>  drivers/acpi/internal.h            | 25 +++++++++++++++++++++++++
>  drivers/acpi/sbshc.c               |  9 +--------
>  drivers/char/Kconfig               |  1 +
>  drivers/hwmon/Kconfig              |  3 ++-

For hwmon:

Acked-by: Guenter Roeck <linux@roeck-us.net>

Guenter

Re: [PATCH v2 1/3] acpi: make EC support compile-time conditional

[Hans de Goede]

Hi,

On 11-Oct-24 8:18 AM, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The embedded controller code is mainly used on x86 laptops and cannot
> work without PC style I/O port access.
> 
> Make this a user-visible configuration option that is default enabled
> on x86 but otherwise disabled, and that can never be enabled unless
> CONFIG_HAS_IOPORT is also available.
> 
> The empty stubs in internal.h help ignore the EC code in configurations
> that don't support it. In order to see those stubs, the sbshc code also
> has to include this header and drop duplicate declarations.
> 
> All the direct callers of ec_read/ec_write already had an x86
> dependency and now also need to depend on APCI_EC.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Thanks, patch looks good to me.

For platform/driver/x86:

Acked-by: Hans de Goede <hdegoede@redhat.com>

Regards,

Hans



> ---
>  drivers/acpi/Kconfig               | 11 ++++++++++-
>  drivers/acpi/Makefile              |  2 +-
>  drivers/acpi/internal.h            | 25 +++++++++++++++++++++++++
>  drivers/acpi/sbshc.c               |  9 +--------
>  drivers/char/Kconfig               |  1 +
>  drivers/hwmon/Kconfig              |  3 ++-
>  drivers/platform/x86/Kconfig       | 22 ++++++++++++----------
>  drivers/platform/x86/dell/Kconfig  |  1 +
>  drivers/platform/x86/hp/Kconfig    |  1 +
>  drivers/platform/x86/intel/Kconfig |  2 +-
>  include/linux/acpi.h               |  8 ++++++--
>  11 files changed, 61 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index d67f63d93b2a..d65cd08ba8e1 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -132,8 +132,17 @@ config ACPI_REV_OVERRIDE_POSSIBLE
>  	  makes it possible to force the kernel to return "5" as the supported
>  	  ACPI revision via the "acpi_rev_override" command line switch.
>  
> +config ACPI_EC
> +	bool "Embedded Controller"
> +	depends on HAS_IOPORT
> +	default X86
> +	help
> +	  This driver handles communication with the microcontroller
> +	  on many x86 laptops and other machines.
> +
>  config ACPI_EC_DEBUGFS
>  	tristate "EC read/write access through /sys/kernel/debug/ec"
> +	depends on ACPI_EC
>  	help
>  	  Say N to disable Embedded Controller /sys/kernel/debug interface
>  
> @@ -433,7 +442,7 @@ config ACPI_HOTPLUG_IOAPIC
>  
>  config ACPI_SBS
>  	tristate "Smart Battery System"
> -	depends on X86
> +	depends on X86 && ACPI_EC
>  	select POWER_SUPPLY
>  	help
>  	  This driver supports the Smart Battery System, another
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 61ca4afe83dc..40208a0f5dfb 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -41,7 +41,7 @@ acpi-y				+= resource.o
>  acpi-y				+= acpi_processor.o
>  acpi-y				+= processor_core.o
>  acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) += processor_pdc.o
> -acpi-y				+= ec.o
> +acpi-$(CONFIG_ACPI_EC)		+= ec.o
>  acpi-$(CONFIG_ACPI_DOCK)	+= dock.o
>  acpi-$(CONFIG_PCI)		+= pci_root.o pci_link.o pci_irq.o
>  obj-$(CONFIG_ACPI_MCFG)		+= pci_mcfg.o
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index ced7dff9a5db..00910ccd7eda 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -215,6 +215,8 @@ extern struct acpi_ec *first_ec;
>  /* External interfaces use first EC only, so remember */
>  typedef int (*acpi_ec_query_func) (void *data);
>  
> +#ifdef CONFIG_ACPI_EC
> +
>  void acpi_ec_init(void);
>  void acpi_ec_ecdt_probe(void);
>  void acpi_ec_dsdt_probe(void);
> @@ -231,6 +233,29 @@ void acpi_ec_flush_work(void);
>  bool acpi_ec_dispatch_gpe(void);
>  #endif
>  
> +#else
> +
> +static inline void acpi_ec_init(void) {}
> +static inline void acpi_ec_ecdt_probe(void) {}
> +static inline void acpi_ec_dsdt_probe(void) {}
> +static inline void acpi_ec_block_transactions(void) {}
> +static inline void acpi_ec_unblock_transactions(void) {}
> +static inline int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
> +			      acpi_handle handle, acpi_ec_query_func func,
> +			      void *data)
> +{
> +	return -ENXIO;
> +}
> +static inline void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit) {}
> +static inline void acpi_ec_register_opregions(struct acpi_device *adev) {}
> +
> +static inline void acpi_ec_flush_work(void) {}
> +static inline bool acpi_ec_dispatch_gpe(void)
> +{
> +	return false;
> +}
> +
> +#endif
>  
>  /*--------------------------------------------------------------------------
>                                    Suspend/Resume
> diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
> index 16f2daaa2c45..2b63cd18cca2 100644
> --- a/drivers/acpi/sbshc.c
> +++ b/drivers/acpi/sbshc.c
> @@ -14,6 +14,7 @@
>  #include <linux/module.h>
>  #include <linux/interrupt.h>
>  #include "sbshc.h"
> +#include "internal.h"
>  
>  #define ACPI_SMB_HC_CLASS	"smbus_host_ctl"
>  #define ACPI_SMB_HC_DEVICE_NAME	"ACPI SMBus HC"
> @@ -236,12 +237,6 @@ static int smbus_alarm(void *context)
>  	return 0;
>  }
>  
> -typedef int (*acpi_ec_query_func) (void *data);
> -
> -extern int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
> -			      acpi_handle handle, acpi_ec_query_func func,
> -			      void *data);
> -
>  static int acpi_smbus_hc_add(struct acpi_device *device)
>  {
>  	int status;
> @@ -278,8 +273,6 @@ static int acpi_smbus_hc_add(struct acpi_device *device)
>  	return 0;
>  }
>  
> -extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
> -
>  static void acpi_smbus_hc_remove(struct acpi_device *device)
>  {
>  	struct acpi_smb_hc *hc;
> diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
> index 7c8dd0abcfdf..8fb33c90482f 100644
> --- a/drivers/char/Kconfig
> +++ b/drivers/char/Kconfig
> @@ -238,6 +238,7 @@ config APPLICOM
>  config SONYPI
>  	tristate "Sony Vaio Programmable I/O Control Device support"
>  	depends on X86_32 && PCI && INPUT
> +	depends on ACPI_EC || !ACPI
>  	help
>  	  This driver enables access to the Sony Programmable I/O Control
>  	  Device which can be found in many (all ?) Sony Vaio laptops.
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 65ea92529406..25ae0a00ea2c 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1747,7 +1747,7 @@ source "drivers/hwmon/occ/Kconfig"
>  
>  config SENSORS_OXP
>  	tristate "OneXPlayer EC fan control"
> -	depends on ACPI
> +	depends on ACPI_EC
>  	depends on X86
>  	help
>  		If you say yes here you get support for fan readings and control over
> @@ -2586,6 +2586,7 @@ config SENSORS_ASUS_WMI
>  config SENSORS_ASUS_EC
>  	tristate "ASUS EC Sensors"
>  	depends on X86
> +	depends on ACPI_EC
>  	help
>  	  If you say yes here you get support for the ACPI embedded controller
>  	  hardware monitoring interface found in ASUS motherboards. The driver
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 3875abba5a79..0258dd879d64 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -52,6 +52,7 @@ config WMI_BMOF
>  config HUAWEI_WMI
>  	tristate "Huawei WMI laptop extras driver"
>  	depends on ACPI_BATTERY
> +	depends on ACPI_EC
>  	depends on ACPI_WMI
>  	depends on INPUT
>  	select INPUT_SPARSEKMAP
> @@ -147,7 +148,7 @@ config YT2_1380
>  
>  config ACERHDF
>  	tristate "Acer Aspire One temperature and fan driver"
> -	depends on ACPI && THERMAL
> +	depends on ACPI_EC && THERMAL
>  	select THERMAL_GOV_BANG_BANG
>  	help
>  	  This is a driver for Acer Aspire One netbooks. It allows to access
> @@ -186,6 +187,7 @@ config ACER_WMI
>  	depends on SERIO_I8042
>  	depends on INPUT
>  	depends on RFKILL || RFKILL = n
> +	depends on ACPI_EC
>  	depends on ACPI_WMI
>  	depends on ACPI_VIDEO || ACPI_VIDEO = n
>  	depends on HWMON
> @@ -334,7 +336,7 @@ config MERAKI_MX100
>  
>  config EEEPC_LAPTOP
>  	tristate "Eee PC Hotkey Driver"
> -	depends on ACPI
> +	depends on ACPI_EC
>  	depends on INPUT
>  	depends on RFKILL || RFKILL = n
>  	depends on ACPI_VIDEO || ACPI_VIDEO = n
> @@ -503,7 +505,7 @@ config SENSORS_HDAPS
>  
>  config THINKPAD_ACPI
>  	tristate "ThinkPad ACPI Laptop Extras"
> -	depends on ACPI
> +	depends on ACPI_EC
>  	depends on ACPI_BATTERY
>  	depends on INPUT
>  	depends on RFKILL || RFKILL = n
> @@ -682,7 +684,7 @@ config MEEGOPAD_ANX7428
>  
>  config MSI_EC
>  	tristate "MSI EC Extras"
> -	depends on ACPI
> +	depends on ACPI_EC
>  	depends on ACPI_BATTERY
>  	help
>  	  This driver allows various MSI laptops' functionalities to be
> @@ -690,7 +692,7 @@ config MSI_EC
>  
>  config MSI_LAPTOP
>  	tristate "MSI Laptop Extras"
> -	depends on ACPI
> +	depends on ACPI_EC
>  	depends on BACKLIGHT_CLASS_DEVICE
>  	depends on ACPI_VIDEO || ACPI_VIDEO = n
>  	depends on RFKILL
> @@ -796,7 +798,7 @@ config SAMSUNG_LAPTOP
>  
>  config SAMSUNG_Q10
>  	tristate "Samsung Q10 Extras"
> -	depends on ACPI
> +	depends on ACPI_EC
>  	select BACKLIGHT_CLASS_DEVICE
>  	help
>  	  This driver provides support for backlight control on Samsung Q10
> @@ -804,7 +806,7 @@ config SAMSUNG_Q10
>  
>  config ACPI_TOSHIBA
>  	tristate "Toshiba Laptop Extras"
> -	depends on ACPI
> +	depends on ACPI_EC
>  	depends on ACPI_BATTERY
>  	depends on ACPI_WMI
>  	select LEDS_CLASS
> @@ -904,7 +906,7 @@ config ACPI_CMPC
>  
>  config COMPAL_LAPTOP
>  	tristate "Compal (and others) Laptop Extras"
> -	depends on ACPI
> +	depends on ACPI_EC
>  	depends on BACKLIGHT_CLASS_DEVICE
>  	depends on ACPI_VIDEO || ACPI_VIDEO = n
>  	depends on RFKILL
> @@ -949,7 +951,7 @@ config PANASONIC_LAPTOP
>  
>  config SONY_LAPTOP
>  	tristate "Sony Laptop Extras"
> -	depends on ACPI
> +	depends on ACPI_EC
>  	depends on ACPI_VIDEO || ACPI_VIDEO = n
>  	depends on BACKLIGHT_CLASS_DEVICE
>  	depends on INPUT
> @@ -972,7 +974,7 @@ config SONYPI_COMPAT
>  
>  config SYSTEM76_ACPI
>  	tristate "System76 ACPI Driver"
> -	depends on ACPI
> +	depends on ACPI_EC
>  	depends on ACPI_BATTERY
>  	depends on HWMON
>  	depends on INPUT
> diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
> index 68a49788a396..dc21227dd66e 100644
> --- a/drivers/platform/x86/dell/Kconfig
> +++ b/drivers/platform/x86/dell/Kconfig
> @@ -194,6 +194,7 @@ config DELL_WMI
>  config DELL_WMI_PRIVACY
>  	bool "Dell WMI Hardware Privacy Support"
>  	depends on DELL_WMI
> +	depends on ACPI_EC
>  	help
>  	  This option adds integration with the "Dell Hardware Privacy"
>  	  feature of Dell laptops to the dell-wmi driver.
> diff --git a/drivers/platform/x86/hp/Kconfig b/drivers/platform/x86/hp/Kconfig
> index d776761cd5fd..dd51491b9bcd 100644
> --- a/drivers/platform/x86/hp/Kconfig
> +++ b/drivers/platform/x86/hp/Kconfig
> @@ -37,6 +37,7 @@ config HP_ACCEL
>  config HP_WMI
>  	tristate "HP WMI extras"
>  	default m
> +	depends on ACPI_EC
>  	depends on ACPI_WMI
>  	depends on INPUT
>  	depends on RFKILL || RFKILL = n
> diff --git a/drivers/platform/x86/intel/Kconfig b/drivers/platform/x86/intel/Kconfig
> index ad50bbabec61..eb698dcb9af9 100644
> --- a/drivers/platform/x86/intel/Kconfig
> +++ b/drivers/platform/x86/intel/Kconfig
> @@ -62,7 +62,7 @@ config INTEL_INT0002_VGPIO
>  
>  config INTEL_OAKTRAIL
>  	tristate "Intel Oaktrail Platform Extras"
> -	depends on ACPI
> +	depends on ACPI_EC
>  	depends on ACPI_VIDEO || ACPI_VIDEO=n
>  	depends on RFKILL && BACKLIGHT_CLASS_DEVICE && ACPI
>  	help
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 4d5ee84c468b..7dd24acd9ffe 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1164,8 +1164,6 @@ int acpi_subsys_suspend_noirq(struct device *dev);
>  int acpi_subsys_suspend(struct device *dev);
>  int acpi_subsys_freeze(struct device *dev);
>  int acpi_subsys_poweroff(struct device *dev);
> -void acpi_ec_mark_gpe_for_wake(void);
> -void acpi_ec_set_gpe_wake_mask(u8 action);
>  int acpi_subsys_restore_early(struct device *dev);
>  #else
>  static inline int acpi_subsys_prepare(struct device *dev) { return 0; }
> @@ -1176,6 +1174,12 @@ static inline int acpi_subsys_suspend(struct device *dev) { return 0; }
>  static inline int acpi_subsys_freeze(struct device *dev) { return 0; }
>  static inline int acpi_subsys_poweroff(struct device *dev) { return 0; }
>  static inline int acpi_subsys_restore_early(struct device *dev) { return 0; }
> +#endif
> +
> +#if defined(CONFIG_ACPI_EC) && defined(CONFIG_PM_SLEEP)
> +void acpi_ec_mark_gpe_for_wake(void);
> +void acpi_ec_set_gpe_wake_mask(u8 action);
> +#else
>  static inline void acpi_ec_mark_gpe_for_wake(void) {}
>  static inline void acpi_ec_set_gpe_wake_mask(u8 action) {}
>  #endif

Re: [PATCH v2 3/3] acpi: processor_perflib: extend X86 dependency

[kernel test robot]

Hi Arnd,

kernel test robot noticed the following build warnings:

[auto build test WARNING on rafael-pm/linux-next]
[also build test WARNING on rafael-pm/bleeding-edge char-misc/char-misc-testing char-misc/char-misc-next char-misc/char-misc-linus groeck-staging/hwmon-next soc/for-next linus/master v6.12-rc2 next-20241011]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Arnd-Bergmann/acpi-allow-building-without-CONFIG_HAS_IOPORT/20241011-142245
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
patch link:    https://lore.kernel.org/r/20241011061948.3211423-3-arnd%40kernel.org
patch subject: [PATCH v2 3/3] acpi: processor_perflib: extend X86 dependency
config: riscv-allmodconfig (https://download.01.org/0day-ci/archive/20241013/202410130611.9S3HzhgF-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 70e0a7e7e6a8541bcc46908c592eed561850e416)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241013/202410130611.9S3HzhgF-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410130611.9S3HzhgF-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/acpi/processor_perflib.c:27:21: warning: unused variable 'performance_mutex' [-Wunused-variable]
      27 | static DEFINE_MUTEX(performance_mutex);
         |                     ^~~~~~~~~~~~~~~~~
   include/linux/mutex.h:112:15: note: expanded from macro 'DEFINE_MUTEX'
     112 |         struct mutex mutexname = __MUTEX_INITIALIZER(mutexname)
         |                      ^~~~~~~~~
   1 warning generated.


vim +/performance_mutex +27 drivers/acpi/processor_perflib.c

^1da177e4c3f41 Linus Torvalds   2005-04-16  26  
65c19bbd28cba5 Arjan van de Ven 2006-04-27 @27  static DEFINE_MUTEX(performance_mutex);
^1da177e4c3f41 Linus Torvalds   2005-04-16  28  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/3] acpi: make EC support compile-time conditional

[Rafael J. Wysocki]

On Fri, Oct 11, 2024 at 8:19 AM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> The embedded controller code is mainly used on x86 laptops and cannot
> work without PC style I/O port access.
>
> Make this a user-visible configuration option that is default enabled
> on x86 but otherwise disabled, and that can never be enabled unless
> CONFIG_HAS_IOPORT is also available.
>
> The empty stubs in internal.h help ignore the EC code in configurations
> that don't support it. In order to see those stubs, the sbshc code also
> has to include this header and drop duplicate declarations.
>
> All the direct callers of ec_read/ec_write already had an x86
> dependency and now also need to depend on APCI_EC.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

I think I can pick up this one as the other two patches in the series
don't depend on it.

Any concerns about doing that?

> ---
>  drivers/acpi/Kconfig               | 11 ++++++++++-
>  drivers/acpi/Makefile              |  2 +-
>  drivers/acpi/internal.h            | 25 +++++++++++++++++++++++++
>  drivers/acpi/sbshc.c               |  9 +--------
>  drivers/char/Kconfig               |  1 +
>  drivers/hwmon/Kconfig              |  3 ++-
>  drivers/platform/x86/Kconfig       | 22 ++++++++++++----------
>  drivers/platform/x86/dell/Kconfig  |  1 +
>  drivers/platform/x86/hp/Kconfig    |  1 +
>  drivers/platform/x86/intel/Kconfig |  2 +-
>  include/linux/acpi.h               |  8 ++++++--
>  11 files changed, 61 insertions(+), 24 deletions(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index d67f63d93b2a..d65cd08ba8e1 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -132,8 +132,17 @@ config ACPI_REV_OVERRIDE_POSSIBLE
>           makes it possible to force the kernel to return "5" as the supported
>           ACPI revision via the "acpi_rev_override" command line switch.
>
> +config ACPI_EC
> +       bool "Embedded Controller"
> +       depends on HAS_IOPORT
> +       default X86
> +       help
> +         This driver handles communication with the microcontroller
> +         on many x86 laptops and other machines.
> +
>  config ACPI_EC_DEBUGFS
>         tristate "EC read/write access through /sys/kernel/debug/ec"
> +       depends on ACPI_EC
>         help
>           Say N to disable Embedded Controller /sys/kernel/debug interface
>
> @@ -433,7 +442,7 @@ config ACPI_HOTPLUG_IOAPIC
>
>  config ACPI_SBS
>         tristate "Smart Battery System"
> -       depends on X86
> +       depends on X86 && ACPI_EC
>         select POWER_SUPPLY
>         help
>           This driver supports the Smart Battery System, another
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 61ca4afe83dc..40208a0f5dfb 100644
> --- a/drivers/acpi/Makefile
> +++ b/drivers/acpi/Makefile
> @@ -41,7 +41,7 @@ acpi-y                                += resource.o
>  acpi-y                         += acpi_processor.o
>  acpi-y                         += processor_core.o
>  acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) += processor_pdc.o
> -acpi-y                         += ec.o
> +acpi-$(CONFIG_ACPI_EC)         += ec.o
>  acpi-$(CONFIG_ACPI_DOCK)       += dock.o
>  acpi-$(CONFIG_PCI)             += pci_root.o pci_link.o pci_irq.o
>  obj-$(CONFIG_ACPI_MCFG)                += pci_mcfg.o
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index ced7dff9a5db..00910ccd7eda 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -215,6 +215,8 @@ extern struct acpi_ec *first_ec;
>  /* External interfaces use first EC only, so remember */
>  typedef int (*acpi_ec_query_func) (void *data);
>
> +#ifdef CONFIG_ACPI_EC
> +
>  void acpi_ec_init(void);
>  void acpi_ec_ecdt_probe(void);
>  void acpi_ec_dsdt_probe(void);
> @@ -231,6 +233,29 @@ void acpi_ec_flush_work(void);
>  bool acpi_ec_dispatch_gpe(void);
>  #endif
>
> +#else
> +
> +static inline void acpi_ec_init(void) {}
> +static inline void acpi_ec_ecdt_probe(void) {}
> +static inline void acpi_ec_dsdt_probe(void) {}
> +static inline void acpi_ec_block_transactions(void) {}
> +static inline void acpi_ec_unblock_transactions(void) {}
> +static inline int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
> +                             acpi_handle handle, acpi_ec_query_func func,
> +                             void *data)
> +{
> +       return -ENXIO;
> +}
> +static inline void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit) {}
> +static inline void acpi_ec_register_opregions(struct acpi_device *adev) {}
> +
> +static inline void acpi_ec_flush_work(void) {}
> +static inline bool acpi_ec_dispatch_gpe(void)
> +{
> +       return false;
> +}
> +
> +#endif
>
>  /*--------------------------------------------------------------------------
>                                    Suspend/Resume
> diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
> index 16f2daaa2c45..2b63cd18cca2 100644
> --- a/drivers/acpi/sbshc.c
> +++ b/drivers/acpi/sbshc.c
> @@ -14,6 +14,7 @@
>  #include <linux/module.h>
>  #include <linux/interrupt.h>
>  #include "sbshc.h"
> +#include "internal.h"
>
>  #define ACPI_SMB_HC_CLASS      "smbus_host_ctl"
>  #define ACPI_SMB_HC_DEVICE_NAME        "ACPI SMBus HC"
> @@ -236,12 +237,6 @@ static int smbus_alarm(void *context)
>         return 0;
>  }
>
> -typedef int (*acpi_ec_query_func) (void *data);
> -
> -extern int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
> -                             acpi_handle handle, acpi_ec_query_func func,
> -                             void *data);
> -
>  static int acpi_smbus_hc_add(struct acpi_device *device)
>  {
>         int status;
> @@ -278,8 +273,6 @@ static int acpi_smbus_hc_add(struct acpi_device *device)
>         return 0;
>  }
>
> -extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
> -
>  static void acpi_smbus_hc_remove(struct acpi_device *device)
>  {
>         struct acpi_smb_hc *hc;
> diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
> index 7c8dd0abcfdf..8fb33c90482f 100644
> --- a/drivers/char/Kconfig
> +++ b/drivers/char/Kconfig
> @@ -238,6 +238,7 @@ config APPLICOM
>  config SONYPI
>         tristate "Sony Vaio Programmable I/O Control Device support"
>         depends on X86_32 && PCI && INPUT
> +       depends on ACPI_EC || !ACPI
>         help
>           This driver enables access to the Sony Programmable I/O Control
>           Device which can be found in many (all ?) Sony Vaio laptops.
> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> index 65ea92529406..25ae0a00ea2c 100644
> --- a/drivers/hwmon/Kconfig
> +++ b/drivers/hwmon/Kconfig
> @@ -1747,7 +1747,7 @@ source "drivers/hwmon/occ/Kconfig"
>
>  config SENSORS_OXP
>         tristate "OneXPlayer EC fan control"
> -       depends on ACPI
> +       depends on ACPI_EC
>         depends on X86
>         help
>                 If you say yes here you get support for fan readings and control over
> @@ -2586,6 +2586,7 @@ config SENSORS_ASUS_WMI
>  config SENSORS_ASUS_EC
>         tristate "ASUS EC Sensors"
>         depends on X86
> +       depends on ACPI_EC
>         help
>           If you say yes here you get support for the ACPI embedded controller
>           hardware monitoring interface found in ASUS motherboards. The driver
> diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> index 3875abba5a79..0258dd879d64 100644
> --- a/drivers/platform/x86/Kconfig
> +++ b/drivers/platform/x86/Kconfig
> @@ -52,6 +52,7 @@ config WMI_BMOF
>  config HUAWEI_WMI
>         tristate "Huawei WMI laptop extras driver"
>         depends on ACPI_BATTERY
> +       depends on ACPI_EC
>         depends on ACPI_WMI
>         depends on INPUT
>         select INPUT_SPARSEKMAP
> @@ -147,7 +148,7 @@ config YT2_1380
>
>  config ACERHDF
>         tristate "Acer Aspire One temperature and fan driver"
> -       depends on ACPI && THERMAL
> +       depends on ACPI_EC && THERMAL
>         select THERMAL_GOV_BANG_BANG
>         help
>           This is a driver for Acer Aspire One netbooks. It allows to access
> @@ -186,6 +187,7 @@ config ACER_WMI
>         depends on SERIO_I8042
>         depends on INPUT
>         depends on RFKILL || RFKILL = n
> +       depends on ACPI_EC
>         depends on ACPI_WMI
>         depends on ACPI_VIDEO || ACPI_VIDEO = n
>         depends on HWMON
> @@ -334,7 +336,7 @@ config MERAKI_MX100
>
>  config EEEPC_LAPTOP
>         tristate "Eee PC Hotkey Driver"
> -       depends on ACPI
> +       depends on ACPI_EC
>         depends on INPUT
>         depends on RFKILL || RFKILL = n
>         depends on ACPI_VIDEO || ACPI_VIDEO = n
> @@ -503,7 +505,7 @@ config SENSORS_HDAPS
>
>  config THINKPAD_ACPI
>         tristate "ThinkPad ACPI Laptop Extras"
> -       depends on ACPI
> +       depends on ACPI_EC
>         depends on ACPI_BATTERY
>         depends on INPUT
>         depends on RFKILL || RFKILL = n
> @@ -682,7 +684,7 @@ config MEEGOPAD_ANX7428
>
>  config MSI_EC
>         tristate "MSI EC Extras"
> -       depends on ACPI
> +       depends on ACPI_EC
>         depends on ACPI_BATTERY
>         help
>           This driver allows various MSI laptops' functionalities to be
> @@ -690,7 +692,7 @@ config MSI_EC
>
>  config MSI_LAPTOP
>         tristate "MSI Laptop Extras"
> -       depends on ACPI
> +       depends on ACPI_EC
>         depends on BACKLIGHT_CLASS_DEVICE
>         depends on ACPI_VIDEO || ACPI_VIDEO = n
>         depends on RFKILL
> @@ -796,7 +798,7 @@ config SAMSUNG_LAPTOP
>
>  config SAMSUNG_Q10
>         tristate "Samsung Q10 Extras"
> -       depends on ACPI
> +       depends on ACPI_EC
>         select BACKLIGHT_CLASS_DEVICE
>         help
>           This driver provides support for backlight control on Samsung Q10
> @@ -804,7 +806,7 @@ config SAMSUNG_Q10
>
>  config ACPI_TOSHIBA
>         tristate "Toshiba Laptop Extras"
> -       depends on ACPI
> +       depends on ACPI_EC
>         depends on ACPI_BATTERY
>         depends on ACPI_WMI
>         select LEDS_CLASS
> @@ -904,7 +906,7 @@ config ACPI_CMPC
>
>  config COMPAL_LAPTOP
>         tristate "Compal (and others) Laptop Extras"
> -       depends on ACPI
> +       depends on ACPI_EC
>         depends on BACKLIGHT_CLASS_DEVICE
>         depends on ACPI_VIDEO || ACPI_VIDEO = n
>         depends on RFKILL
> @@ -949,7 +951,7 @@ config PANASONIC_LAPTOP
>
>  config SONY_LAPTOP
>         tristate "Sony Laptop Extras"
> -       depends on ACPI
> +       depends on ACPI_EC
>         depends on ACPI_VIDEO || ACPI_VIDEO = n
>         depends on BACKLIGHT_CLASS_DEVICE
>         depends on INPUT
> @@ -972,7 +974,7 @@ config SONYPI_COMPAT
>
>  config SYSTEM76_ACPI
>         tristate "System76 ACPI Driver"
> -       depends on ACPI
> +       depends on ACPI_EC
>         depends on ACPI_BATTERY
>         depends on HWMON
>         depends on INPUT
> diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
> index 68a49788a396..dc21227dd66e 100644
> --- a/drivers/platform/x86/dell/Kconfig
> +++ b/drivers/platform/x86/dell/Kconfig
> @@ -194,6 +194,7 @@ config DELL_WMI
>  config DELL_WMI_PRIVACY
>         bool "Dell WMI Hardware Privacy Support"
>         depends on DELL_WMI
> +       depends on ACPI_EC
>         help
>           This option adds integration with the "Dell Hardware Privacy"
>           feature of Dell laptops to the dell-wmi driver.
> diff --git a/drivers/platform/x86/hp/Kconfig b/drivers/platform/x86/hp/Kconfig
> index d776761cd5fd..dd51491b9bcd 100644
> --- a/drivers/platform/x86/hp/Kconfig
> +++ b/drivers/platform/x86/hp/Kconfig
> @@ -37,6 +37,7 @@ config HP_ACCEL
>  config HP_WMI
>         tristate "HP WMI extras"
>         default m
> +       depends on ACPI_EC
>         depends on ACPI_WMI
>         depends on INPUT
>         depends on RFKILL || RFKILL = n
> diff --git a/drivers/platform/x86/intel/Kconfig b/drivers/platform/x86/intel/Kconfig
> index ad50bbabec61..eb698dcb9af9 100644
> --- a/drivers/platform/x86/intel/Kconfig
> +++ b/drivers/platform/x86/intel/Kconfig
> @@ -62,7 +62,7 @@ config INTEL_INT0002_VGPIO
>
>  config INTEL_OAKTRAIL
>         tristate "Intel Oaktrail Platform Extras"
> -       depends on ACPI
> +       depends on ACPI_EC
>         depends on ACPI_VIDEO || ACPI_VIDEO=n
>         depends on RFKILL && BACKLIGHT_CLASS_DEVICE && ACPI
>         help
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 4d5ee84c468b..7dd24acd9ffe 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -1164,8 +1164,6 @@ int acpi_subsys_suspend_noirq(struct device *dev);
>  int acpi_subsys_suspend(struct device *dev);
>  int acpi_subsys_freeze(struct device *dev);
>  int acpi_subsys_poweroff(struct device *dev);
> -void acpi_ec_mark_gpe_for_wake(void);
> -void acpi_ec_set_gpe_wake_mask(u8 action);
>  int acpi_subsys_restore_early(struct device *dev);
>  #else
>  static inline int acpi_subsys_prepare(struct device *dev) { return 0; }
> @@ -1176,6 +1174,12 @@ static inline int acpi_subsys_suspend(struct device *dev) { return 0; }
>  static inline int acpi_subsys_freeze(struct device *dev) { return 0; }
>  static inline int acpi_subsys_poweroff(struct device *dev) { return 0; }
>  static inline int acpi_subsys_restore_early(struct device *dev) { return 0; }
> +#endif
> +
> +#if defined(CONFIG_ACPI_EC) && defined(CONFIG_PM_SLEEP)
> +void acpi_ec_mark_gpe_for_wake(void);
> +void acpi_ec_set_gpe_wake_mask(u8 action);
> +#else
>  static inline void acpi_ec_mark_gpe_for_wake(void) {}
>  static inline void acpi_ec_set_gpe_wake_mask(u8 action) {}
>  #endif
> --
> 2.39.5
>

Re: [PATCH v2 1/3] acpi: make EC support compile-time conditional

[Rafael J. Wysocki]

On Mon, Oct 21, 2024 at 1:39 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>
> On Fri, Oct 11, 2024 at 8:19 AM Arnd Bergmann <arnd@kernel.org> wrote:
> >
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > The embedded controller code is mainly used on x86 laptops and cannot
> > work without PC style I/O port access.
> >
> > Make this a user-visible configuration option that is default enabled
> > on x86 but otherwise disabled, and that can never be enabled unless
> > CONFIG_HAS_IOPORT is also available.
> >
> > The empty stubs in internal.h help ignore the EC code in configurations
> > that don't support it. In order to see those stubs, the sbshc code also
> > has to include this header and drop duplicate declarations.
> >
> > All the direct callers of ec_read/ec_write already had an x86
> > dependency and now also need to depend on APCI_EC.
> >
> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> I think I can pick up this one as the other two patches in the series
> don't depend on it.
>
> Any concerns about doing that?

No concerns, so applied (as 6.13 material, with minor edits in the subject).

The other two patches in the series need to be updated AFAICS.

Thanks!

> > ---
> >  drivers/acpi/Kconfig               | 11 ++++++++++-
> >  drivers/acpi/Makefile              |  2 +-
> >  drivers/acpi/internal.h            | 25 +++++++++++++++++++++++++
> >  drivers/acpi/sbshc.c               |  9 +--------
> >  drivers/char/Kconfig               |  1 +
> >  drivers/hwmon/Kconfig              |  3 ++-
> >  drivers/platform/x86/Kconfig       | 22 ++++++++++++----------
> >  drivers/platform/x86/dell/Kconfig  |  1 +
> >  drivers/platform/x86/hp/Kconfig    |  1 +
> >  drivers/platform/x86/intel/Kconfig |  2 +-
> >  include/linux/acpi.h               |  8 ++++++--
> >  11 files changed, 61 insertions(+), 24 deletions(-)
> >
> > diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> > index d67f63d93b2a..d65cd08ba8e1 100644
> > --- a/drivers/acpi/Kconfig
> > +++ b/drivers/acpi/Kconfig
> > @@ -132,8 +132,17 @@ config ACPI_REV_OVERRIDE_POSSIBLE
> >           makes it possible to force the kernel to return "5" as the supported
> >           ACPI revision via the "acpi_rev_override" command line switch.
> >
> > +config ACPI_EC
> > +       bool "Embedded Controller"
> > +       depends on HAS_IOPORT
> > +       default X86
> > +       help
> > +         This driver handles communication with the microcontroller
> > +         on many x86 laptops and other machines.
> > +
> >  config ACPI_EC_DEBUGFS
> >         tristate "EC read/write access through /sys/kernel/debug/ec"
> > +       depends on ACPI_EC
> >         help
> >           Say N to disable Embedded Controller /sys/kernel/debug interface
> >
> > @@ -433,7 +442,7 @@ config ACPI_HOTPLUG_IOAPIC
> >
> >  config ACPI_SBS
> >         tristate "Smart Battery System"
> > -       depends on X86
> > +       depends on X86 && ACPI_EC
> >         select POWER_SUPPLY
> >         help
> >           This driver supports the Smart Battery System, another
> > diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> > index 61ca4afe83dc..40208a0f5dfb 100644
> > --- a/drivers/acpi/Makefile
> > +++ b/drivers/acpi/Makefile
> > @@ -41,7 +41,7 @@ acpi-y                                += resource.o
> >  acpi-y                         += acpi_processor.o
> >  acpi-y                         += processor_core.o
> >  acpi-$(CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC) += processor_pdc.o
> > -acpi-y                         += ec.o
> > +acpi-$(CONFIG_ACPI_EC)         += ec.o
> >  acpi-$(CONFIG_ACPI_DOCK)       += dock.o
> >  acpi-$(CONFIG_PCI)             += pci_root.o pci_link.o pci_irq.o
> >  obj-$(CONFIG_ACPI_MCFG)                += pci_mcfg.o
> > diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> > index ced7dff9a5db..00910ccd7eda 100644
> > --- a/drivers/acpi/internal.h
> > +++ b/drivers/acpi/internal.h
> > @@ -215,6 +215,8 @@ extern struct acpi_ec *first_ec;
> >  /* External interfaces use first EC only, so remember */
> >  typedef int (*acpi_ec_query_func) (void *data);
> >
> > +#ifdef CONFIG_ACPI_EC
> > +
> >  void acpi_ec_init(void);
> >  void acpi_ec_ecdt_probe(void);
> >  void acpi_ec_dsdt_probe(void);
> > @@ -231,6 +233,29 @@ void acpi_ec_flush_work(void);
> >  bool acpi_ec_dispatch_gpe(void);
> >  #endif
> >
> > +#else
> > +
> > +static inline void acpi_ec_init(void) {}
> > +static inline void acpi_ec_ecdt_probe(void) {}
> > +static inline void acpi_ec_dsdt_probe(void) {}
> > +static inline void acpi_ec_block_transactions(void) {}
> > +static inline void acpi_ec_unblock_transactions(void) {}
> > +static inline int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
> > +                             acpi_handle handle, acpi_ec_query_func func,
> > +                             void *data)
> > +{
> > +       return -ENXIO;
> > +}
> > +static inline void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit) {}
> > +static inline void acpi_ec_register_opregions(struct acpi_device *adev) {}
> > +
> > +static inline void acpi_ec_flush_work(void) {}
> > +static inline bool acpi_ec_dispatch_gpe(void)
> > +{
> > +       return false;
> > +}
> > +
> > +#endif
> >
> >  /*--------------------------------------------------------------------------
> >                                    Suspend/Resume
> > diff --git a/drivers/acpi/sbshc.c b/drivers/acpi/sbshc.c
> > index 16f2daaa2c45..2b63cd18cca2 100644
> > --- a/drivers/acpi/sbshc.c
> > +++ b/drivers/acpi/sbshc.c
> > @@ -14,6 +14,7 @@
> >  #include <linux/module.h>
> >  #include <linux/interrupt.h>
> >  #include "sbshc.h"
> > +#include "internal.h"
> >
> >  #define ACPI_SMB_HC_CLASS      "smbus_host_ctl"
> >  #define ACPI_SMB_HC_DEVICE_NAME        "ACPI SMBus HC"
> > @@ -236,12 +237,6 @@ static int smbus_alarm(void *context)
> >         return 0;
> >  }
> >
> > -typedef int (*acpi_ec_query_func) (void *data);
> > -
> > -extern int acpi_ec_add_query_handler(struct acpi_ec *ec, u8 query_bit,
> > -                             acpi_handle handle, acpi_ec_query_func func,
> > -                             void *data);
> > -
> >  static int acpi_smbus_hc_add(struct acpi_device *device)
> >  {
> >         int status;
> > @@ -278,8 +273,6 @@ static int acpi_smbus_hc_add(struct acpi_device *device)
> >         return 0;
> >  }
> >
> > -extern void acpi_ec_remove_query_handler(struct acpi_ec *ec, u8 query_bit);
> > -
> >  static void acpi_smbus_hc_remove(struct acpi_device *device)
> >  {
> >         struct acpi_smb_hc *hc;
> > diff --git a/drivers/char/Kconfig b/drivers/char/Kconfig
> > index 7c8dd0abcfdf..8fb33c90482f 100644
> > --- a/drivers/char/Kconfig
> > +++ b/drivers/char/Kconfig
> > @@ -238,6 +238,7 @@ config APPLICOM
> >  config SONYPI
> >         tristate "Sony Vaio Programmable I/O Control Device support"
> >         depends on X86_32 && PCI && INPUT
> > +       depends on ACPI_EC || !ACPI
> >         help
> >           This driver enables access to the Sony Programmable I/O Control
> >           Device which can be found in many (all ?) Sony Vaio laptops.
> > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
> > index 65ea92529406..25ae0a00ea2c 100644
> > --- a/drivers/hwmon/Kconfig
> > +++ b/drivers/hwmon/Kconfig
> > @@ -1747,7 +1747,7 @@ source "drivers/hwmon/occ/Kconfig"
> >
> >  config SENSORS_OXP
> >         tristate "OneXPlayer EC fan control"
> > -       depends on ACPI
> > +       depends on ACPI_EC
> >         depends on X86
> >         help
> >                 If you say yes here you get support for fan readings and control over
> > @@ -2586,6 +2586,7 @@ config SENSORS_ASUS_WMI
> >  config SENSORS_ASUS_EC
> >         tristate "ASUS EC Sensors"
> >         depends on X86
> > +       depends on ACPI_EC
> >         help
> >           If you say yes here you get support for the ACPI embedded controller
> >           hardware monitoring interface found in ASUS motherboards. The driver
> > diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
> > index 3875abba5a79..0258dd879d64 100644
> > --- a/drivers/platform/x86/Kconfig
> > +++ b/drivers/platform/x86/Kconfig
> > @@ -52,6 +52,7 @@ config WMI_BMOF
> >  config HUAWEI_WMI
> >         tristate "Huawei WMI laptop extras driver"
> >         depends on ACPI_BATTERY
> > +       depends on ACPI_EC
> >         depends on ACPI_WMI
> >         depends on INPUT
> >         select INPUT_SPARSEKMAP
> > @@ -147,7 +148,7 @@ config YT2_1380
> >
> >  config ACERHDF
> >         tristate "Acer Aspire One temperature and fan driver"
> > -       depends on ACPI && THERMAL
> > +       depends on ACPI_EC && THERMAL
> >         select THERMAL_GOV_BANG_BANG
> >         help
> >           This is a driver for Acer Aspire One netbooks. It allows to access
> > @@ -186,6 +187,7 @@ config ACER_WMI
> >         depends on SERIO_I8042
> >         depends on INPUT
> >         depends on RFKILL || RFKILL = n
> > +       depends on ACPI_EC
> >         depends on ACPI_WMI
> >         depends on ACPI_VIDEO || ACPI_VIDEO = n
> >         depends on HWMON
> > @@ -334,7 +336,7 @@ config MERAKI_MX100
> >
> >  config EEEPC_LAPTOP
> >         tristate "Eee PC Hotkey Driver"
> > -       depends on ACPI
> > +       depends on ACPI_EC
> >         depends on INPUT
> >         depends on RFKILL || RFKILL = n
> >         depends on ACPI_VIDEO || ACPI_VIDEO = n
> > @@ -503,7 +505,7 @@ config SENSORS_HDAPS
> >
> >  config THINKPAD_ACPI
> >         tristate "ThinkPad ACPI Laptop Extras"
> > -       depends on ACPI
> > +       depends on ACPI_EC
> >         depends on ACPI_BATTERY
> >         depends on INPUT
> >         depends on RFKILL || RFKILL = n
> > @@ -682,7 +684,7 @@ config MEEGOPAD_ANX7428
> >
> >  config MSI_EC
> >         tristate "MSI EC Extras"
> > -       depends on ACPI
> > +       depends on ACPI_EC
> >         depends on ACPI_BATTERY
> >         help
> >           This driver allows various MSI laptops' functionalities to be
> > @@ -690,7 +692,7 @@ config MSI_EC
> >
> >  config MSI_LAPTOP
> >         tristate "MSI Laptop Extras"
> > -       depends on ACPI
> > +       depends on ACPI_EC
> >         depends on BACKLIGHT_CLASS_DEVICE
> >         depends on ACPI_VIDEO || ACPI_VIDEO = n
> >         depends on RFKILL
> > @@ -796,7 +798,7 @@ config SAMSUNG_LAPTOP
> >
> >  config SAMSUNG_Q10
> >         tristate "Samsung Q10 Extras"
> > -       depends on ACPI
> > +       depends on ACPI_EC
> >         select BACKLIGHT_CLASS_DEVICE
> >         help
> >           This driver provides support for backlight control on Samsung Q10
> > @@ -804,7 +806,7 @@ config SAMSUNG_Q10
> >
> >  config ACPI_TOSHIBA
> >         tristate "Toshiba Laptop Extras"
> > -       depends on ACPI
> > +       depends on ACPI_EC
> >         depends on ACPI_BATTERY
> >         depends on ACPI_WMI
> >         select LEDS_CLASS
> > @@ -904,7 +906,7 @@ config ACPI_CMPC
> >
> >  config COMPAL_LAPTOP
> >         tristate "Compal (and others) Laptop Extras"
> > -       depends on ACPI
> > +       depends on ACPI_EC
> >         depends on BACKLIGHT_CLASS_DEVICE
> >         depends on ACPI_VIDEO || ACPI_VIDEO = n
> >         depends on RFKILL
> > @@ -949,7 +951,7 @@ config PANASONIC_LAPTOP
> >
> >  config SONY_LAPTOP
> >         tristate "Sony Laptop Extras"
> > -       depends on ACPI
> > +       depends on ACPI_EC
> >         depends on ACPI_VIDEO || ACPI_VIDEO = n
> >         depends on BACKLIGHT_CLASS_DEVICE
> >         depends on INPUT
> > @@ -972,7 +974,7 @@ config SONYPI_COMPAT
> >
> >  config SYSTEM76_ACPI
> >         tristate "System76 ACPI Driver"
> > -       depends on ACPI
> > +       depends on ACPI_EC
> >         depends on ACPI_BATTERY
> >         depends on HWMON
> >         depends on INPUT
> > diff --git a/drivers/platform/x86/dell/Kconfig b/drivers/platform/x86/dell/Kconfig
> > index 68a49788a396..dc21227dd66e 100644
> > --- a/drivers/platform/x86/dell/Kconfig
> > +++ b/drivers/platform/x86/dell/Kconfig
> > @@ -194,6 +194,7 @@ config DELL_WMI
> >  config DELL_WMI_PRIVACY
> >         bool "Dell WMI Hardware Privacy Support"
> >         depends on DELL_WMI
> > +       depends on ACPI_EC
> >         help
> >           This option adds integration with the "Dell Hardware Privacy"
> >           feature of Dell laptops to the dell-wmi driver.
> > diff --git a/drivers/platform/x86/hp/Kconfig b/drivers/platform/x86/hp/Kconfig
> > index d776761cd5fd..dd51491b9bcd 100644
> > --- a/drivers/platform/x86/hp/Kconfig
> > +++ b/drivers/platform/x86/hp/Kconfig
> > @@ -37,6 +37,7 @@ config HP_ACCEL
> >  config HP_WMI
> >         tristate "HP WMI extras"
> >         default m
> > +       depends on ACPI_EC
> >         depends on ACPI_WMI
> >         depends on INPUT
> >         depends on RFKILL || RFKILL = n
> > diff --git a/drivers/platform/x86/intel/Kconfig b/drivers/platform/x86/intel/Kconfig
> > index ad50bbabec61..eb698dcb9af9 100644
> > --- a/drivers/platform/x86/intel/Kconfig
> > +++ b/drivers/platform/x86/intel/Kconfig
> > @@ -62,7 +62,7 @@ config INTEL_INT0002_VGPIO
> >
> >  config INTEL_OAKTRAIL
> >         tristate "Intel Oaktrail Platform Extras"
> > -       depends on ACPI
> > +       depends on ACPI_EC
> >         depends on ACPI_VIDEO || ACPI_VIDEO=n
> >         depends on RFKILL && BACKLIGHT_CLASS_DEVICE && ACPI
> >         help
> > diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> > index 4d5ee84c468b..7dd24acd9ffe 100644
> > --- a/include/linux/acpi.h
> > +++ b/include/linux/acpi.h
> > @@ -1164,8 +1164,6 @@ int acpi_subsys_suspend_noirq(struct device *dev);
> >  int acpi_subsys_suspend(struct device *dev);
> >  int acpi_subsys_freeze(struct device *dev);
> >  int acpi_subsys_poweroff(struct device *dev);
> > -void acpi_ec_mark_gpe_for_wake(void);
> > -void acpi_ec_set_gpe_wake_mask(u8 action);
> >  int acpi_subsys_restore_early(struct device *dev);
> >  #else
> >  static inline int acpi_subsys_prepare(struct device *dev) { return 0; }
> > @@ -1176,6 +1174,12 @@ static inline int acpi_subsys_suspend(struct device *dev) { return 0; }
> >  static inline int acpi_subsys_freeze(struct device *dev) { return 0; }
> >  static inline int acpi_subsys_poweroff(struct device *dev) { return 0; }
> >  static inline int acpi_subsys_restore_early(struct device *dev) { return 0; }
> > +#endif
> > +
> > +#if defined(CONFIG_ACPI_EC) && defined(CONFIG_PM_SLEEP)
> > +void acpi_ec_mark_gpe_for_wake(void);
> > +void acpi_ec_set_gpe_wake_mask(u8 action);
> > +#else
> >  static inline void acpi_ec_mark_gpe_for_wake(void) {}
> >  static inline void acpi_ec_set_gpe_wake_mask(u8 action) {}
> >  #endif
> > --
> > 2.39.5
> >

Re: [PATCH v2 1/3] acpi: make EC support compile-time conditional

[Arnd Bergmann]

On Thu, Oct 24, 2024, at 15:49, Rafael J. Wysocki wrote:
> On Mon, Oct 21, 2024 at 1:39 PM Rafael J. Wysocki <rafael@kernel.org> wrote:
>> On Fri, Oct 11, 2024 at 8:19 AM Arnd Bergmann <arnd@kernel.org> wrote:
>> >
>> > All the direct callers of ec_read/ec_write already had an x86
>> > dependency and now also need to depend on APCI_EC.
>> >
>> > Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>>
>> I think I can pick up this one as the other two patches in the series
>> don't depend on it.
>>
>> Any concerns about doing that?
>
> No concerns, so applied (as 6.13 material, with minor edits in the subject).
>
> The other two patches in the series need to be updated AFAICS.

Thanks for picking up the first patch, I sent the other
updated patches as well now.

    ARnd