On Mon, Dec 27, 2021 at 6:43 PM Niklas Schnelle [off-list ref] wrote:

On Mon, 2021-12-27 at 18:15 +0100, Rafael J. Wysocki wrote:

quoted

On Mon, Dec 27, 2021 at 6:12 PM Rafael J. Wysocki [off-list ref] wrote:

quoted

On Mon, Dec 27, 2021 at 6:02 PM Niklas Schnelle [off-list ref] wrote:

quoted

On Mon, 2021-12-27 at 17:47 +0100, Rafael J. Wysocki wrote:

quoted

On Mon, Dec 27, 2021 at 5:44 PM Niklas Schnelle [off-list ref] wrote:

quoted

In a future patch HAS_IOPORT=n will result in inb()/outb() and friends
not being declared. As ACPI always uses I/O port access

The ARM64 people may not agree with this.

Maybe my wording is bad. This is my rewording of what Arnd had in his
original mail: "The ACPI subsystem needs access to I/O ports, so that
also gets a dependency."(
https://lore.kernel.org/lkml/CAK8P3a0MNbx-iuzW_-=0ab6-TTZzwV-PT_6gAC1Gp5PgYyHcrA@mail.gmail.com/ (local)
).

And my point is that on ARM64 the ACPI subsystem does not need to
access IO ports.

It may not even need to access them on x86, but that depends on the
platform firmware in use.

Well at least it does compile code calling outb() in
drivers/acpi/ec.c:acpi_ec_write_cmd().

That's the EC driver which is not used on arm64 AFAICS and that driver
itself can be made depend on HAS_IOPORT.

Not sure if there is an
alternative path at runtime if there is then we might want to instead
use ifdefs to always use the non I/O port path if HAS_IOPORT is
undefined.

quoted

quoted

If arm64 is going to set HAS_IOPORT, then fine, but is it (and this
applies to ia64 too)?

Yes x86, arm64 and ia64 that is all arches that set ACH_SUPPORTS_ACPI
all select HAS_IOPORT too. See patch 02 or the summary in the cover
letter which notes that only s390, nds32, um, h8300, nios2, openrisc,
hexagon, csky, and xtensa do not select it.

If that is the case, there should be no need to add the extra
dependency to CONFIG_ACPI.

quoted

quoted

quoted

quoted

quoted

we depend on HAS_IOPORT unconditionally.

Co-developed-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
---
 drivers/acpi/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index cdbdf68bd98f..b57f15817ede 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig

@@ -9,6 +9,7 @@ config ARCH_SUPPORTS_ACPI
 menuconfig ACPI
        bool "ACPI (Advanced Configuration and Power Interface) Support"
        depends on ARCH_SUPPORTS_ACPI

Besides, I'm not sure why ARCH_SUPPORTS_ACPI cannot cover this new dependency.

If you prefer to have the dependency there that should work too yes.

I would prefer that.

IMO, if ARCH_SUPPORTS_ACPI is set, all of the requisite dependencies
should be met.