diff mbox series

acpi: allow building without CONFIG_HAS_IOPORT

Message ID 20241004204845.970951-1-arnd@kernel.org (mailing list archive)
State Superseded, archived
Headers show
Series acpi: allow building without CONFIG_HAS_IOPORT | expand

Commit Message

Arnd Bergmann Oct. 4, 2024, 8:47 p.m. UTC
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. 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

In function 'acpi_ec_read_status',
    inlined from 'advance_transaction' at drivers/acpi/ec.c:665:11:
include/asm-generic/io.h:542:14: error: call to '_inb' declared with attribute error: inb()) requires CONFIG_HAS_IOPORT

Since the embedded controller can only exist when port I/O is
active, it makes sense to disable that code on targets that don't
have it. The same is true for anything using acpi_os_read_port()
and similar functions.

Add compile-time conditionals around all of those and their callers.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Should this be split up into smaller patches?
---
 drivers/acpi/Kconfig             | 1 +
 drivers/acpi/Makefile            | 2 +-
 drivers/acpi/acpica/Makefile     | 4 +++-
 drivers/acpi/acpica/evhandler.c  | 3 ++-
 drivers/acpi/acpica/exregion.c   | 2 ++
 drivers/acpi/acpica/hwregs.c     | 6 ++++--
 drivers/acpi/acpica/hwxface.c    | 3 ++-
 drivers/acpi/apei/apei-base.c    | 4 ++++
 drivers/acpi/bus.c               | 9 ++++++---
 drivers/acpi/cppc_acpi.c         | 6 ++++--
 drivers/acpi/osl.c               | 2 ++
 drivers/acpi/processor_perflib.c | 3 ++-
 drivers/acpi/scan.c              | 3 ++-
 13 files changed, 35 insertions(+), 13 deletions(-)

Comments

Rafael J. Wysocki Oct. 7, 2024, 4:04 p.m. UTC | #1
On Fri, Oct 4, 2024 at 10:48 PM Arnd Bergmann <arnd@kernel.org> 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.

Can addressing this be deferred to that point?

> 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
>
> In function 'acpi_ec_read_status',
>     inlined from 'advance_transaction' at drivers/acpi/ec.c:665:11:
> include/asm-generic/io.h:542:14: error: call to '_inb' declared with attribute error: inb()) requires CONFIG_HAS_IOPORT
>
> Since the embedded controller can only exist when port I/O is
> active, it makes sense to disable that code on targets that don't
> have it. The same is true for anything using acpi_os_read_port()
> and similar functions.
>
> Add compile-time conditionals around all of those and their callers.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> Should this be split up into smaller patches?

No need, but the ACPICA part is kind of nasty.

> ---
>  drivers/acpi/Kconfig             | 1 +
>  drivers/acpi/Makefile            | 2 +-
>  drivers/acpi/acpica/Makefile     | 4 +++-
>  drivers/acpi/acpica/evhandler.c  | 3 ++-
>  drivers/acpi/acpica/exregion.c   | 2 ++
>  drivers/acpi/acpica/hwregs.c     | 6 ++++--
>  drivers/acpi/acpica/hwxface.c    | 3 ++-
>  drivers/acpi/apei/apei-base.c    | 4 ++++
>  drivers/acpi/bus.c               | 9 ++++++---
>  drivers/acpi/cppc_acpi.c         | 6 ++++--
>  drivers/acpi/osl.c               | 2 ++
>  drivers/acpi/processor_perflib.c | 3 ++-
>  drivers/acpi/scan.c              | 3 ++-
>  13 files changed, 35 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index b8924077163b..5ec58c4e0332 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -134,6 +134,7 @@ config ACPI_REV_OVERRIDE_POSSIBLE
>
>  config ACPI_EC_DEBUGFS
>         tristate "EC read/write access through /sys/kernel/debug/ec"
> +       depends on HAS_IOPORT
>         help
>           Say N to disable Embedded Controller /sys/kernel/debug interface
>
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 61ca4afe83dc..132357815324 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_HAS_IOPORT)      += 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/acpica/Makefile b/drivers/acpi/acpica/Makefile
> index 8d18af396de9..9ba5e71348cb 100644
> --- a/drivers/acpi/acpica/Makefile
> +++ b/drivers/acpi/acpica/Makefile
> @@ -80,10 +80,12 @@ acpi-y +=           \
>         hwgpe.o         \
>         hwregs.o        \
>         hwsleep.o       \
> -       hwvalid.o       \
>         hwxface.o       \
>         hwxfsleep.o
>
> +acpi-$(CONFIG_HAS_IOPORT) += \
> +       hwvalid.o
> +
>  acpi-$(CONFIG_PCI) += hwpci.o
>  acpi-$(ACPI_FUTURE_USAGE) += hwtimer.o
>
> diff --git a/drivers/acpi/acpica/evhandler.c b/drivers/acpi/acpica/evhandler.c
> index 1c8cb6d924df..20f61936ff9b 100644
> --- a/drivers/acpi/acpica/evhandler.c
> +++ b/drivers/acpi/acpica/evhandler.c
> @@ -358,12 +358,13 @@ acpi_ev_install_space_handler(struct acpi_namespace_node *node,
>                         handler = acpi_ex_system_memory_space_handler;
>                         setup = acpi_ev_system_memory_region_setup;
>                         break;
> -
> +#ifdef CONFIG_HAS_IOPORT
>                 case ACPI_ADR_SPACE_SYSTEM_IO:
>
>                         handler = acpi_ex_system_io_space_handler;
>                         setup = acpi_ev_io_space_region_setup;
>                         break;
> +#endif

All changes like the above in the ACPICA code potentially increase the
number of times when upstream ACPICA patches will have to be ported to
Linux manually, which in turn increases the number of potential
mistakes in the process.

I'd rather avoid making them, if possible.

>  #ifdef ACPI_PCI_CONFIGURED
>                 case ACPI_ADR_SPACE_PCI_CONFIG:
>
> diff --git a/drivers/acpi/acpica/exregion.c b/drivers/acpi/acpica/exregion.c
> index c49b9f8de723..8f96828614ed 100644
> --- a/drivers/acpi/acpica/exregion.c
> +++ b/drivers/acpi/acpica/exregion.c
> @@ -261,6 +261,7 @@ acpi_ex_system_memory_space_handler(u32 function,
>         return_ACPI_STATUS(status);
>  }
>
> +#ifdef CONFIG_HAS_IOPORT
>  /*******************************************************************************
>   *
>   * FUNCTION:    acpi_ex_system_io_space_handler
> @@ -319,6 +320,7 @@ acpi_ex_system_io_space_handler(u32 function,
>
>         return_ACPI_STATUS(status);
>  }
> +#endif
>
>  #ifdef ACPI_PCI_CONFIGURED
>  /*******************************************************************************
> diff --git a/drivers/acpi/acpica/hwregs.c b/drivers/acpi/acpica/hwregs.c
> index f62d5d024205..845d88a01272 100644
> --- a/drivers/acpi/acpica/hwregs.c
> +++ b/drivers/acpi/acpica/hwregs.c
> @@ -239,7 +239,8 @@ acpi_status acpi_hw_read(u64 *value, struct acpi_generic_address *reg)
>                                                         ACPI_DIV_8
>                                                         (access_width),
>                                                         &value64, access_width);
> -                       } else {        /* ACPI_ADR_SPACE_SYSTEM_IO, validated earlier */
> +                       } else if (IS_ENABLED(CONFIG_HAS_IOPORT)) {
> +                               /* ACPI_ADR_SPACE_SYSTEM_IO, validated earlier */
>
>                                 status = acpi_hw_read_port((acpi_io_address)
>                                                            address +
> @@ -336,7 +337,8 @@ acpi_status acpi_hw_write(u64 value, struct acpi_generic_address *reg)
>                                                          ACPI_DIV_8
>                                                          (access_width),
>                                                          value64, access_width);
> -                       } else {        /* ACPI_ADR_SPACE_SYSTEM_IO, validated earlier */
> +                       } else if (IS_ENABLED(CONFIG_HAS_IOPORT)) {
> +                               /* ACPI_ADR_SPACE_SYSTEM_IO, validated earlier */
>
>                                 status = acpi_hw_write_port((acpi_io_address)
>                                                             address +
> diff --git a/drivers/acpi/acpica/hwxface.c b/drivers/acpi/acpica/hwxface.c
> index c31f803995c6..022e706e10a1 100644
> --- a/drivers/acpi/acpica/hwxface.c
> +++ b/drivers/acpi/acpica/hwxface.c
> @@ -45,7 +45,8 @@ acpi_status acpi_reset(void)
>                 return_ACPI_STATUS(AE_NOT_EXIST);
>         }
>
> -       if (reset_reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
> +       if (IS_ENABLED(CONFIG_HAS_IOPORT) &&
> +           reset_reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
>                 /*
>                  * For I/O space, write directly to the OSL. This bypasses the port
>                  * validation mechanism, which may block a valid write to the reset
> diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
> index c7c26872f4ce..19357f951bae 100644
> --- a/drivers/acpi/apei/apei-base.c
> +++ b/drivers/acpi/apei/apei-base.c
> @@ -661,12 +661,14 @@ int apei_read(u64 *val, struct acpi_generic_address *reg)
>                 if (ACPI_FAILURE(status))
>                         return -EIO;
>                 break;
> +#ifdef CONFIG_HAS_IOPORT
>         case ACPI_ADR_SPACE_SYSTEM_IO:
>                 status = acpi_os_read_port(address, (u32 *)val,
>                                            access_bit_width);
>                 if (ACPI_FAILURE(status))
>                         return -EIO;
>                 break;
> +#endif
>         default:
>                 return -EINVAL;
>         }
> @@ -694,11 +696,13 @@ int apei_write(u64 val, struct acpi_generic_address *reg)
>                 if (ACPI_FAILURE(status))
>                         return -EIO;
>                 break;
> +#ifdef CONFIG_HAS_IOPORT
>         case ACPI_ADR_SPACE_SYSTEM_IO:
>                 status = acpi_os_write_port(address, val, access_bit_width);
>                 if (ACPI_FAILURE(status))
>                         return -EIO;
>                 break;
> +#endif
>         default:
>                 return -EINVAL;
>         }
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 16917dc3ad60..535d6a72ce1b 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -1356,7 +1356,8 @@ static int __init acpi_bus_init(void)
>          * Do that before calling acpi_initialize_objects() which may trigger EC
>          * address space accesses.
>          */
> -       acpi_ec_ecdt_probe();
> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
> +               acpi_ec_ecdt_probe();
>
>         status = acpi_enable_subsystem(ACPI_NO_ACPI_ENABLE);
>         if (ACPI_FAILURE(status)) {
> @@ -1391,7 +1392,8 @@ static int __init acpi_bus_init(void)
>          * Maybe EC region is required at bus_scan/acpi_get_devices. So it
>          * is necessary to enable it as early as possible.
>          */
> -       acpi_ec_dsdt_probe();
> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
> +               acpi_ec_dsdt_probe();
>
>         pr_info("Interpreter enabled\n");
>
> @@ -1464,7 +1466,8 @@ static int __init acpi_init(void)
>         acpi_arm_init();
>         acpi_riscv_init();
>         acpi_scan_init();
> -       acpi_ec_init();
> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
> +               acpi_ec_init();
>         acpi_debugfs_init();
>         acpi_sleep_proc_init();
>         acpi_wakeup_device_init();
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 5b06e236aabe..cb545cdfdc19 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 78a81969d90e..28eb5ff123a9 100644
> --- a/drivers/acpi/osl.c
> +++ b/drivers/acpi/osl.c
> @@ -638,6 +638,7 @@ u64 acpi_os_get_timer(void)
>                 (ACPI_100NSEC_PER_SEC / HZ);
>  }
>
> +#ifdef CONFIG_HAS_IOPORT
>  acpi_status acpi_os_read_port(acpi_io_address port, u32 *value, u32 width)
>  {
>         u32 dummy;
> @@ -680,6 +681,7 @@ acpi_status acpi_os_write_port(acpi_io_address port, u32 value, u32 width)
>  }
>
>  EXPORT_SYMBOL(acpi_os_write_port);
> +#endif
>
>  int acpi_os_read_iomem(void __iomem *virt_addr, u64 *value, u32 width)
>  {
> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> index 4265814c74f8..8be453d89ef8 100644
> --- a/drivers/acpi/processor_perflib.c
> +++ b/drivers/acpi/processor_perflib.c
> @@ -455,7 +455,8 @@ int acpi_processor_pstate_control(void)
>  {
>         acpi_status status;
>
> -       if (!acpi_gbl_FADT.smi_command || !acpi_gbl_FADT.pstate_control)
> +       if (!IS_ENABLED(CONFIG_HAS_IOPORT) ||
> +           !acpi_gbl_FADT.smi_command || !acpi_gbl_FADT.pstate_control)
>                 return 0;
>
>         pr_debug("Writing pstate_control [0x%x] to smi_command [0x%x]\n",
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 7ecc401fb97f..9d5e6dd542bf 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2293,7 +2293,8 @@ static int acpi_bus_attach(struct acpi_device *device, void *first_pass)
>         if (device->handler)
>                 goto ok;
>
> -       acpi_ec_register_opregions(device);
> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
> +               acpi_ec_register_opregions(device);
>
>         if (!device->flags.initialized) {
>                 device->flags.power_manageable =
> --
> 2.39.2
>
>
Arnd Bergmann Oct. 7, 2024, 7:22 p.m. UTC | #2
On Mon, Oct 7, 2024, at 16:04, Rafael J. Wysocki wrote:
> On Fri, Oct 4, 2024 at 10:48 PM Arnd Bergmann <arnd@kernel.org> 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.
>
> Can addressing this be deferred to that point?

Yes. I would like to have all of arm64 and riscv be able to turn
off HAS_IOPORT eventually, but nothing depends on doing this
when ACPI is enabled.

>> Since the embedded controller can only exist when port I/O is
>> active, it makes sense to disable that code on targets that don't
>> have it. The same is true for anything using acpi_os_read_port()
>> and similar functions.
>>
>> Add compile-time conditionals around all of those and their callers.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>> Should this be split up into smaller patches?
>
> No need, but the ACPICA part is kind of nasty.

Right, I see.

>> --- a/drivers/acpi/acpica/evhandler.c
>> +++ b/drivers/acpi/acpica/evhandler.c
>> @@ -358,12 +358,13 @@ acpi_ev_install_space_handler(struct acpi_namespace_node *node,
>>                         handler = acpi_ex_system_memory_space_handler;
>>                         setup = acpi_ev_system_memory_region_setup;
>>                         break;
>> -
>> +#ifdef CONFIG_HAS_IOPORT
>>                 case ACPI_ADR_SPACE_SYSTEM_IO:
>>
>>                         handler = acpi_ex_system_io_space_handler;
>>                         setup = acpi_ev_io_space_region_setup;
>>                         break;
>> +#endif
>
> All changes like the above in the ACPICA code potentially increase the
> number of times when upstream ACPICA patches will have to be ported to
> Linux manually, which in turn increases the number of potential
> mistakes in the process.
>
> I'd rather avoid making them, if possible.

Understood. Does that mean that on the flip-side we can change
the drivers/acpi/osl.c portion to turn acpi_os_read_port()
and acpi_os_write_port() into a runtime error for configurations
without port I/O, without causing the same maintenance overhead?

The version below builds fine and doesn't touch acpica but
it's a bit harder to predict what would happen at runtime.

      Arnd


diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index b8924077163b..5ec58c4e0332 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -134,6 +134,7 @@ config ACPI_REV_OVERRIDE_POSSIBLE
 
 config ACPI_EC_DEBUGFS
 	tristate "EC read/write access through /sys/kernel/debug/ec"
+	depends on HAS_IOPORT
 	help
 	  Say N to disable Embedded Controller /sys/kernel/debug interface
 
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 61ca4afe83dc..132357815324 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_HAS_IOPORT)	+= 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/bus.c b/drivers/acpi/bus.c
index 16917dc3ad60..535d6a72ce1b 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1356,7 +1356,8 @@ static int __init acpi_bus_init(void)
 	 * Do that before calling acpi_initialize_objects() which may trigger EC
 	 * address space accesses.
 	 */
-	acpi_ec_ecdt_probe();
+	if (IS_ENABLED(CONFIG_HAS_IOPORT))
+		acpi_ec_ecdt_probe();
 
 	status = acpi_enable_subsystem(ACPI_NO_ACPI_ENABLE);
 	if (ACPI_FAILURE(status)) {
@@ -1391,7 +1392,8 @@ static int __init acpi_bus_init(void)
 	 * Maybe EC region is required at bus_scan/acpi_get_devices. So it
 	 * is necessary to enable it as early as possible.
 	 */
-	acpi_ec_dsdt_probe();
+	if (IS_ENABLED(CONFIG_HAS_IOPORT))
+		acpi_ec_dsdt_probe();
 
 	pr_info("Interpreter enabled\n");
 
@@ -1464,7 +1466,8 @@ static int __init acpi_init(void)
 	acpi_arm_init();
 	acpi_riscv_init();
 	acpi_scan_init();
-	acpi_ec_init();
+	if (IS_ENABLED(CONFIG_HAS_IOPORT))
+		acpi_ec_init();
 	acpi_debugfs_init();
 	acpi_sleep_proc_init();
 	acpi_wakeup_device_init();
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 78a81969d90e..04d3864073ba 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) {
diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
index 4265814c74f8..8be453d89ef8 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -455,7 +455,8 @@ int acpi_processor_pstate_control(void)
 {
 	acpi_status status;
 
-	if (!acpi_gbl_FADT.smi_command || !acpi_gbl_FADT.pstate_control)
+	if (!IS_ENABLED(CONFIG_HAS_IOPORT) ||
+	    !acpi_gbl_FADT.smi_command || !acpi_gbl_FADT.pstate_control)
 		return 0;
 
 	pr_debug("Writing pstate_control [0x%x] to smi_command [0x%x]\n",
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 7ecc401fb97f..9d5e6dd542bf 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2293,7 +2293,8 @@ static int acpi_bus_attach(struct acpi_device *device, void *first_pass)
 	if (device->handler)
 		goto ok;
 
-	acpi_ec_register_opregions(device);
+	if (IS_ENABLED(CONFIG_HAS_IOPORT))
+		acpi_ec_register_opregions(device);
 
 	if (!device->flags.initialized) {
 		device->flags.power_manageable =
Rafael J. Wysocki Oct. 9, 2024, 7:40 p.m. UTC | #3
On Mon, Oct 7, 2024 at 9:23 PM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Mon, Oct 7, 2024, at 16:04, Rafael J. Wysocki wrote:
> > On Fri, Oct 4, 2024 at 10:48 PM Arnd Bergmann <arnd@kernel.org> 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.
> >
> > Can addressing this be deferred to that point?
>
> Yes. I would like to have all of arm64 and riscv be able to turn
> off HAS_IOPORT eventually, but nothing depends on doing this
> when ACPI is enabled.
>
> >> Since the embedded controller can only exist when port I/O is
> >> active, it makes sense to disable that code on targets that don't
> >> have it. The same is true for anything using acpi_os_read_port()
> >> and similar functions.
> >>
> >> Add compile-time conditionals around all of those and their callers.
> >>
> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >> ---
> >> Should this be split up into smaller patches?
> >
> > No need, but the ACPICA part is kind of nasty.
>
> Right, I see.
>
> >> --- a/drivers/acpi/acpica/evhandler.c
> >> +++ b/drivers/acpi/acpica/evhandler.c
> >> @@ -358,12 +358,13 @@ acpi_ev_install_space_handler(struct acpi_namespace_node *node,
> >>                         handler = acpi_ex_system_memory_space_handler;
> >>                         setup = acpi_ev_system_memory_region_setup;
> >>                         break;
> >> -
> >> +#ifdef CONFIG_HAS_IOPORT
> >>                 case ACPI_ADR_SPACE_SYSTEM_IO:
> >>
> >>                         handler = acpi_ex_system_io_space_handler;
> >>                         setup = acpi_ev_io_space_region_setup;
> >>                         break;
> >> +#endif
> >
> > All changes like the above in the ACPICA code potentially increase the
> > number of times when upstream ACPICA patches will have to be ported to
> > Linux manually, which in turn increases the number of potential
> > mistakes in the process.
> >
> > I'd rather avoid making them, if possible.
>
> Understood. Does that mean that on the flip-side we can change
> the drivers/acpi/osl.c portion to turn acpi_os_read_port()
> and acpi_os_write_port() into a runtime error for configurations
> without port I/O, without causing the same maintenance overhead?

Yes, that can be done.

> The version below builds fine and doesn't touch acpica but
> it's a bit harder to predict what would happen at runtime.

Sure.

> diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
> index b8924077163b..5ec58c4e0332 100644
> --- a/drivers/acpi/Kconfig
> +++ b/drivers/acpi/Kconfig
> @@ -134,6 +134,7 @@ config ACPI_REV_OVERRIDE_POSSIBLE
>
>  config ACPI_EC_DEBUGFS
>         tristate "EC read/write access through /sys/kernel/debug/ec"
> +       depends on HAS_IOPORT
>         help
>           Say N to disable Embedded Controller /sys/kernel/debug interface
>
> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> index 61ca4afe83dc..132357815324 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_HAS_IOPORT)      += 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/bus.c b/drivers/acpi/bus.c
> index 16917dc3ad60..535d6a72ce1b 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -1356,7 +1356,8 @@ static int __init acpi_bus_init(void)
>          * Do that before calling acpi_initialize_objects() which may trigger EC
>          * address space accesses.
>          */
> -       acpi_ec_ecdt_probe();
> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
> +               acpi_ec_ecdt_probe();
>
>         status = acpi_enable_subsystem(ACPI_NO_ACPI_ENABLE);
>         if (ACPI_FAILURE(status)) {
> @@ -1391,7 +1392,8 @@ static int __init acpi_bus_init(void)
>          * Maybe EC region is required at bus_scan/acpi_get_devices. So it
>          * is necessary to enable it as early as possible.
>          */
> -       acpi_ec_dsdt_probe();
> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
> +               acpi_ec_dsdt_probe();

The above two changes mean that it is not necessary to compile either
acpi_ec_ecdt_probe() or acpi_ec_dsdt_probe() at all if
CONFIG_HAS_IOPORT is not enabled.

>
>         pr_info("Interpreter enabled\n");
>
> @@ -1464,7 +1466,8 @@ static int __init acpi_init(void)
>         acpi_arm_init();
>         acpi_riscv_init();
>         acpi_scan_init();
> -       acpi_ec_init();
> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
> +               acpi_ec_init();

And this means that the whole EC driver is not going to work at all then.

>         acpi_debugfs_init();
>         acpi_sleep_proc_init();
>         acpi_wakeup_device_init();
> 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 78a81969d90e..04d3864073ba 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) {

The above two changes look reasonable to me.

> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> index 4265814c74f8..8be453d89ef8 100644
> --- a/drivers/acpi/processor_perflib.c
> +++ b/drivers/acpi/processor_perflib.c
> @@ -455,7 +455,8 @@ int acpi_processor_pstate_control(void)
>  {
>         acpi_status status;
>
> -       if (!acpi_gbl_FADT.smi_command || !acpi_gbl_FADT.pstate_control)
> +       if (!IS_ENABLED(CONFIG_HAS_IOPORT) ||
> +           !acpi_gbl_FADT.smi_command || !acpi_gbl_FADT.pstate_control)
>                 return 0;

All of the existing callers of acpi_processor_pstate_control() are x86
which has CONFIG_HAS_IOPORT AFAICS.

And if you care about the code size, acpi_processor_notify_smm() can
go away for !CONFIG_HAS_IOPORT too.

>         pr_debug("Writing pstate_control [0x%x] to smi_command [0x%x]\n",
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 7ecc401fb97f..9d5e6dd542bf 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -2293,7 +2293,8 @@ static int acpi_bus_attach(struct acpi_device *device, void *first_pass)
>         if (device->handler)
>                 goto ok;
>
> -       acpi_ec_register_opregions(device);
> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
> +               acpi_ec_register_opregions(device);

I'd rather have an empty stub of acpi_ec_register_opregions() for
!CONFIG_HAS_IOPORT instead.

>
>         if (!device->flags.initialized) {
>                 device->flags.power_manageable =
Arnd Bergmann Oct. 10, 2024, 5:40 a.m. UTC | #4
On Wed, Oct 9, 2024, at 19:40, Rafael J. Wysocki wrote:
> On Mon, Oct 7, 2024 at 9:23 PM Arnd Bergmann <arnd@arndb.de> wrote:
>> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
>> index 61ca4afe83dc..132357815324 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_HAS_IOPORT)      += 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/bus.c b/drivers/acpi/bus.c
>> index 16917dc3ad60..535d6a72ce1b 100644
>> --- a/drivers/acpi/bus.c
>> +++ b/drivers/acpi/bus.c
>> @@ -1356,7 +1356,8 @@ static int __init acpi_bus_init(void)
>>          * Do that before calling acpi_initialize_objects() which may trigger EC
>>          * address space accesses.
>>          */
>> -       acpi_ec_ecdt_probe();
>> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
>> +               acpi_ec_ecdt_probe();
>>
>>         status = acpi_enable_subsystem(ACPI_NO_ACPI_ENABLE);
>>         if (ACPI_FAILURE(status)) {
>> @@ -1391,7 +1392,8 @@ static int __init acpi_bus_init(void)
>>          * Maybe EC region is required at bus_scan/acpi_get_devices. So it
>>          * is necessary to enable it as early as possible.
>>          */
>> -       acpi_ec_dsdt_probe();
>> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
>> +               acpi_ec_dsdt_probe();
>
> The above two changes mean that it is not necessary to compile either
> acpi_ec_ecdt_probe() or acpi_ec_dsdt_probe() at all if
> CONFIG_HAS_IOPORT is not enabled.

Correct, this is a result of removing ec.o from the list of objects.

>>         pr_info("Interpreter enabled\n");
>>
>> @@ -1464,7 +1466,8 @@ static int __init acpi_init(void)
>>         acpi_arm_init();
>>         acpi_riscv_init();
>>         acpi_scan_init();
>> -       acpi_ec_init();
>> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
>> +               acpi_ec_init();
>
> And this means that the whole EC driver is not going to work at all then.

Correct. The way I read ec.c it makes no sense if acpi_ec_write_cmd()
and acpi_ec_write_data() don't actually access the registers. Is
there anything in ec.c that still makes sense without port I/O?

>> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
>> index 4265814c74f8..8be453d89ef8 100644
>> --- a/drivers/acpi/processor_perflib.c
>> +++ b/drivers/acpi/processor_perflib.c
>> @@ -455,7 +455,8 @@ int acpi_processor_pstate_control(void)
>>  {
>>         acpi_status status;
>>
>> -       if (!acpi_gbl_FADT.smi_command || !acpi_gbl_FADT.pstate_control)
>> +       if (!IS_ENABLED(CONFIG_HAS_IOPORT) ||
>> +           !acpi_gbl_FADT.smi_command || !acpi_gbl_FADT.pstate_control)
>>                 return 0;
>
> All of the existing callers of acpi_processor_pstate_control() are x86
> which has CONFIG_HAS_IOPORT AFAICS.
>
> And if you care about the code size, acpi_processor_notify_smm() can
> go away for !CONFIG_HAS_IOPORT too.
>
>>         pr_debug("Writing pstate_control [0x%x] to smi_command [0x%x]\n",
>> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
>> index 7ecc401fb97f..9d5e6dd542bf 100644
>> --- a/drivers/acpi/scan.c
>> +++ b/drivers/acpi/scan.c
>> @@ -2293,7 +2293,8 @@ static int acpi_bus_attach(struct acpi_device *device, void *first_pass)
>>         if (device->handler)
>>                 goto ok;
>>
>> -       acpi_ec_register_opregions(device);
>> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
>> +               acpi_ec_register_opregions(device);
>
> I'd rather have an empty stub of acpi_ec_register_opregions() for
> !CONFIG_HAS_IOPORT instead.

Fair enough. Should I add stubs for all the global EC functions then?
I also wonder if there should be a separate CONFIG_ACPI_EC symbol
that allows the EC logic to be turned off on non-x86 architectures
even when HAS_IOPORT is otherwise enabled. 

      Arnd
Rafael J. Wysocki Oct. 10, 2024, 10:01 a.m. UTC | #5
On Thu, Oct 10, 2024 at 7:40 AM Arnd Bergmann <arnd@arndb.de> wrote:
>
> On Wed, Oct 9, 2024, at 19:40, Rafael J. Wysocki wrote:
> > On Mon, Oct 7, 2024 at 9:23 PM Arnd Bergmann <arnd@arndb.de> wrote:
> >> diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
> >> index 61ca4afe83dc..132357815324 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_HAS_IOPORT)      += 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/bus.c b/drivers/acpi/bus.c
> >> index 16917dc3ad60..535d6a72ce1b 100644
> >> --- a/drivers/acpi/bus.c
> >> +++ b/drivers/acpi/bus.c
> >> @@ -1356,7 +1356,8 @@ static int __init acpi_bus_init(void)
> >>          * Do that before calling acpi_initialize_objects() which may trigger EC
> >>          * address space accesses.
> >>          */
> >> -       acpi_ec_ecdt_probe();
> >> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
> >> +               acpi_ec_ecdt_probe();
> >>
> >>         status = acpi_enable_subsystem(ACPI_NO_ACPI_ENABLE);
> >>         if (ACPI_FAILURE(status)) {
> >> @@ -1391,7 +1392,8 @@ static int __init acpi_bus_init(void)
> >>          * Maybe EC region is required at bus_scan/acpi_get_devices. So it
> >>          * is necessary to enable it as early as possible.
> >>          */
> >> -       acpi_ec_dsdt_probe();
> >> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
> >> +               acpi_ec_dsdt_probe();
> >
> > The above two changes mean that it is not necessary to compile either
> > acpi_ec_ecdt_probe() or acpi_ec_dsdt_probe() at all if
> > CONFIG_HAS_IOPORT is not enabled.
>
> Correct, this is a result of removing ec.o from the list of objects.
>
> >>         pr_info("Interpreter enabled\n");
> >>
> >> @@ -1464,7 +1466,8 @@ static int __init acpi_init(void)
> >>         acpi_arm_init();
> >>         acpi_riscv_init();
> >>         acpi_scan_init();
> >> -       acpi_ec_init();
> >> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
> >> +               acpi_ec_init();
> >
> > And this means that the whole EC driver is not going to work at all then.
>
> Correct. The way I read ec.c it makes no sense if acpi_ec_write_cmd()
> and acpi_ec_write_data() don't actually access the registers. Is
> there anything in ec.c that still makes sense without port I/O?
>
> >> diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
> >> index 4265814c74f8..8be453d89ef8 100644
> >> --- a/drivers/acpi/processor_perflib.c
> >> +++ b/drivers/acpi/processor_perflib.c
> >> @@ -455,7 +455,8 @@ int acpi_processor_pstate_control(void)
> >>  {
> >>         acpi_status status;
> >>
> >> -       if (!acpi_gbl_FADT.smi_command || !acpi_gbl_FADT.pstate_control)
> >> +       if (!IS_ENABLED(CONFIG_HAS_IOPORT) ||
> >> +           !acpi_gbl_FADT.smi_command || !acpi_gbl_FADT.pstate_control)
> >>                 return 0;
> >
> > All of the existing callers of acpi_processor_pstate_control() are x86
> > which has CONFIG_HAS_IOPORT AFAICS.
> >
> > And if you care about the code size, acpi_processor_notify_smm() can
> > go away for !CONFIG_HAS_IOPORT too.
> >
> >>         pr_debug("Writing pstate_control [0x%x] to smi_command [0x%x]\n",
> >> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> >> index 7ecc401fb97f..9d5e6dd542bf 100644
> >> --- a/drivers/acpi/scan.c
> >> +++ b/drivers/acpi/scan.c
> >> @@ -2293,7 +2293,8 @@ static int acpi_bus_attach(struct acpi_device *device, void *first_pass)
> >>         if (device->handler)
> >>                 goto ok;
> >>
> >> -       acpi_ec_register_opregions(device);
> >> +       if (IS_ENABLED(CONFIG_HAS_IOPORT))
> >> +               acpi_ec_register_opregions(device);
> >
> > I'd rather have an empty stub of acpi_ec_register_opregions() for
> > !CONFIG_HAS_IOPORT instead.
>
> Fair enough. Should I add stubs for all the global EC functions then?

Yes, please.

> I also wonder if there should be a separate CONFIG_ACPI_EC symbol
> that allows the EC logic to be turned off on non-x86 architectures
> even when HAS_IOPORT is otherwise enabled.

Yes, CONFIG_ACPI_EC would make sense IMV.

Thanks!
diff mbox series

Patch

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index b8924077163b..5ec58c4e0332 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -134,6 +134,7 @@  config ACPI_REV_OVERRIDE_POSSIBLE
 
 config ACPI_EC_DEBUGFS
 	tristate "EC read/write access through /sys/kernel/debug/ec"
+	depends on HAS_IOPORT
 	help
 	  Say N to disable Embedded Controller /sys/kernel/debug interface
 
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index 61ca4afe83dc..132357815324 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_HAS_IOPORT)	+= 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/acpica/Makefile b/drivers/acpi/acpica/Makefile
index 8d18af396de9..9ba5e71348cb 100644
--- a/drivers/acpi/acpica/Makefile
+++ b/drivers/acpi/acpica/Makefile
@@ -80,10 +80,12 @@  acpi-y +=		\
 	hwgpe.o		\
 	hwregs.o	\
 	hwsleep.o	\
-	hwvalid.o	\
 	hwxface.o	\
 	hwxfsleep.o
 
+acpi-$(CONFIG_HAS_IOPORT) += \
+	hwvalid.o
+
 acpi-$(CONFIG_PCI) += hwpci.o
 acpi-$(ACPI_FUTURE_USAGE) += hwtimer.o
 
diff --git a/drivers/acpi/acpica/evhandler.c b/drivers/acpi/acpica/evhandler.c
index 1c8cb6d924df..20f61936ff9b 100644
--- a/drivers/acpi/acpica/evhandler.c
+++ b/drivers/acpi/acpica/evhandler.c
@@ -358,12 +358,13 @@  acpi_ev_install_space_handler(struct acpi_namespace_node *node,
 			handler = acpi_ex_system_memory_space_handler;
 			setup = acpi_ev_system_memory_region_setup;
 			break;
-
+#ifdef CONFIG_HAS_IOPORT
 		case ACPI_ADR_SPACE_SYSTEM_IO:
 
 			handler = acpi_ex_system_io_space_handler;
 			setup = acpi_ev_io_space_region_setup;
 			break;
+#endif
 #ifdef ACPI_PCI_CONFIGURED
 		case ACPI_ADR_SPACE_PCI_CONFIG:
 
diff --git a/drivers/acpi/acpica/exregion.c b/drivers/acpi/acpica/exregion.c
index c49b9f8de723..8f96828614ed 100644
--- a/drivers/acpi/acpica/exregion.c
+++ b/drivers/acpi/acpica/exregion.c
@@ -261,6 +261,7 @@  acpi_ex_system_memory_space_handler(u32 function,
 	return_ACPI_STATUS(status);
 }
 
+#ifdef CONFIG_HAS_IOPORT
 /*******************************************************************************
  *
  * FUNCTION:    acpi_ex_system_io_space_handler
@@ -319,6 +320,7 @@  acpi_ex_system_io_space_handler(u32 function,
 
 	return_ACPI_STATUS(status);
 }
+#endif
 
 #ifdef ACPI_PCI_CONFIGURED
 /*******************************************************************************
diff --git a/drivers/acpi/acpica/hwregs.c b/drivers/acpi/acpica/hwregs.c
index f62d5d024205..845d88a01272 100644
--- a/drivers/acpi/acpica/hwregs.c
+++ b/drivers/acpi/acpica/hwregs.c
@@ -239,7 +239,8 @@  acpi_status acpi_hw_read(u64 *value, struct acpi_generic_address *reg)
 							ACPI_DIV_8
 							(access_width),
 							&value64, access_width);
-			} else {	/* ACPI_ADR_SPACE_SYSTEM_IO, validated earlier */
+			} else if (IS_ENABLED(CONFIG_HAS_IOPORT)) {
+				/* ACPI_ADR_SPACE_SYSTEM_IO, validated earlier */
 
 				status = acpi_hw_read_port((acpi_io_address)
 							   address +
@@ -336,7 +337,8 @@  acpi_status acpi_hw_write(u64 value, struct acpi_generic_address *reg)
 							 ACPI_DIV_8
 							 (access_width),
 							 value64, access_width);
-			} else {	/* ACPI_ADR_SPACE_SYSTEM_IO, validated earlier */
+			} else if (IS_ENABLED(CONFIG_HAS_IOPORT)) {
+				/* ACPI_ADR_SPACE_SYSTEM_IO, validated earlier */
 
 				status = acpi_hw_write_port((acpi_io_address)
 							    address +
diff --git a/drivers/acpi/acpica/hwxface.c b/drivers/acpi/acpica/hwxface.c
index c31f803995c6..022e706e10a1 100644
--- a/drivers/acpi/acpica/hwxface.c
+++ b/drivers/acpi/acpica/hwxface.c
@@ -45,7 +45,8 @@  acpi_status acpi_reset(void)
 		return_ACPI_STATUS(AE_NOT_EXIST);
 	}
 
-	if (reset_reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
+	if (IS_ENABLED(CONFIG_HAS_IOPORT) &&
+	    reset_reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
 		/*
 		 * For I/O space, write directly to the OSL. This bypasses the port
 		 * validation mechanism, which may block a valid write to the reset
diff --git a/drivers/acpi/apei/apei-base.c b/drivers/acpi/apei/apei-base.c
index c7c26872f4ce..19357f951bae 100644
--- a/drivers/acpi/apei/apei-base.c
+++ b/drivers/acpi/apei/apei-base.c
@@ -661,12 +661,14 @@  int apei_read(u64 *val, struct acpi_generic_address *reg)
 		if (ACPI_FAILURE(status))
 			return -EIO;
 		break;
+#ifdef CONFIG_HAS_IOPORT
 	case ACPI_ADR_SPACE_SYSTEM_IO:
 		status = acpi_os_read_port(address, (u32 *)val,
 					   access_bit_width);
 		if (ACPI_FAILURE(status))
 			return -EIO;
 		break;
+#endif
 	default:
 		return -EINVAL;
 	}
@@ -694,11 +696,13 @@  int apei_write(u64 val, struct acpi_generic_address *reg)
 		if (ACPI_FAILURE(status))
 			return -EIO;
 		break;
+#ifdef CONFIG_HAS_IOPORT
 	case ACPI_ADR_SPACE_SYSTEM_IO:
 		status = acpi_os_write_port(address, val, access_bit_width);
 		if (ACPI_FAILURE(status))
 			return -EIO;
 		break;
+#endif
 	default:
 		return -EINVAL;
 	}
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 16917dc3ad60..535d6a72ce1b 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -1356,7 +1356,8 @@  static int __init acpi_bus_init(void)
 	 * Do that before calling acpi_initialize_objects() which may trigger EC
 	 * address space accesses.
 	 */
-	acpi_ec_ecdt_probe();
+	if (IS_ENABLED(CONFIG_HAS_IOPORT))
+		acpi_ec_ecdt_probe();
 
 	status = acpi_enable_subsystem(ACPI_NO_ACPI_ENABLE);
 	if (ACPI_FAILURE(status)) {
@@ -1391,7 +1392,8 @@  static int __init acpi_bus_init(void)
 	 * Maybe EC region is required at bus_scan/acpi_get_devices. So it
 	 * is necessary to enable it as early as possible.
 	 */
-	acpi_ec_dsdt_probe();
+	if (IS_ENABLED(CONFIG_HAS_IOPORT))
+		acpi_ec_dsdt_probe();
 
 	pr_info("Interpreter enabled\n");
 
@@ -1464,7 +1466,8 @@  static int __init acpi_init(void)
 	acpi_arm_init();
 	acpi_riscv_init();
 	acpi_scan_init();
-	acpi_ec_init();
+	if (IS_ENABLED(CONFIG_HAS_IOPORT))
+		acpi_ec_init();
 	acpi_debugfs_init();
 	acpi_sleep_proc_init();
 	acpi_wakeup_device_init();
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 5b06e236aabe..cb545cdfdc19 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 78a81969d90e..28eb5ff123a9 100644
--- a/drivers/acpi/osl.c
+++ b/drivers/acpi/osl.c
@@ -638,6 +638,7 @@  u64 acpi_os_get_timer(void)
 		(ACPI_100NSEC_PER_SEC / HZ);
 }
 
+#ifdef CONFIG_HAS_IOPORT
 acpi_status acpi_os_read_port(acpi_io_address port, u32 *value, u32 width)
 {
 	u32 dummy;
@@ -680,6 +681,7 @@  acpi_status acpi_os_write_port(acpi_io_address port, u32 value, u32 width)
 }
 
 EXPORT_SYMBOL(acpi_os_write_port);
+#endif
 
 int acpi_os_read_iomem(void __iomem *virt_addr, u64 *value, u32 width)
 {
diff --git a/drivers/acpi/processor_perflib.c b/drivers/acpi/processor_perflib.c
index 4265814c74f8..8be453d89ef8 100644
--- a/drivers/acpi/processor_perflib.c
+++ b/drivers/acpi/processor_perflib.c
@@ -455,7 +455,8 @@  int acpi_processor_pstate_control(void)
 {
 	acpi_status status;
 
-	if (!acpi_gbl_FADT.smi_command || !acpi_gbl_FADT.pstate_control)
+	if (!IS_ENABLED(CONFIG_HAS_IOPORT) ||
+	    !acpi_gbl_FADT.smi_command || !acpi_gbl_FADT.pstate_control)
 		return 0;
 
 	pr_debug("Writing pstate_control [0x%x] to smi_command [0x%x]\n",
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 7ecc401fb97f..9d5e6dd542bf 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -2293,7 +2293,8 @@  static int acpi_bus_attach(struct acpi_device *device, void *first_pass)
 	if (device->handler)
 		goto ok;
 
-	acpi_ec_register_opregions(device);
+	if (IS_ENABLED(CONFIG_HAS_IOPORT))
+		acpi_ec_register_opregions(device);
 
 	if (!device->flags.initialized) {
 		device->flags.power_manageable =