On 5/22/23 14:31, Dan Williams wrote:
Dave Jiang wrote:
Some of the routines in ACPI tables.c can be shared with parsing CDAT.
s,ACPI tables.c,driver/acpi/tables.c,
However, CDAT is used by CXL and can exist on platforms that do not use
ACPI.
Clarify that CDAT is not an ACPI table:
CDAT is a device-provided data structure that is formatted similar to a
platform-provided ACPI table.
Split out the common routine from ACPI to accomodate platforms that
do not support ACPI. The common routines can be built outside of ACPI if
ACPI_TABLES_LIB is selected.
Might be just me but I get confused where this is indicating "ACPI" the
platform vs "CONFIG_ACPI" the code. How about just:
Refactor the table parsing routines in driver/acpi/tables.c into helpers
that can be shared with the CXL driver even in the CONFIG_ACPI=n case.
Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
---
drivers/Makefile | 2
drivers/acpi/Kconfig | 4 +
drivers/acpi/Makefile | 3 +
drivers/acpi/tables.c | 173 ----------------------------------------
drivers/acpi/tables_lib.c | 194 +++++++++++++++++++++++++++++++++++++++++++++
include/linux/acpi.h | 63 +++++++++------
6 files changed, 241 insertions(+), 198 deletions(-)
create mode 100644 drivers/acpi/tables_lib.c
Conversion looks ok to me. Even though the cover letter said "Hi Rafael,
Please consider these for 6.5 merge window" my expectation is to take
these through CXL with ACPI acks.
I thought you wanted Rafael to take the ACPI patches. But going to the
CXL tree works.
One question below:
diff --git a/drivers/Makefile b/drivers/Makefile
index 20b118dca999..1824797f7dfe 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -31,7 +31,7 @@ obj-y += idle/
# IPMI must come before ACPI in order to provide IPMI opregion support
obj-y += char/ipmi/
-obj-$(CONFIG_ACPI) += acpi/
+obj-y += acpi/
# PnP must come after ACPI since it will eventually need to check if acpi
# was used and do nothing if so
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index ccbeab9500ec..ce74a20dc42f 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -6,12 +6,16 @@
config ARCH_SUPPORTS_ACPI
bool
+config ACPI_TABLES_LIB
+ bool
+
menuconfig ACPI
bool "ACPI (Advanced Configuration and Power Interface) Support"
depends on ARCH_SUPPORTS_ACPI
select PNP
select NLS
select CRC32
+ select ACPI_TABLES_LIB
default y if X86
help
Advanced Configuration and Power Interface (ACPI) support for
diff --git a/drivers/acpi/Makefile b/drivers/acpi/Makefile
index feb36c0b9446..4558e2876823 100644
--- a/drivers/acpi/Makefile
+++ b/drivers/acpi/Makefile
@@ -13,6 +13,9 @@ tables.o: $(src)/../../include/$(CONFIG_ACPI_CUSTOM_DSDT_FILE) ;
endif
+obj-$(CONFIG_ACPI_TABLES_LIB) += acpi_tables_lib.o
+acpi_tables_lib-y := tables_lib.o
Why is a separate object name needed?
Not all code in tables.c will be shared. There are ACPI table parsing
specific code in tables.c that CXL does not care about. Or do you mean
just do
obj-$(CONFIG_ACPI_TABLES_LIB) += tables_lib.o
?
