Re: [PATCH v2 1/4] acpi: Move common tables helper functions to common lib

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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

?





[Index of Archives]     [Linux IBM ACPI]     [Linux Power Management]     [Linux Kernel]     [Linux Laptop]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]
  Powered by Linux