[PATCH v1 0/4] gpiolib: acpi: Split quirks to its own file

[PATCH v1 0/4] gpiolib: acpi: Split quirks to its own file

[Andy Shevchenko]

The GPIO ACPI helpers use a few quirks which consumes approximately 20%
of the file. Besides that the necessary bits are sparse and being directly
referred. Split them to a separate file. There is no functional change.

For the new file I used the Hans' authorship of Hans as he the author of
all those bits (expect very tiny changes made by this series).

Hans, please check if it's okay and confirm, or suggest better alternative.

Andy Shevchenko (4):
  gpiolib: acpi: Switch to use enum in acpi_gpio_in_ignore_list()
  gpiolib: acpi: Handle deferred list via new API
  gpiolib: acpi: Add acpi_gpio_need_run_edge_events_on_boot() getter
  gpiolib: acpi: Move quirks to a separate file

 drivers/gpio/Makefile                         |   1 +
 .../{gpiolib-acpi.c => gpiolib-acpi-core.c}   | 344 +----------------
 drivers/gpio/gpiolib-acpi-quirks.c            | 363 ++++++++++++++++++
 drivers/gpio/gpiolib-acpi.h                   |  15 +
 4 files changed, 392 insertions(+), 331 deletions(-)
 rename drivers/gpio/{gpiolib-acpi.c => gpiolib-acpi-core.c} (79%)
 create mode 100644 drivers/gpio/gpiolib-acpi-quirks.c

-- 
2.47.2

[PATCH v1 1/4] gpiolib: acpi: Switch to use enum in acpi_gpio_in_ignore_list()

[Andy Shevchenko]

Switch to use enum instead of pointers in acpi_gpio_in_ignore_list()
which moves towards isolating the GPIO ACPI and quirk APIs. It will
helps splitting them completely in the next changes.

No functional changes.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 21 ++++++++++++++++-----
 drivers/gpio/gpiolib-acpi.h |  8 ++++++++
 2 files changed, 24 insertions(+), 5 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 3c9535d767e7..fb573b5f0ba1 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -350,14 +350,25 @@ static struct gpio_desc *acpi_request_own_gpiod(struct gpio_chip *chip,
 	return desc;
 }
 
-static bool acpi_gpio_in_ignore_list(const char *ignore_list, const char *controller_in,
-				     unsigned int pin_in)
+bool acpi_gpio_in_ignore_list(enum acpi_gpio_ignore_list list, const char *controller_in,
+			      unsigned int pin_in)
 {
-	const char *controller, *pin_str;
+	const char *ignore_list, *controller, *pin_str;
 	unsigned int pin;
 	char *endp;
 	int len;
 
+	switch (list) {
+	case ACPI_GPIO_IGNORE_WAKE:
+		ignore_list = ignore_wake;
+		break;
+	case ACPI_GPIO_IGNORE_INTERRUPT:
+		ignore_list = ignore_interrupt;
+		break;
+	default:
+		return false;
+	}
+
 	controller = ignore_list;
 	while (controller) {
 		pin_str = strchr(controller, '@');
@@ -394,7 +405,7 @@ static bool acpi_gpio_irq_is_wake(struct device *parent,
 	if (agpio->wake_capable != ACPI_WAKE_CAPABLE)
 		return false;
 
-	if (acpi_gpio_in_ignore_list(ignore_wake, dev_name(parent), pin)) {
+	if (acpi_gpio_in_ignore_list(ACPI_GPIO_IGNORE_WAKE, dev_name(parent), pin)) {
 		dev_info(parent, "Ignoring wakeup on pin %u\n", pin);
 		return false;
 	}
@@ -437,7 +448,7 @@ static acpi_status acpi_gpiochip_alloc_event(struct acpi_resource *ares,
 	if (!handler)
 		return AE_OK;
 
-	if (acpi_gpio_in_ignore_list(ignore_interrupt, dev_name(chip->parent), pin)) {
+	if (acpi_gpio_in_ignore_list(ACPI_GPIO_IGNORE_INTERRUPT, dev_name(chip->parent), pin)) {
 		dev_info(chip->parent, "Ignoring interrupt on pin %u\n", pin);
 		return AE_OK;
 	}
diff --git a/drivers/gpio/gpiolib-acpi.h b/drivers/gpio/gpiolib-acpi.h
index 7e1c51d04040..ef0b1a3c85d7 100644
--- a/drivers/gpio/gpiolib-acpi.h
+++ b/drivers/gpio/gpiolib-acpi.h
@@ -58,4 +58,12 @@ static inline int acpi_gpio_count(const struct fwnode_handle *fwnode,
 }
 #endif
 
+enum acpi_gpio_ignore_list {
+	ACPI_GPIO_IGNORE_WAKE,
+	ACPI_GPIO_IGNORE_INTERRUPT,
+};
+
+bool acpi_gpio_in_ignore_list(enum acpi_gpio_ignore_list list,
+			      const char *controller_in, unsigned int pin_in);
+
 #endif /* GPIOLIB_ACPI_H */
-- 
2.47.2

[PATCH v1 2/4] gpiolib: acpi: Handle deferred list via new API

[Andy Shevchenko]

Introduce a new API and handle deferred list via it which moves
towards isolating the GPIO ACPI and quirk APIs. It will helps
splitting them completely in the next changes.

No functional changes.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 52 +++++++++++++++++++++++--------------
 drivers/gpio/gpiolib-acpi.h |  5 ++++
 2 files changed, 37 insertions(+), 20 deletions(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index fb573b5f0ba1..ac5f8ec6e72b 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -350,6 +350,27 @@ static struct gpio_desc *acpi_request_own_gpiod(struct gpio_chip *chip,
 	return desc;
 }
 
+bool acpi_gpio_add_to_deferred_list(struct list_head *list)
+{
+	bool defer;
+
+	mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
+	defer = !acpi_gpio_deferred_req_irqs_done;
+	if (defer)
+		list_add(list, &acpi_gpio_deferred_req_irqs_list);
+	mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
+
+	return defer;
+}
+
+void acpi_gpio_remove_from_deferred_list(struct list_head *list)
+{
+	mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
+	if (!list_empty(list))
+		list_del_init(list);
+	mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
+}
+
 bool acpi_gpio_in_ignore_list(enum acpi_gpio_ignore_list list, const char *controller_in,
 			      unsigned int pin_in)
 {
@@ -536,7 +557,6 @@ void acpi_gpiochip_request_interrupts(struct gpio_chip *chip)
 	struct acpi_gpio_chip *acpi_gpio;
 	acpi_handle handle;
 	acpi_status status;
-	bool defer;
 
 	if (!chip->parent || !chip->to_irq)
 		return;
@@ -555,14 +575,7 @@ void acpi_gpiochip_request_interrupts(struct gpio_chip *chip)
 	acpi_walk_resources(handle, METHOD_NAME__AEI,
 			    acpi_gpiochip_alloc_event, acpi_gpio);
 
-	mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
-	defer = !acpi_gpio_deferred_req_irqs_done;
-	if (defer)
-		list_add(&acpi_gpio->deferred_req_irqs_list_entry,
-			 &acpi_gpio_deferred_req_irqs_list);
-	mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
-
-	if (defer)
+	if (acpi_gpio_add_to_deferred_list(&acpi_gpio->deferred_req_irqs_list_entry))
 		return;
 
 	acpi_gpiochip_request_irqs(acpi_gpio);
@@ -594,10 +607,7 @@ void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
 	if (ACPI_FAILURE(status))
 		return;
 
-	mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
-	if (!list_empty(&acpi_gpio->deferred_req_irqs_list_entry))
-		list_del_init(&acpi_gpio->deferred_req_irqs_list_entry);
-	mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
+	acpi_gpio_remove_from_deferred_list(&acpi_gpio->deferred_req_irqs_list_entry);
 
 	list_for_each_entry_safe_reverse(event, ep, &acpi_gpio->events, node) {
 		if (event->irq_requested) {
@@ -615,6 +625,14 @@ void acpi_gpiochip_free_interrupts(struct gpio_chip *chip)
 }
 EXPORT_SYMBOL_GPL(acpi_gpiochip_free_interrupts);
 
+void __init acpi_gpio_process_deferred_list(struct list_head *list)
+{
+	struct acpi_gpio_chip *acpi_gpio, *tmp;
+
+	list_for_each_entry_safe(acpi_gpio, tmp, list, deferred_req_irqs_list_entry)
+		acpi_gpiochip_request_irqs(acpi_gpio);
+}
+
 int acpi_dev_add_driver_gpios(struct acpi_device *adev,
 			      const struct acpi_gpio_mapping *gpios)
 {
@@ -1503,14 +1521,8 @@ int acpi_gpio_count(const struct fwnode_handle *fwnode, const char *con_id)
 /* Run deferred acpi_gpiochip_request_irqs() */
 static int __init acpi_gpio_handle_deferred_request_irqs(void)
 {
-	struct acpi_gpio_chip *acpi_gpio, *tmp;
-
 	mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
-	list_for_each_entry_safe(acpi_gpio, tmp,
-				 &acpi_gpio_deferred_req_irqs_list,
-				 deferred_req_irqs_list_entry)
-		acpi_gpiochip_request_irqs(acpi_gpio);
-
+	acpi_gpio_process_deferred_list(&acpi_gpio_deferred_req_irqs_list);
 	acpi_gpio_deferred_req_irqs_done = true;
 	mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
 
diff --git a/drivers/gpio/gpiolib-acpi.h b/drivers/gpio/gpiolib-acpi.h
index ef0b1a3c85d7..8249977e6140 100644
--- a/drivers/gpio/gpiolib-acpi.h
+++ b/drivers/gpio/gpiolib-acpi.h
@@ -58,6 +58,11 @@ static inline int acpi_gpio_count(const struct fwnode_handle *fwnode,
 }
 #endif
 
+void acpi_gpio_process_deferred_list(struct list_head *list);
+
+bool acpi_gpio_add_to_deferred_list(struct list_head *list);
+void acpi_gpio_remove_from_deferred_list(struct list_head *list);
+
 enum acpi_gpio_ignore_list {
 	ACPI_GPIO_IGNORE_WAKE,
 	ACPI_GPIO_IGNORE_INTERRUPT,
-- 
2.47.2

[PATCH v1 3/4] gpiolib: acpi: Add acpi_gpio_need_run_edge_events_on_boot() getter

[Andy Shevchenko]

Add acpi_gpio_need_run_edge_events_on_boot() getter which moves
towards isolating the GPIO ACPI and quirk APIs. It will helps
splitting them completely in the next changes.

No functional changes.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/gpiolib-acpi.c | 7 ++++++-
 drivers/gpio/gpiolib-acpi.h | 2 ++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index ac5f8ec6e72b..609e3a7f9636 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -268,7 +268,7 @@ static void acpi_gpiochip_request_irq(struct acpi_gpio_chip *acpi_gpio,
 	event->irq_requested = true;
 
 	/* Make sure we trigger the initial state of edge-triggered IRQs */
-	if (run_edge_events_on_boot &&
+	if (acpi_gpio_need_run_edge_events_on_boot() &&
 	    (event->irqflags & (IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING))) {
 		value = gpiod_get_raw_value_cansleep(event->desc);
 		if (((event->irqflags & IRQF_TRIGGER_RISING) && value == 1) ||
@@ -371,6 +371,11 @@ void acpi_gpio_remove_from_deferred_list(struct list_head *list)
 	mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
 }
 
+int acpi_gpio_need_run_edge_events_on_boot(void)
+{
+	return run_edge_events_on_boot;
+}
+
 bool acpi_gpio_in_ignore_list(enum acpi_gpio_ignore_list list, const char *controller_in,
 			      unsigned int pin_in)
 {
diff --git a/drivers/gpio/gpiolib-acpi.h b/drivers/gpio/gpiolib-acpi.h
index 8249977e6140..a90267470a4e 100644
--- a/drivers/gpio/gpiolib-acpi.h
+++ b/drivers/gpio/gpiolib-acpi.h
@@ -63,6 +63,8 @@ void acpi_gpio_process_deferred_list(struct list_head *list);
 bool acpi_gpio_add_to_deferred_list(struct list_head *list);
 void acpi_gpio_remove_from_deferred_list(struct list_head *list);
 
+int acpi_gpio_need_run_edge_events_on_boot(void);
+
 enum acpi_gpio_ignore_list {
 	ACPI_GPIO_IGNORE_WAKE,
 	ACPI_GPIO_IGNORE_INTERRUPT,
-- 
2.47.2

[PATCH v1 4/4] gpiolib: acpi: Move quirks to a separate file

[Andy Shevchenko]

The gpiolib-acpi.c is huge enough even without DMI quirks.
Move them to a separate file for a better maintenance.

No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/gpio/Makefile                         |   1 +
 .../{gpiolib-acpi.c => gpiolib-acpi-core.c}   | 346 -----------------
 drivers/gpio/gpiolib-acpi-quirks.c            | 363 ++++++++++++++++++
 3 files changed, 364 insertions(+), 346 deletions(-)
 rename drivers/gpio/{gpiolib-acpi.c => gpiolib-acpi-core.c} (79%)
 create mode 100644 drivers/gpio/gpiolib-acpi-quirks.c

diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 9aabbb9cb4c6..7b4ba3475f0b 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_OF_GPIO)		+= gpiolib-of.o
 obj-$(CONFIG_GPIO_CDEV)		+= gpiolib-cdev.o
 obj-$(CONFIG_GPIO_SYSFS)	+= gpiolib-sysfs.o
 obj-$(CONFIG_GPIO_ACPI)		+= gpiolib-acpi.o
+gpiolib-acpi-y			:= gpiolib-acpi-core.o gpiolib-acpi-quirks.o
 obj-$(CONFIG_GPIOLIB)		+= gpiolib-swnode.o
 
 # Device drivers. Generally keep list sorted alphabetically
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi-core.c
similarity index 79%
rename from drivers/gpio/gpiolib-acpi.c
rename to drivers/gpio/gpiolib-acpi-core.c
index 609e3a7f9636..12b24a717e43 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi-core.c
@@ -23,29 +23,6 @@
 #include "gpiolib.h"
 #include "gpiolib-acpi.h"
 
-static int run_edge_events_on_boot = -1;
-module_param(run_edge_events_on_boot, int, 0444);
-MODULE_PARM_DESC(run_edge_events_on_boot,
-		 "Run edge _AEI event-handlers at boot: 0=no, 1=yes, -1=auto");
-
-static char *ignore_wake;
-module_param(ignore_wake, charp, 0444);
-MODULE_PARM_DESC(ignore_wake,
-		 "controller@pin combos on which to ignore the ACPI wake flag "
-		 "ignore_wake=controller@pin[,controller@pin[,...]]");
-
-static char *ignore_interrupt;
-module_param(ignore_interrupt, charp, 0444);
-MODULE_PARM_DESC(ignore_interrupt,
-		 "controller@pin combos on which to ignore interrupt "
-		 "ignore_interrupt=controller@pin[,controller@pin[,...]]");
-
-struct acpi_gpiolib_dmi_quirk {
-	bool no_edge_events_on_boot;
-	char *ignore_wake;
-	char *ignore_interrupt;
-};
-
 /**
  * struct acpi_gpio_event - ACPI GPIO event handler data
  *
@@ -115,17 +92,6 @@ struct acpi_gpio_info {
 	unsigned int quirks;
 };
 
-/*
- * For GPIO chips which call acpi_gpiochip_request_interrupts() before late_init
- * (so builtin drivers) we register the ACPI GpioInt IRQ handlers from a
- * late_initcall_sync() handler, so that other builtin drivers can register their
- * OpRegions before the event handlers can run. This list contains GPIO chips
- * for which the acpi_gpiochip_request_irqs() call has been deferred.
- */
-static DEFINE_MUTEX(acpi_gpio_deferred_req_irqs_lock);
-static LIST_HEAD(acpi_gpio_deferred_req_irqs_list);
-static bool acpi_gpio_deferred_req_irqs_done;
-
 static int acpi_gpiochip_find(struct gpio_chip *gc, const void *data)
 {
 	/* First check the actual GPIO device */
@@ -350,79 +316,6 @@ static struct gpio_desc *acpi_request_own_gpiod(struct gpio_chip *chip,
 	return desc;
 }
 
-bool acpi_gpio_add_to_deferred_list(struct list_head *list)
-{
-	bool defer;
-
-	mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
-	defer = !acpi_gpio_deferred_req_irqs_done;
-	if (defer)
-		list_add(list, &acpi_gpio_deferred_req_irqs_list);
-	mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
-
-	return defer;
-}
-
-void acpi_gpio_remove_from_deferred_list(struct list_head *list)
-{
-	mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
-	if (!list_empty(list))
-		list_del_init(list);
-	mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
-}
-
-int acpi_gpio_need_run_edge_events_on_boot(void)
-{
-	return run_edge_events_on_boot;
-}
-
-bool acpi_gpio_in_ignore_list(enum acpi_gpio_ignore_list list, const char *controller_in,
-			      unsigned int pin_in)
-{
-	const char *ignore_list, *controller, *pin_str;
-	unsigned int pin;
-	char *endp;
-	int len;
-
-	switch (list) {
-	case ACPI_GPIO_IGNORE_WAKE:
-		ignore_list = ignore_wake;
-		break;
-	case ACPI_GPIO_IGNORE_INTERRUPT:
-		ignore_list = ignore_interrupt;
-		break;
-	default:
-		return false;
-	}
-
-	controller = ignore_list;
-	while (controller) {
-		pin_str = strchr(controller, '@');
-		if (!pin_str)
-			goto err;
-
-		len = pin_str - controller;
-		if (len == strlen(controller_in) &&
-		    strncmp(controller, controller_in, len) == 0) {
-			pin = simple_strtoul(pin_str + 1, &endp, 10);
-			if (*endp != 0 && *endp != ',')
-				goto err;
-
-			if (pin == pin_in)
-				return true;
-		}
-
-		controller = strchr(controller, ',');
-		if (controller)
-			controller++;
-	}
-
-	return false;
-err:
-	pr_err_once("Error: Invalid value for gpiolib_acpi.ignore_...: %s\n", ignore_list);
-	return false;
-}
-
 static bool acpi_gpio_irq_is_wake(struct device *parent,
 				  const struct acpi_resource_gpio *agpio)
 {
@@ -1522,242 +1415,3 @@ int acpi_gpio_count(const struct fwnode_handle *fwnode, const char *con_id)
 	}
 	return count ? count : -ENOENT;
 }
-
-/* Run deferred acpi_gpiochip_request_irqs() */
-static int __init acpi_gpio_handle_deferred_request_irqs(void)
-{
-	mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
-	acpi_gpio_process_deferred_list(&acpi_gpio_deferred_req_irqs_list);
-	acpi_gpio_deferred_req_irqs_done = true;
-	mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
-
-	return 0;
-}
-/* We must use _sync so that this runs after the first deferred_probe run */
-late_initcall_sync(acpi_gpio_handle_deferred_request_irqs);
-
-static const struct dmi_system_id gpiolib_acpi_quirks[] __initconst = {
-	{
-		/*
-		 * The Minix Neo Z83-4 has a micro-USB-B id-pin handler for
-		 * a non existing micro-USB-B connector which puts the HDMI
-		 * DDC pins in GPIO mode, breaking HDMI support.
-		 */
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "MINIX"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "Z83-4"),
-		},
-		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
-			.no_edge_events_on_boot = true,
-		},
-	},
-	{
-		/*
-		 * The Terra Pad 1061 has a micro-USB-B id-pin handler, which
-		 * instead of controlling the actual micro-USB-B turns the 5V
-		 * boost for its USB-A connector off. The actual micro-USB-B
-		 * connector is wired for charging only.
-		 */
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Wortmann_AG"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "TERRA_PAD_1061"),
-		},
-		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
-			.no_edge_events_on_boot = true,
-		},
-	},
-	{
-		/*
-		 * The Dell Venue 10 Pro 5055, with Bay Trail SoC + TI PMIC uses an
-		 * external embedded-controller connected via I2C + an ACPI GPIO
-		 * event handler on INT33FFC:02 pin 12, causing spurious wakeups.
-		 */
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "Venue 10 Pro 5055"),
-		},
-		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
-			.ignore_wake = "INT33FC:02@12",
-		},
-	},
-	{
-		/*
-		 * HP X2 10 models with Cherry Trail SoC + TI PMIC use an
-		 * external embedded-controller connected via I2C + an ACPI GPIO
-		 * event handler on INT33FF:01 pin 0, causing spurious wakeups.
-		 * When suspending by closing the LID, the power to the USB
-		 * keyboard is turned off, causing INT0002 ACPI events to
-		 * trigger once the XHCI controller notices the keyboard is
-		 * gone. So INT0002 events cause spurious wakeups too. Ignoring
-		 * EC wakes breaks wakeup when opening the lid, the user needs
-		 * to press the power-button to wakeup the system. The
-		 * alternative is suspend simply not working, which is worse.
-		 */
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "HP x2 Detachable 10-p0XX"),
-		},
-		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
-			.ignore_wake = "INT33FF:01@0,INT0002:00@2",
-		},
-	},
-	{
-		/*
-		 * HP X2 10 models with Bay Trail SoC + AXP288 PMIC use an
-		 * external embedded-controller connected via I2C + an ACPI GPIO
-		 * event handler on INT33FC:02 pin 28, causing spurious wakeups.
-		 */
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion x2 Detachable"),
-			DMI_MATCH(DMI_BOARD_NAME, "815D"),
-		},
-		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
-			.ignore_wake = "INT33FC:02@28",
-		},
-	},
-	{
-		/*
-		 * HP X2 10 models with Cherry Trail SoC + AXP288 PMIC use an
-		 * external embedded-controller connected via I2C + an ACPI GPIO
-		 * event handler on INT33FF:01 pin 0, causing spurious wakeups.
-		 */
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion x2 Detachable"),
-			DMI_MATCH(DMI_BOARD_NAME, "813E"),
-		},
-		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
-			.ignore_wake = "INT33FF:01@0",
-		},
-	},
-	{
-		/*
-		 * Interrupt storm caused from edge triggered floating pin
-		 * Found in BIOS UX325UAZ.300
-		 * https://bugzilla.kernel.org/show_bug.cgi?id=216208
-		 */
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
-			DMI_MATCH(DMI_PRODUCT_NAME, "ZenBook UX325UAZ_UM325UAZ"),
-		},
-		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
-			.ignore_interrupt = "AMDI0030:00@18",
-		},
-	},
-	{
-		/*
-		 * Spurious wakeups from TP_ATTN# pin
-		 * Found in BIOS 1.7.8
-		 * https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627
-		 */
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "NL5xNU"),
-		},
-		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
-			.ignore_wake = "ELAN0415:00@9",
-		},
-	},
-	{
-		/*
-		 * Spurious wakeups from TP_ATTN# pin
-		 * Found in BIOS 1.7.8
-		 * https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627
-		 */
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "NL5xRU"),
-		},
-		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
-			.ignore_wake = "ELAN0415:00@9",
-		},
-	},
-	{
-		/*
-		 * Spurious wakeups from TP_ATTN# pin
-		 * Found in BIOS 1.7.7
-		 */
-		.matches = {
-			DMI_MATCH(DMI_BOARD_NAME, "NH5xAx"),
-		},
-		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
-			.ignore_wake = "SYNA1202:00@16",
-		},
-	},
-	{
-		/*
-		 * On the Peaq C1010 2-in-1 INT33FC:00 pin 3 is connected to
-		 * a "dolby" button. At the ACPI level an _AEI event-handler
-		 * is connected which sets an ACPI variable to 1 on both
-		 * edges. This variable can be polled + cleared to 0 using
-		 * WMI. But since the variable is set on both edges the WMI
-		 * interface is pretty useless even when polling.
-		 * So instead the x86-android-tablets code instantiates
-		 * a gpio-keys platform device for it.
-		 * Ignore the _AEI handler for the pin, so that it is not busy.
-		 */
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "PEAQ"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "PEAQ PMM C1010 MD99187"),
-		},
-		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
-			.ignore_interrupt = "INT33FC:00@3",
-		},
-	},
-	{
-		/*
-		 * Spurious wakeups from TP_ATTN# pin
-		 * Found in BIOS 0.35
-		 * https://gitlab.freedesktop.org/drm/amd/-/issues/3073
-		 */
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "GPD"),
-			DMI_MATCH(DMI_PRODUCT_NAME, "G1619-04"),
-		},
-		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
-			.ignore_wake = "PNP0C50:00@8",
-		},
-	},
-	{
-		/*
-		 * Spurious wakeups from GPIO 11
-		 * Found in BIOS 1.04
-		 * https://gitlab.freedesktop.org/drm/amd/-/issues/3954
-		 */
-		.matches = {
-			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
-			DMI_MATCH(DMI_PRODUCT_FAMILY, "Acer Nitro V 14"),
-		},
-		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
-			.ignore_interrupt = "AMDI0030:00@11",
-		},
-	},
-	{} /* Terminating entry */
-};
-
-static int __init acpi_gpio_setup_params(void)
-{
-	const struct acpi_gpiolib_dmi_quirk *quirk = NULL;
-	const struct dmi_system_id *id;
-
-	id = dmi_first_match(gpiolib_acpi_quirks);
-	if (id)
-		quirk = id->driver_data;
-
-	if (run_edge_events_on_boot < 0) {
-		if (quirk && quirk->no_edge_events_on_boot)
-			run_edge_events_on_boot = 0;
-		else
-			run_edge_events_on_boot = 1;
-	}
-
-	if (ignore_wake == NULL && quirk && quirk->ignore_wake)
-		ignore_wake = quirk->ignore_wake;
-
-	if (ignore_interrupt == NULL && quirk && quirk->ignore_interrupt)
-		ignore_interrupt = quirk->ignore_interrupt;
-
-	return 0;
-}
-
-/* Directly after dmi_setup() which runs as core_initcall() */
-postcore_initcall(acpi_gpio_setup_params);
diff --git a/drivers/gpio/gpiolib-acpi-quirks.c b/drivers/gpio/gpiolib-acpi-quirks.c
new file mode 100644
index 000000000000..219667315b2c
--- /dev/null
+++ b/drivers/gpio/gpiolib-acpi-quirks.c
@@ -0,0 +1,363 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ACPI quirks for GPIO ACPI helpers
+ *
+ * Author: Hans de Goede <hdegoede@redhat.com>
+ */
+
+#include <linux/dmi.h>
+#include <linux/kstrtox.h>
+#include <linux/list.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/printk.h>
+#include <linux/string.h>
+#include <linux/types.h>
+
+#include "gpiolib-acpi.h"
+
+static int run_edge_events_on_boot = -1;
+module_param(run_edge_events_on_boot, int, 0444);
+MODULE_PARM_DESC(run_edge_events_on_boot,
+		 "Run edge _AEI event-handlers at boot: 0=no, 1=yes, -1=auto");
+
+static char *ignore_wake;
+module_param(ignore_wake, charp, 0444);
+MODULE_PARM_DESC(ignore_wake,
+		 "controller@pin combos on which to ignore the ACPI wake flag "
+		 "ignore_wake=controller@pin[,controller@pin[,...]]");
+
+static char *ignore_interrupt;
+module_param(ignore_interrupt, charp, 0444);
+MODULE_PARM_DESC(ignore_interrupt,
+		 "controller@pin combos on which to ignore interrupt "
+		 "ignore_interrupt=controller@pin[,controller@pin[,...]]");
+
+/*
+ * For GPIO chips which call acpi_gpiochip_request_interrupts() before late_init
+ * (so builtin drivers) we register the ACPI GpioInt IRQ handlers from a
+ * late_initcall_sync() handler, so that other builtin drivers can register their
+ * OpRegions before the event handlers can run. This list contains GPIO chips
+ * for which the acpi_gpiochip_request_irqs() call has been deferred.
+ */
+static DEFINE_MUTEX(acpi_gpio_deferred_req_irqs_lock);
+static LIST_HEAD(acpi_gpio_deferred_req_irqs_list);
+static bool acpi_gpio_deferred_req_irqs_done;
+
+bool acpi_gpio_add_to_deferred_list(struct list_head *list)
+{
+	bool defer;
+
+	mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
+	defer = !acpi_gpio_deferred_req_irqs_done;
+	if (defer)
+		list_add(list, &acpi_gpio_deferred_req_irqs_list);
+	mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
+
+	return defer;
+}
+
+void acpi_gpio_remove_from_deferred_list(struct list_head *list)
+{
+	mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
+	if (!list_empty(list))
+		list_del_init(list);
+	mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
+}
+
+int acpi_gpio_need_run_edge_events_on_boot(void)
+{
+	return run_edge_events_on_boot;
+}
+
+bool acpi_gpio_in_ignore_list(enum acpi_gpio_ignore_list list,
+			      const char *controller_in, unsigned int pin_in)
+{
+	const char *ignore_list, *controller, *pin_str;
+	unsigned int pin;
+	char *endp;
+	int len;
+
+	switch (list) {
+	case ACPI_GPIO_IGNORE_WAKE:
+		ignore_list = ignore_wake;
+		break;
+	case ACPI_GPIO_IGNORE_INTERRUPT:
+		ignore_list = ignore_interrupt;
+		break;
+	default:
+		return false;
+	}
+
+	controller = ignore_list;
+	while (controller) {
+		pin_str = strchr(controller, '@');
+		if (!pin_str)
+			goto err;
+
+		len = pin_str - controller;
+		if (len == strlen(controller_in) &&
+		    strncmp(controller, controller_in, len) == 0) {
+			pin = simple_strtoul(pin_str + 1, &endp, 10);
+			if (*endp != 0 && *endp != ',')
+				goto err;
+
+			if (pin == pin_in)
+				return true;
+		}
+
+		controller = strchr(controller, ',');
+		if (controller)
+			controller++;
+	}
+
+	return false;
+err:
+	pr_err_once("Error: Invalid value for gpiolib_acpi.ignore_...: %s\n", ignore_list);
+	return false;
+}
+
+/* Run deferred acpi_gpiochip_request_irqs() */
+static int __init acpi_gpio_handle_deferred_request_irqs(void)
+{
+	mutex_lock(&acpi_gpio_deferred_req_irqs_lock);
+	acpi_gpio_process_deferred_list(&acpi_gpio_deferred_req_irqs_list);
+	acpi_gpio_deferred_req_irqs_done = true;
+	mutex_unlock(&acpi_gpio_deferred_req_irqs_lock);
+
+	return 0;
+}
+/* We must use _sync so that this runs after the first deferred_probe run */
+late_initcall_sync(acpi_gpio_handle_deferred_request_irqs);
+
+struct acpi_gpiolib_dmi_quirk {
+	bool no_edge_events_on_boot;
+	char *ignore_wake;
+	char *ignore_interrupt;
+};
+
+static const struct dmi_system_id gpiolib_acpi_quirks[] __initconst = {
+	{
+		/*
+		 * The Minix Neo Z83-4 has a micro-USB-B id-pin handler for
+		 * a non existing micro-USB-B connector which puts the HDMI
+		 * DDC pins in GPIO mode, breaking HDMI support.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "MINIX"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Z83-4"),
+		},
+		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+			.no_edge_events_on_boot = true,
+		},
+	},
+	{
+		/*
+		 * The Terra Pad 1061 has a micro-USB-B id-pin handler, which
+		 * instead of controlling the actual micro-USB-B turns the 5V
+		 * boost for its USB-A connector off. The actual micro-USB-B
+		 * connector is wired for charging only.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Wortmann_AG"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "TERRA_PAD_1061"),
+		},
+		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+			.no_edge_events_on_boot = true,
+		},
+	},
+	{
+		/*
+		 * The Dell Venue 10 Pro 5055, with Bay Trail SoC + TI PMIC uses an
+		 * external embedded-controller connected via I2C + an ACPI GPIO
+		 * event handler on INT33FFC:02 pin 12, causing spurious wakeups.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Dell Inc."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "Venue 10 Pro 5055"),
+		},
+		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+			.ignore_wake = "INT33FC:02@12",
+		},
+	},
+	{
+		/*
+		 * HP X2 10 models with Cherry Trail SoC + TI PMIC use an
+		 * external embedded-controller connected via I2C + an ACPI GPIO
+		 * event handler on INT33FF:01 pin 0, causing spurious wakeups.
+		 * When suspending by closing the LID, the power to the USB
+		 * keyboard is turned off, causing INT0002 ACPI events to
+		 * trigger once the XHCI controller notices the keyboard is
+		 * gone. So INT0002 events cause spurious wakeups too. Ignoring
+		 * EC wakes breaks wakeup when opening the lid, the user needs
+		 * to press the power-button to wakeup the system. The
+		 * alternative is suspend simply not working, which is worse.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "HP x2 Detachable 10-p0XX"),
+		},
+		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+			.ignore_wake = "INT33FF:01@0,INT0002:00@2",
+		},
+	},
+	{
+		/*
+		 * HP X2 10 models with Bay Trail SoC + AXP288 PMIC use an
+		 * external embedded-controller connected via I2C + an ACPI GPIO
+		 * event handler on INT33FC:02 pin 28, causing spurious wakeups.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion x2 Detachable"),
+			DMI_MATCH(DMI_BOARD_NAME, "815D"),
+		},
+		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+			.ignore_wake = "INT33FC:02@28",
+		},
+	},
+	{
+		/*
+		 * HP X2 10 models with Cherry Trail SoC + AXP288 PMIC use an
+		 * external embedded-controller connected via I2C + an ACPI GPIO
+		 * event handler on INT33FF:01 pin 0, causing spurious wakeups.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "HP"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "HP Pavilion x2 Detachable"),
+			DMI_MATCH(DMI_BOARD_NAME, "813E"),
+		},
+		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+			.ignore_wake = "INT33FF:01@0",
+		},
+	},
+	{
+		/*
+		 * Interrupt storm caused from edge triggered floating pin
+		 * Found in BIOS UX325UAZ.300
+		 * https://bugzilla.kernel.org/show_bug.cgi?id=216208
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
+			DMI_MATCH(DMI_PRODUCT_NAME, "ZenBook UX325UAZ_UM325UAZ"),
+		},
+		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+			.ignore_interrupt = "AMDI0030:00@18",
+		},
+	},
+	{
+		/*
+		 * Spurious wakeups from TP_ATTN# pin
+		 * Found in BIOS 1.7.8
+		 * https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627
+		 */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "NL5xNU"),
+		},
+		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+			.ignore_wake = "ELAN0415:00@9",
+		},
+	},
+	{
+		/*
+		 * Spurious wakeups from TP_ATTN# pin
+		 * Found in BIOS 1.7.8
+		 * https://gitlab.freedesktop.org/drm/amd/-/issues/1722#note_1720627
+		 */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "NL5xRU"),
+		},
+		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+			.ignore_wake = "ELAN0415:00@9",
+		},
+	},
+	{
+		/*
+		 * Spurious wakeups from TP_ATTN# pin
+		 * Found in BIOS 1.7.7
+		 */
+		.matches = {
+			DMI_MATCH(DMI_BOARD_NAME, "NH5xAx"),
+		},
+		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+			.ignore_wake = "SYNA1202:00@16",
+		},
+	},
+	{
+		/*
+		 * On the Peaq C1010 2-in-1 INT33FC:00 pin 3 is connected to
+		 * a "dolby" button. At the ACPI level an _AEI event-handler
+		 * is connected which sets an ACPI variable to 1 on both
+		 * edges. This variable can be polled + cleared to 0 using
+		 * WMI. But since the variable is set on both edges the WMI
+		 * interface is pretty useless even when polling.
+		 * So instead the x86-android-tablets code instantiates
+		 * a gpio-keys platform device for it.
+		 * Ignore the _AEI handler for the pin, so that it is not busy.
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "PEAQ"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "PEAQ PMM C1010 MD99187"),
+		},
+		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+			.ignore_interrupt = "INT33FC:00@3",
+		},
+	},
+	{
+		/*
+		 * Spurious wakeups from TP_ATTN# pin
+		 * Found in BIOS 0.35
+		 * https://gitlab.freedesktop.org/drm/amd/-/issues/3073
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "GPD"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "G1619-04"),
+		},
+		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+			.ignore_wake = "PNP0C50:00@8",
+		},
+	},
+	{
+		/*
+		 * Spurious wakeups from GPIO 11
+		 * Found in BIOS 1.04
+		 * https://gitlab.freedesktop.org/drm/amd/-/issues/3954
+		 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Acer"),
+			DMI_MATCH(DMI_PRODUCT_FAMILY, "Acer Nitro V 14"),
+		},
+		.driver_data = &(struct acpi_gpiolib_dmi_quirk) {
+			.ignore_interrupt = "AMDI0030:00@11",
+		},
+	},
+	{} /* Terminating entry */
+};
+
+static int __init acpi_gpio_setup_params(void)
+{
+	const struct acpi_gpiolib_dmi_quirk *quirk = NULL;
+	const struct dmi_system_id *id;
+
+	id = dmi_first_match(gpiolib_acpi_quirks);
+	if (id)
+		quirk = id->driver_data;
+
+	if (run_edge_events_on_boot < 0) {
+		if (quirk && quirk->no_edge_events_on_boot)
+			run_edge_events_on_boot = 0;
+		else
+			run_edge_events_on_boot = 1;
+	}
+
+	if (ignore_wake == NULL && quirk && quirk->ignore_wake)
+		ignore_wake = quirk->ignore_wake;
+
+	if (ignore_interrupt == NULL && quirk && quirk->ignore_interrupt)
+		ignore_interrupt = quirk->ignore_interrupt;
+
+	return 0;
+}
+
+/* Directly after dmi_setup() which runs as core_initcall() */
+postcore_initcall(acpi_gpio_setup_params);
-- 
2.47.2

Re: [PATCH v1 0/4] gpiolib: acpi: Split quirks to its own file

[Hans de Goede]

Hi,

On 13-May-25 12:00, Andy Shevchenko wrote:
> The GPIO ACPI helpers use a few quirks which consumes approximately 20%
> of the file. Besides that the necessary bits are sparse and being directly
> referred. Split them to a separate file. There is no functional change.
> 
> For the new file I used the Hans' authorship of Hans as he the author of
> all those bits (expect very tiny changes made by this series).
> 
> Hans, please check if it's okay and confirm, or suggest better alternative.

Thanks, the entire series looks good to me:

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

Regards,

Hans




> Andy Shevchenko (4):
>   gpiolib: acpi: Switch to use enum in acpi_gpio_in_ignore_list()
>   gpiolib: acpi: Handle deferred list via new API
>   gpiolib: acpi: Add acpi_gpio_need_run_edge_events_on_boot() getter
>   gpiolib: acpi: Move quirks to a separate file
> 
>  drivers/gpio/Makefile                         |   1 +
>  .../{gpiolib-acpi.c => gpiolib-acpi-core.c}   | 344 +----------------
>  drivers/gpio/gpiolib-acpi-quirks.c            | 363 ++++++++++++++++++
>  drivers/gpio/gpiolib-acpi.h                   |  15 +
>  4 files changed, 392 insertions(+), 331 deletions(-)
>  rename drivers/gpio/{gpiolib-acpi.c => gpiolib-acpi-core.c} (79%)
>  create mode 100644 drivers/gpio/gpiolib-acpi-quirks.c
> 

Re: [PATCH v1 0/4] gpiolib: acpi: Split quirks to its own file

[Mika Westerberg]

On Tue, May 13, 2025 at 01:00:30PM +0300, Andy Shevchenko wrote:
> The GPIO ACPI helpers use a few quirks which consumes approximately 20%
> of the file. Besides that the necessary bits are sparse and being directly
> referred. Split them to a separate file. There is no functional change.
> 
> For the new file I used the Hans' authorship of Hans as he the author of
> all those bits (expect very tiny changes made by this series).
> 
> Hans, please check if it's okay and confirm, or suggest better alternative.
> 
> Andy Shevchenko (4):
>   gpiolib: acpi: Switch to use enum in acpi_gpio_in_ignore_list()
>   gpiolib: acpi: Handle deferred list via new API
>   gpiolib: acpi: Add acpi_gpio_need_run_edge_events_on_boot() getter
>   gpiolib: acpi: Move quirks to a separate file
> 
>  drivers/gpio/Makefile                         |   1 +
>  .../{gpiolib-acpi.c => gpiolib-acpi-core.c}   | 344 +----------------
>  drivers/gpio/gpiolib-acpi-quirks.c            | 363 ++++++++++++++++++
>  drivers/gpio/gpiolib-acpi.h                   |  15 +

All this -foo-core things look redundant to me. Why not just split it out
and call it gpiolib-quirks.c and put there all the quirks not just ACPI? I
Don't think we want to have gpiolib-of-quirks.c and gpiolog-swnode-quirks.c
and so on.

>  4 files changed, 392 insertions(+), 331 deletions(-)
>  rename drivers/gpio/{gpiolib-acpi.c => gpiolib-acpi-core.c} (79%)
>  create mode 100644 drivers/gpio/gpiolib-acpi-quirks.c
> 
> -- 
> 2.47.2

Re: [PATCH v1 0/4] gpiolib: acpi: Split quirks to its own file

[Linus Walleij]

On Wed, May 14, 2025 at 6:00 PM Mika Westerberg
<mika.westerberg@linux.intel.com> wrote:

> >  drivers/gpio/Makefile                         |   1 +
> >  .../{gpiolib-acpi.c => gpiolib-acpi-core.c}   | 344 +----------------
> >  drivers/gpio/gpiolib-acpi-quirks.c            | 363 ++++++++++++++++++
> >  drivers/gpio/gpiolib-acpi.h                   |  15 +
>
> All this -foo-core things look redundant to me. Why not just split it out
> and call it gpiolib-quirks.c and put there all the quirks not just ACPI? I
> Don't think we want to have gpiolib-of-quirks.c and gpiolog-swnode-quirks.c
> and so on.

For OF/device tree the quirks are in gpiolib-of.c and we probably do
not want to put these into a shared file with ACPI (and swnode?)
quirks as systems with OF compile objects (Makefile entries)
and ACPI compile objects are not always included in the same build,
so having them per-hw-config-principle cuts down compiletime
overhead. Also it's pretty clear separation of concerns I think.

Yours,
Linus Walleij

Re: [PATCH v1 0/4] gpiolib: acpi: Split quirks to its own file

[Andy Shevchenko]

On Wed, May 14, 2025 at 06:59:55PM +0300, Mika Westerberg wrote:
> On Tue, May 13, 2025 at 01:00:30PM +0300, Andy Shevchenko wrote:
> > The GPIO ACPI helpers use a few quirks which consumes approximately 20%
> > of the file. Besides that the necessary bits are sparse and being directly
> > referred. Split them to a separate file. There is no functional change.
> > 
> > For the new file I used the Hans' authorship of Hans as he the author of
> > all those bits (expect very tiny changes made by this series).
> > 
> > Hans, please check if it's okay and confirm, or suggest better alternative.
> > 
> > Andy Shevchenko (4):
> >   gpiolib: acpi: Switch to use enum in acpi_gpio_in_ignore_list()
> >   gpiolib: acpi: Handle deferred list via new API
> >   gpiolib: acpi: Add acpi_gpio_need_run_edge_events_on_boot() getter
> >   gpiolib: acpi: Move quirks to a separate file
> > 
> >  drivers/gpio/Makefile                         |   1 +
> >  .../{gpiolib-acpi.c => gpiolib-acpi-core.c}   | 344 +----------------
> >  drivers/gpio/gpiolib-acpi-quirks.c            | 363 ++++++++++++++++++
> >  drivers/gpio/gpiolib-acpi.h                   |  15 +
> 
> All this -foo-core things look redundant to me. Why not just split it out
> and call it gpiolib-quirks.c and put there all the quirks not just ACPI? I
> Don't think we want to have gpiolib-of-quirks.c and gpiolog-swnode-quirks.c
> and so on.

That's might be the next step to have for all of them, but these are ACPI
specific. In any case they can't be put to gpiolib-quirks.c due to module
parameters. If we do that we will need a dirty hack to support old module
parameters (see 8250 how it's done there, and even author of that didn't like
the approach).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/4] gpiolib: acpi: Split quirks to its own file

[Andy Shevchenko]

On Thu, May 15, 2025 at 09:21:10AM +0200, Linus Walleij wrote:
> On Wed, May 14, 2025 at 6:00 PM Mika Westerberg
> <mika.westerberg@linux.intel.com> wrote:
> 
> > >  drivers/gpio/Makefile                         |   1 +
> > >  .../{gpiolib-acpi.c => gpiolib-acpi-core.c}   | 344 +----------------
> > >  drivers/gpio/gpiolib-acpi-quirks.c            | 363 ++++++++++++++++++
> > >  drivers/gpio/gpiolib-acpi.h                   |  15 +
> >
> > All this -foo-core things look redundant to me. Why not just split it out
> > and call it gpiolib-quirks.c and put there all the quirks not just ACPI? I
> > Don't think we want to have gpiolib-of-quirks.c and gpiolog-swnode-quirks.c
> > and so on.
> 
> For OF/device tree the quirks are in gpiolib-of.c and we probably do
> not want to put these into a shared file with ACPI (and swnode?)
> quirks as systems with OF compile objects (Makefile entries)
> and ACPI compile objects are not always included in the same build,
> so having them per-hw-config-principle cuts down compiletime
> overhead. Also it's pretty clear separation of concerns I think.

Yes, gpiolib-quirks.c would make sense for the shared code, but I don't
expect we will have any reasonable amount of those that are shared between
ACPI, DT, swnode cases.

But main problem here is the module parameters that are already exist,
there is no clean way in Linux kernel to provide an aliases table when
renaming files (AFAIK, otherwise tell me how to achieve that).

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/4] gpiolib: acpi: Split quirks to its own file

[Mika Westerberg]

On Thu, May 15, 2025 at 11:04:22AM +0300, Andy Shevchenko wrote:
> On Wed, May 14, 2025 at 06:59:55PM +0300, Mika Westerberg wrote:
> > On Tue, May 13, 2025 at 01:00:30PM +0300, Andy Shevchenko wrote:
> > > The GPIO ACPI helpers use a few quirks which consumes approximately 20%
> > > of the file. Besides that the necessary bits are sparse and being directly
> > > referred. Split them to a separate file. There is no functional change.
> > > 
> > > For the new file I used the Hans' authorship of Hans as he the author of
> > > all those bits (expect very tiny changes made by this series).
> > > 
> > > Hans, please check if it's okay and confirm, or suggest better alternative.
> > > 
> > > Andy Shevchenko (4):
> > >   gpiolib: acpi: Switch to use enum in acpi_gpio_in_ignore_list()
> > >   gpiolib: acpi: Handle deferred list via new API
> > >   gpiolib: acpi: Add acpi_gpio_need_run_edge_events_on_boot() getter
> > >   gpiolib: acpi: Move quirks to a separate file
> > > 
> > >  drivers/gpio/Makefile                         |   1 +
> > >  .../{gpiolib-acpi.c => gpiolib-acpi-core.c}   | 344 +----------------
> > >  drivers/gpio/gpiolib-acpi-quirks.c            | 363 ++++++++++++++++++
> > >  drivers/gpio/gpiolib-acpi.h                   |  15 +
> > 
> > All this -foo-core things look redundant to me. Why not just split it out
> > and call it gpiolib-quirks.c and put there all the quirks not just ACPI? I
> > Don't think we want to have gpiolib-of-quirks.c and gpiolog-swnode-quirks.c
> > and so on.
> 
> That's might be the next step to have for all of them, but these are ACPI
> specific. In any case they can't be put to gpiolib-quirks.c due to module
> parameters. If we do that we will need a dirty hack to support old module
> parameters (see 8250 how it's done there, and even author of that didn't like
> the approach).

Hmm, how does it affect module paremeters? I thought they are
gpiolib.something as all these object files are linked to it?

At least can we drop the gpiolib-acpi-core.c rename?

Re: [PATCH v1 0/4] gpiolib: acpi: Split quirks to its own file

[Andy Shevchenko]

On Thu, May 15, 2025 at 11:34:51AM +0300, Mika Westerberg wrote:
> On Thu, May 15, 2025 at 11:04:22AM +0300, Andy Shevchenko wrote:
> > On Wed, May 14, 2025 at 06:59:55PM +0300, Mika Westerberg wrote:
> > > On Tue, May 13, 2025 at 01:00:30PM +0300, Andy Shevchenko wrote:
> > > > The GPIO ACPI helpers use a few quirks which consumes approximately 20%
> > > > of the file. Besides that the necessary bits are sparse and being directly
> > > > referred. Split them to a separate file. There is no functional change.
> > > > 
> > > > For the new file I used the Hans' authorship of Hans as he the author of
> > > > all those bits (expect very tiny changes made by this series).
> > > > 
> > > > Hans, please check if it's okay and confirm, or suggest better alternative.
> > > > 
> > > > Andy Shevchenko (4):
> > > >   gpiolib: acpi: Switch to use enum in acpi_gpio_in_ignore_list()
> > > >   gpiolib: acpi: Handle deferred list via new API
> > > >   gpiolib: acpi: Add acpi_gpio_need_run_edge_events_on_boot() getter
> > > >   gpiolib: acpi: Move quirks to a separate file
> > > > 
> > > >  drivers/gpio/Makefile                         |   1 +
> > > >  .../{gpiolib-acpi.c => gpiolib-acpi-core.c}   | 344 +----------------
> > > >  drivers/gpio/gpiolib-acpi-quirks.c            | 363 ++++++++++++++++++
> > > >  drivers/gpio/gpiolib-acpi.h                   |  15 +
> > > 
> > > All this -foo-core things look redundant to me. Why not just split it out
> > > and call it gpiolib-quirks.c and put there all the quirks not just ACPI? I
> > > Don't think we want to have gpiolib-of-quirks.c and gpiolog-swnode-quirks.c
> > > and so on.
> > 
> > That's might be the next step to have for all of them, but these are ACPI
> > specific. In any case they can't be put to gpiolib-quirks.c due to module
> > parameters. If we do that we will need a dirty hack to support old module
> > parameters (see 8250 how it's done there, and even author of that didn't like
> > the approach).
> 
> Hmm, how does it affect module paremeters? I thought they are
> gpiolib.something as all these object files are linked to it?

gpiolib_acpi.FOO because the object file is gpiolib-acpi.o.

> At least can we drop the gpiolib-acpi-core.c rename?

Unfortunately no due to the above.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/4] gpiolib: acpi: Split quirks to its own file

[Mika Westerberg]

On Thu, May 15, 2025 at 11:41:59AM +0300, Andy Shevchenko wrote:
> On Thu, May 15, 2025 at 11:34:51AM +0300, Mika Westerberg wrote:
> > On Thu, May 15, 2025 at 11:04:22AM +0300, Andy Shevchenko wrote:
> > > On Wed, May 14, 2025 at 06:59:55PM +0300, Mika Westerberg wrote:
> > > > On Tue, May 13, 2025 at 01:00:30PM +0300, Andy Shevchenko wrote:
> > > > > The GPIO ACPI helpers use a few quirks which consumes approximately 20%
> > > > > of the file. Besides that the necessary bits are sparse and being directly
> > > > > referred. Split them to a separate file. There is no functional change.
> > > > > 
> > > > > For the new file I used the Hans' authorship of Hans as he the author of
> > > > > all those bits (expect very tiny changes made by this series).
> > > > > 
> > > > > Hans, please check if it's okay and confirm, or suggest better alternative.
> > > > > 
> > > > > Andy Shevchenko (4):
> > > > >   gpiolib: acpi: Switch to use enum in acpi_gpio_in_ignore_list()
> > > > >   gpiolib: acpi: Handle deferred list via new API
> > > > >   gpiolib: acpi: Add acpi_gpio_need_run_edge_events_on_boot() getter
> > > > >   gpiolib: acpi: Move quirks to a separate file
> > > > > 
> > > > >  drivers/gpio/Makefile                         |   1 +
> > > > >  .../{gpiolib-acpi.c => gpiolib-acpi-core.c}   | 344 +----------------
> > > > >  drivers/gpio/gpiolib-acpi-quirks.c            | 363 ++++++++++++++++++
> > > > >  drivers/gpio/gpiolib-acpi.h                   |  15 +
> > > > 
> > > > All this -foo-core things look redundant to me. Why not just split it out
> > > > and call it gpiolib-quirks.c and put there all the quirks not just ACPI? I
> > > > Don't think we want to have gpiolib-of-quirks.c and gpiolog-swnode-quirks.c
> > > > and so on.
> > > 
> > > That's might be the next step to have for all of them, but these are ACPI
> > > specific. In any case they can't be put to gpiolib-quirks.c due to module
> > > parameters. If we do that we will need a dirty hack to support old module
> > > parameters (see 8250 how it's done there, and even author of that didn't like
> > > the approach).
> > 
> > Hmm, how does it affect module paremeters? I thought they are
> > gpiolib.something as all these object files are linked to it?
> 
> gpiolib_acpi.FOO because the object file is gpiolib-acpi.o.

Ah okay.

> > At least can we drop the gpiolib-acpi-core.c rename?
> 
> Unfortunately no due to the above.

This does not work?

gpiolib-acpi-y                 := gpiolib-acpi.o gpiolib-acpi-quirks.o

Re: [PATCH v1 0/4] gpiolib: acpi: Split quirks to its own file

[Andy Shevchenko]

On Thu, May 15, 2025 at 11:47:27AM +0300, Mika Westerberg wrote:
> On Thu, May 15, 2025 at 11:41:59AM +0300, Andy Shevchenko wrote:
> > On Thu, May 15, 2025 at 11:34:51AM +0300, Mika Westerberg wrote:
> > > On Thu, May 15, 2025 at 11:04:22AM +0300, Andy Shevchenko wrote:
> > > > On Wed, May 14, 2025 at 06:59:55PM +0300, Mika Westerberg wrote:

...

> > > > That's might be the next step to have for all of them, but these are ACPI
> > > > specific. In any case they can't be put to gpiolib-quirks.c due to module
> > > > parameters. If we do that we will need a dirty hack to support old module
> > > > parameters (see 8250 how it's done there, and even author of that didn't like
> > > > the approach).
> > > 
> > > Hmm, how does it affect module paremeters? I thought they are
> > > gpiolib.something as all these object files are linked to it?
> > 
> > gpiolib_acpi.FOO because the object file is gpiolib-acpi.o.
> 
> Ah okay.
> 
> > > At least can we drop the gpiolib-acpi-core.c rename?
> > 
> > Unfortunately no due to the above.
> 
> This does not work?
> 
> gpiolib-acpi-y                 := gpiolib-acpi.o gpiolib-acpi-quirks.o

No. You can't use the same name on left and right parts.


-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 0/4] gpiolib: acpi: Split quirks to its own file

[Mika Westerberg]

On Thu, May 15, 2025 at 11:58:33AM +0300, Andy Shevchenko wrote:
> On Thu, May 15, 2025 at 11:47:27AM +0300, Mika Westerberg wrote:
> > On Thu, May 15, 2025 at 11:41:59AM +0300, Andy Shevchenko wrote:
> > > On Thu, May 15, 2025 at 11:34:51AM +0300, Mika Westerberg wrote:
> > > > On Thu, May 15, 2025 at 11:04:22AM +0300, Andy Shevchenko wrote:
> > > > > On Wed, May 14, 2025 at 06:59:55PM +0300, Mika Westerberg wrote:
> 
> ...
> 
> > > > > That's might be the next step to have for all of them, but these are ACPI
> > > > > specific. In any case they can't be put to gpiolib-quirks.c due to module
> > > > > parameters. If we do that we will need a dirty hack to support old module
> > > > > parameters (see 8250 how it's done there, and even author of that didn't like
> > > > > the approach).
> > > > 
> > > > Hmm, how does it affect module paremeters? I thought they are
> > > > gpiolib.something as all these object files are linked to it?
> > > 
> > > gpiolib_acpi.FOO because the object file is gpiolib-acpi.o.
> > 
> > Ah okay.
> > 
> > > > At least can we drop the gpiolib-acpi-core.c rename?
> > > 
> > > Unfortunately no due to the above.
> > 
> > This does not work?
> > 
> > gpiolib-acpi-y                 := gpiolib-acpi.o gpiolib-acpi-quirks.o
> 
> No. You can't use the same name on left and right parts.

I see :( Okay then I guess there are no other options than name it like
this.

[ Ideally we would drop the while gpiolib- prefix from all these so you have
acpi.c, sysfs.c and so on without the redundancy but that's outside of
scope of this work anyways ;-) ]

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH v1 0/4] gpiolib: acpi: Split quirks to its own file

[Andy Shevchenko]

On Thu, May 15, 2025 at 12:31:24PM +0300, Mika Westerberg wrote:
> On Thu, May 15, 2025 at 11:58:33AM +0300, Andy Shevchenko wrote:
> > On Thu, May 15, 2025 at 11:47:27AM +0300, Mika Westerberg wrote:
> > > On Thu, May 15, 2025 at 11:41:59AM +0300, Andy Shevchenko wrote:
> > > > On Thu, May 15, 2025 at 11:34:51AM +0300, Mika Westerberg wrote:
> > > > > On Thu, May 15, 2025 at 11:04:22AM +0300, Andy Shevchenko wrote:
> > > > > > On Wed, May 14, 2025 at 06:59:55PM +0300, Mika Westerberg wrote:
> > 
> > ...
> > 
> > > > > > That's might be the next step to have for all of them, but these are ACPI
> > > > > > specific. In any case they can't be put to gpiolib-quirks.c due to module
> > > > > > parameters. If we do that we will need a dirty hack to support old module
> > > > > > parameters (see 8250 how it's done there, and even author of that didn't like
> > > > > > the approach).
> > > > > 
> > > > > Hmm, how does it affect module paremeters? I thought they are
> > > > > gpiolib.something as all these object files are linked to it?
> > > > 
> > > > gpiolib_acpi.FOO because the object file is gpiolib-acpi.o.
> > > 
> > > Ah okay.
> > > 
> > > > > At least can we drop the gpiolib-acpi-core.c rename?
> > > > 
> > > > Unfortunately no due to the above.
> > > 
> > > This does not work?
> > > 
> > > gpiolib-acpi-y                 := gpiolib-acpi.o gpiolib-acpi-quirks.o
> > 
> > No. You can't use the same name on left and right parts.
> 
> I see :( Okay then I guess there are no other options than name it like
> this.
> 
> [ Ideally we would drop the while gpiolib- prefix from all these so you have
> acpi.c, sysfs.c and so on without the redundancy but that's outside of
> scope of this work anyways ;-) ]

I also would like to have prefix to be dropped for the files, but the modules
(and it's can be achieved, but it's indeed out of scope here).

> Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Thank you!
I am going to push to my review and testing queue soon.

-- 
With Best Regards,
Andy Shevchenko