linux-gpio.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] ACPI: x86: Introduce an acpi_quirk_skip_gpio_event_handlers() helper
@ 2023-02-18 10:32 Hans de Goede
  2023-02-18 10:32 ` [PATCH 1/3] " Hans de Goede
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Hans de Goede @ 2023-02-18 10:32 UTC (permalink / raw)
  To: Rafael J . Wysocki, Sebastian Reichel, Mika Westerberg,
	Andy Shevchenko, Linus Walleij
  Cc: Hans de Goede, Len Brown, linux-acpi, linux-pm, linux-gpio

<Resend with the gpiolib-acpi.c folks added to the To: list>

Hi All,

x86 ACPI boards which ship with only Android as their factory image usually
have pretty broken ACPI tables, relying on everything being hardcoded in
the factory kernel image and often disabling parts of the ACPI enumeration
kernel code to avoid the broken tables causing issues.

Part of this broken ACPI code is that sometimes these boards have _AEI
ACPI GPIO event handlers which are broken.

So far this has been dealt with in the platform/x86/x86-android-tablets.c
module by it calling acpi_gpiochip_free_interrupts() on gpiochip-s with
troublesome handlers to disable the handlers.

But this is racy and sometimes too late. This series adds a new
acpi_quirk_skip_gpio_event_handlers() function to drivers/acpi/x86/utils.c
using the existing DMI table for this to avoid duplication of DMI matches.

Patch 2 adds a new x86 Android tablet model which needs the new
SKIP_GPIO_EVENT_HANDLERS quirk.

Patch 3 uses acpi_quirk_skip_gpio_event_handlers() in axp288_charger.c
to deal with there not being a ACPI event handler to disable charging
from Vbus before enable 5V boost output on the tablets micro-USB conn.

Sebastian (sre), since patch 3 depends on patch 1, it is probably
easiest to just merge the entire series through the linux-pm tree.
May we have your ack for this ?

Regards,

Hans


Hans de Goede (3):
  ACPI: x86: Introduce an acpi_quirk_skip_gpio_event_handlers() helper
  ACPI: x86: Add skip i2c clients quirk for Acer Iconia One 7 B1-750
  power: supply: axp288_charger: Use alt usb-id extcon on some x86
    android tablets

 drivers/acpi/x86/utils.c              | 34 ++++++++++++++++++++++++---
 drivers/gpio/gpiolib-acpi.c           |  3 +++
 drivers/power/supply/axp288_charger.c | 15 ++++++++++--
 include/acpi/acpi_bus.h               |  5 ++++
 4 files changed, 52 insertions(+), 5 deletions(-)

-- 
2.39.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] ACPI: x86: Introduce an acpi_quirk_skip_gpio_event_handlers() helper
  2023-02-18 10:32 [PATCH 0/3] ACPI: x86: Introduce an acpi_quirk_skip_gpio_event_handlers() helper Hans de Goede
@ 2023-02-18 10:32 ` Hans de Goede
  2023-02-20 13:34   ` Andy Shevchenko
  2023-02-18 10:32 ` [PATCH 2/3] ACPI: x86: Add skip i2c clients quirk for Acer Iconia One 7 B1-750 Hans de Goede
  2023-02-18 10:32 ` [PATCH 3/3] power: supply: axp288_charger: Use alt usb-id extcon on some x86 android tablets Hans de Goede
  2 siblings, 1 reply; 8+ messages in thread
From: Hans de Goede @ 2023-02-18 10:32 UTC (permalink / raw)
  To: Rafael J . Wysocki, Sebastian Reichel, Mika Westerberg,
	Andy Shevchenko, Linus Walleij
  Cc: Hans de Goede, Len Brown, linux-acpi, linux-pm, linux-gpio

x86 ACPI boards which ship with only Android as their factory image usually
have pretty broken ACPI tables, relying on everything being hardcoded in
the factory kernel image and often disabling parts of the ACPI enumeration
kernel code to avoid the broken tables causing issues.

Part of this broken ACPI code is that sometimes these boards have _AEI
ACPI GPIO event handlers which are broken.

So far this has been dealt with in the platform/x86/x86-android-tablets.c
module, which contains various workarounds for these devices, by it calling
acpi_gpiochip_free_interrupts() on gpiochip-s with troublesome handlers to
disable the handlers.

But in some cases this is too late, if the handlers are of the edge type
then gpiolib-acpi.c's code will already have run them at boot.
This can cause issues such as GPIOs ending up as owned by "ACPI:OpRegion",
making them unavailable for drivers which actually need them.

Boards with these broken ACPI tables are already listed in
drivers/acpi/x86/utils.c for e.g. acpi_quirk_skip_i2c_client_enumeration().
Extend the quirks mechanism for a new acpi_quirk_skip_gpio_event_handlers()
helper, this re-uses the DMI-ids rather then having to duplicate the same
DMI table in gpiolib-acpi.c .

Also add the new ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS quirk to existing
boards with troublesome ACPI gpio event handlers, so that the current
acpi_gpiochip_free_interrupts() hack can be removed from
x86-android-tablets.c .

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/x86/utils.c    | 24 +++++++++++++++++++++---
 drivers/gpio/gpiolib-acpi.c |  3 +++
 include/acpi/acpi_bus.h     |  5 +++++
 3 files changed, 29 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/x86/utils.c b/drivers/acpi/x86/utils.c
index 4e816bb402f6..4a6f3a6726d0 100644
--- a/drivers/acpi/x86/utils.c
+++ b/drivers/acpi/x86/utils.c
@@ -262,6 +262,7 @@ bool force_storage_d3(void)
 #define ACPI_QUIRK_UART1_TTY_UART2_SKIP				BIT(1)
 #define ACPI_QUIRK_SKIP_ACPI_AC_AND_BATTERY			BIT(2)
 #define ACPI_QUIRK_USE_ACPI_AC_AND_BATTERY			BIT(3)
+#define ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS			BIT(4)
 
 static const struct dmi_system_id acpi_quirk_skip_dmi_ids[] = {
 	/*
@@ -297,7 +298,8 @@ static const struct dmi_system_id acpi_quirk_skip_dmi_ids[] = {
 		},
 		.driver_data = (void *)(ACPI_QUIRK_SKIP_I2C_CLIENTS |
 					ACPI_QUIRK_UART1_TTY_UART2_SKIP |
-					ACPI_QUIRK_SKIP_ACPI_AC_AND_BATTERY),
+					ACPI_QUIRK_SKIP_ACPI_AC_AND_BATTERY |
+					ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS),
 	},
 	{
 		.matches = {
@@ -305,7 +307,8 @@ static const struct dmi_system_id acpi_quirk_skip_dmi_ids[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "TF103C"),
 		},
 		.driver_data = (void *)(ACPI_QUIRK_SKIP_I2C_CLIENTS |
-					ACPI_QUIRK_SKIP_ACPI_AC_AND_BATTERY),
+					ACPI_QUIRK_SKIP_ACPI_AC_AND_BATTERY |
+					ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS),
 	},
 	{
 		/* Lenovo Yoga Tablet 2 1050F/L */
@@ -347,7 +350,8 @@ static const struct dmi_system_id acpi_quirk_skip_dmi_ids[] = {
 			DMI_MATCH(DMI_PRODUCT_NAME, "M890BAP"),
 		},
 		.driver_data = (void *)(ACPI_QUIRK_SKIP_I2C_CLIENTS |
-					ACPI_QUIRK_SKIP_ACPI_AC_AND_BATTERY),
+					ACPI_QUIRK_SKIP_ACPI_AC_AND_BATTERY |
+					ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS),
 	},
 	{
 		/* Whitelabel (sold as various brands) TM800A550L */
@@ -424,6 +428,20 @@ int acpi_quirk_skip_serdev_enumeration(struct device *controller_parent, bool *s
 	return 0;
 }
 EXPORT_SYMBOL_GPL(acpi_quirk_skip_serdev_enumeration);
+
+bool acpi_quirk_skip_gpio_event_handlers(void)
+{
+	const struct dmi_system_id *dmi_id;
+	long quirks;
+
+	dmi_id = dmi_first_match(acpi_quirk_skip_dmi_ids);
+	if (!dmi_id)
+		return false;
+
+	quirks = (unsigned long)dmi_id->driver_data;
+	return (quirks & ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS);
+}
+EXPORT_SYMBOL_GPL(acpi_quirk_skip_gpio_event_handlers);
 #endif
 
 /* Lists of PMIC ACPI HIDs with an (often better) native charger driver */
diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index 17c53f484280..6041768bb72b 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -536,6 +536,9 @@ void acpi_gpiochip_request_interrupts(struct gpio_chip *chip)
 	if (ACPI_FAILURE(status))
 		return;
 
+	if (acpi_quirk_skip_gpio_event_handlers())
+		return;
+
 	acpi_walk_resources(handle, METHOD_NAME__AEI,
 			    acpi_gpiochip_alloc_event, acpi_gpio);
 
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index e44be31115a6..d69545cd6a48 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -656,6 +656,7 @@ static inline bool acpi_quirk_skip_acpi_ac_and_battery(void)
 #if IS_ENABLED(CONFIG_X86_ANDROID_TABLETS)
 bool acpi_quirk_skip_i2c_client_enumeration(struct acpi_device *adev);
 int acpi_quirk_skip_serdev_enumeration(struct device *controller_parent, bool *skip);
+bool acpi_quirk_skip_gpio_event_handlers(void);
 #else
 static inline bool acpi_quirk_skip_i2c_client_enumeration(struct acpi_device *adev)
 {
@@ -667,6 +668,10 @@ acpi_quirk_skip_serdev_enumeration(struct device *controller_parent, bool *skip)
 	*skip = false;
 	return 0;
 }
+static inline bool acpi_quirk_skip_gpio_event_handlers(void)
+{
+	return false;
+}
 #endif
 
 #ifdef CONFIG_PM
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] ACPI: x86: Add skip i2c clients quirk for Acer Iconia One 7 B1-750
  2023-02-18 10:32 [PATCH 0/3] ACPI: x86: Introduce an acpi_quirk_skip_gpio_event_handlers() helper Hans de Goede
  2023-02-18 10:32 ` [PATCH 1/3] " Hans de Goede
@ 2023-02-18 10:32 ` Hans de Goede
  2023-02-18 10:32 ` [PATCH 3/3] power: supply: axp288_charger: Use alt usb-id extcon on some x86 android tablets Hans de Goede
  2 siblings, 0 replies; 8+ messages in thread
From: Hans de Goede @ 2023-02-18 10:32 UTC (permalink / raw)
  To: Rafael J . Wysocki, Sebastian Reichel, Mika Westerberg,
	Andy Shevchenko, Linus Walleij
  Cc: Hans de Goede, Len Brown, linux-acpi, linux-pm, linux-gpio

The Acer Iconia One 7 B1-750 is a x86 tablet which ships with Android x86
as factory OS. The Android x86 kernel fork ignores I2C devices described
in the DSDT, except for the PMIC and Audio codecs.

As usual the Acer Iconia One 7 B1-750's DSDT contains a bunch of extra I2C
devices which are not actually there, causing various resource conflicts.
Add an ACPI_QUIRK_SKIP_I2C_CLIENTS quirk for the Acer Iconia One 7 B1-750
to the acpi_quirk_skip_dmi_ids table to woraround this.

The DSDT also contains broken ACPI GPIO event handlers, disable those too.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/acpi/x86/utils.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/acpi/x86/utils.c b/drivers/acpi/x86/utils.c
index 4a6f3a6726d0..644e2a7f4213 100644
--- a/drivers/acpi/x86/utils.c
+++ b/drivers/acpi/x86/utils.c
@@ -291,6 +291,16 @@ static const struct dmi_system_id acpi_quirk_skip_dmi_ids[] = {
 	 *    need the x86-android-tablets module to properly work.
 	 */
 #if IS_ENABLED(CONFIG_X86_ANDROID_TABLETS)
+	{
+		/* Acer Iconia One 7 B1-750 */
+		.matches = {
+			DMI_MATCH(DMI_SYS_VENDOR, "Insyde"),
+			DMI_MATCH(DMI_PRODUCT_NAME, "VESPA2"),
+		},
+		.driver_data = (void *)(ACPI_QUIRK_SKIP_I2C_CLIENTS |
+					ACPI_QUIRK_SKIP_ACPI_AC_AND_BATTERY |
+					ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS),
+	},
 	{
 		.matches = {
 			DMI_EXACT_MATCH(DMI_SYS_VENDOR, "ASUSTeK COMPUTER INC."),
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] power: supply: axp288_charger: Use alt usb-id extcon on some x86 android tablets
  2023-02-18 10:32 [PATCH 0/3] ACPI: x86: Introduce an acpi_quirk_skip_gpio_event_handlers() helper Hans de Goede
  2023-02-18 10:32 ` [PATCH 1/3] " Hans de Goede
  2023-02-18 10:32 ` [PATCH 2/3] ACPI: x86: Add skip i2c clients quirk for Acer Iconia One 7 B1-750 Hans de Goede
@ 2023-02-18 10:32 ` Hans de Goede
  2 siblings, 0 replies; 8+ messages in thread
From: Hans de Goede @ 2023-02-18 10:32 UTC (permalink / raw)
  To: Rafael J . Wysocki, Sebastian Reichel, Mika Westerberg,
	Andy Shevchenko, Linus Walleij
  Cc: Hans de Goede, Len Brown, linux-acpi, linux-pm, linux-gpio

x86 ACPI boards which ship with only Android as their factory image may
have pretty broken ACPI tables. This includes broken _AEI ACPI GPIO event
handlers, which are normally used to listen to the micro-USB ID pin and:

1. Switch the USB-mux to the host / device USB controllers
2. Disable Vbus path before enabling the 5V boost (AXP reg 0x30 bit 7)
3. Turn 5V Vboost on / off

On non broken systems where this is not done through an ACPI GPIO event
handler, there is an ACPI INT3496 device describing the involved GPIOs
which are handled by the extcon-intel-int3496 driver; and axp288-charger.ko
listens to this extcon-device and disables the Vbus path when necessary.

On x86 Android boards, with broken ACPI GPIO event handlers, these are
disabled by acpi_quirk_skip_gpio_event_handlers() and an intel-int3496
extcon device is manually instantiated by x86-android-tablets.ko .

Add support to the axp288-charger code for this setup, so that it
properly disables the Vbus path when necessary. Note this uses
acpi_quirk_skip_gpio_event_handlers() to identify these systems,
to avoid the need to add a separate DMI match table for this.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 drivers/power/supply/axp288_charger.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

diff --git a/drivers/power/supply/axp288_charger.c b/drivers/power/supply/axp288_charger.c
index 15219ed43ce9..b5903193e2f9 100644
--- a/drivers/power/supply/axp288_charger.c
+++ b/drivers/power/supply/axp288_charger.c
@@ -836,6 +836,7 @@ static int axp288_charger_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct axp20x_dev *axp20x = dev_get_drvdata(pdev->dev.parent);
 	struct power_supply_config charger_cfg = {};
+	const char *extcon_name = NULL;
 	unsigned int val;
 
 	/*
@@ -872,8 +873,18 @@ static int axp288_charger_probe(struct platform_device *pdev)
 		return PTR_ERR(info->cable.edev);
 	}
 
-	if (acpi_dev_present(USB_HOST_EXTCON_HID, NULL, -1)) {
-		info->otg.cable = extcon_get_extcon_dev(USB_HOST_EXTCON_NAME);
+	/*
+	 * On devices with broken ACPI GPIO event handlers there also is no ACPI
+	 * "INT3496" (USB_HOST_EXTCON_HID) device. x86-android-tablets.ko
+	 * instantiates an "intel-int3496" extcon on these devs as a workaround.
+	 */
+	if (acpi_quirk_skip_gpio_event_handlers())
+		extcon_name = "intel-int3496";
+	else if (acpi_dev_present(USB_HOST_EXTCON_HID, NULL, -1))
+		extcon_name = USB_HOST_EXTCON_NAME;
+
+	if (extcon_name) {
+		info->otg.cable = extcon_get_extcon_dev(extcon_name);
 		if (IS_ERR(info->otg.cable)) {
 			dev_err_probe(dev, PTR_ERR(info->otg.cable),
 				      "extcon_get_extcon_dev(%s) failed\n",
-- 
2.39.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] ACPI: x86: Introduce an acpi_quirk_skip_gpio_event_handlers() helper
  2023-02-18 10:32 ` [PATCH 1/3] " Hans de Goede
@ 2023-02-20 13:34   ` Andy Shevchenko
  2023-02-20 15:23     ` Hans de Goede
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2023-02-20 13:34 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Rafael J . Wysocki, Sebastian Reichel, Mika Westerberg,
	Linus Walleij, Len Brown, linux-acpi, linux-pm, linux-gpio

On Sat, Feb 18, 2023 at 11:32:33AM +0100, Hans de Goede wrote:
> x86 ACPI boards which ship with only Android as their factory image usually
> have pretty broken ACPI tables, relying on everything being hardcoded in
> the factory kernel image and often disabling parts of the ACPI enumeration
> kernel code to avoid the broken tables causing issues.
> 
> Part of this broken ACPI code is that sometimes these boards have _AEI
> ACPI GPIO event handlers which are broken.
> 
> So far this has been dealt with in the platform/x86/x86-android-tablets.c
> module, which contains various workarounds for these devices, by it calling
> acpi_gpiochip_free_interrupts() on gpiochip-s with troublesome handlers to
> disable the handlers.
> 
> But in some cases this is too late, if the handlers are of the edge type
> then gpiolib-acpi.c's code will already have run them at boot.
> This can cause issues such as GPIOs ending up as owned by "ACPI:OpRegion",
> making them unavailable for drivers which actually need them.
> 
> Boards with these broken ACPI tables are already listed in
> drivers/acpi/x86/utils.c for e.g. acpi_quirk_skip_i2c_client_enumeration().
> Extend the quirks mechanism for a new acpi_quirk_skip_gpio_event_handlers()
> helper, this re-uses the DMI-ids rather then having to duplicate the same
> DMI table in gpiolib-acpi.c .
> 
> Also add the new ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS quirk to existing
> boards with troublesome ACPI gpio event handlers, so that the current
> acpi_gpiochip_free_interrupts() hack can be removed from
> x86-android-tablets.c .

I'm wondering if we can teach acpi_gpio_in_ignore_list() to handle this.

P.S. Why do we lock an IRQ before checking acpi_gpio_in_ignore_list() and
     why do we not free that if the IRQ is in ignore list?

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] ACPI: x86: Introduce an acpi_quirk_skip_gpio_event_handlers() helper
  2023-02-20 13:34   ` Andy Shevchenko
@ 2023-02-20 15:23     ` Hans de Goede
  2023-02-20 15:35       ` Andy Shevchenko
  0 siblings, 1 reply; 8+ messages in thread
From: Hans de Goede @ 2023-02-20 15:23 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J . Wysocki, Sebastian Reichel, Mika Westerberg,
	Linus Walleij, Len Brown, linux-acpi, linux-pm, linux-gpio

Hi,

On 2/20/23 14:34, Andy Shevchenko wrote:
> On Sat, Feb 18, 2023 at 11:32:33AM +0100, Hans de Goede wrote:
>> x86 ACPI boards which ship with only Android as their factory image usually
>> have pretty broken ACPI tables, relying on everything being hardcoded in
>> the factory kernel image and often disabling parts of the ACPI enumeration
>> kernel code to avoid the broken tables causing issues.
>>
>> Part of this broken ACPI code is that sometimes these boards have _AEI
>> ACPI GPIO event handlers which are broken.
>>
>> So far this has been dealt with in the platform/x86/x86-android-tablets.c
>> module, which contains various workarounds for these devices, by it calling
>> acpi_gpiochip_free_interrupts() on gpiochip-s with troublesome handlers to
>> disable the handlers.
>>
>> But in some cases this is too late, if the handlers are of the edge type
>> then gpiolib-acpi.c's code will already have run them at boot.
>> This can cause issues such as GPIOs ending up as owned by "ACPI:OpRegion",
>> making them unavailable for drivers which actually need them.
>>
>> Boards with these broken ACPI tables are already listed in
>> drivers/acpi/x86/utils.c for e.g. acpi_quirk_skip_i2c_client_enumeration().
>> Extend the quirks mechanism for a new acpi_quirk_skip_gpio_event_handlers()
>> helper, this re-uses the DMI-ids rather then having to duplicate the same
>> DMI table in gpiolib-acpi.c .
>>
>> Also add the new ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS quirk to existing
>> boards with troublesome ACPI gpio event handlers, so that the current
>> acpi_gpiochip_free_interrupts() hack can be removed from
>> x86-android-tablets.c .
> 
> I'm wondering if we can teach acpi_gpio_in_ignore_list() to handle this.

You mean have it call acpi_quirk_skip_gpio_event_handlers(), or you mean
extend the DMI matchs inside drivers/gpio/gpiolib-acpi.c to cover these
cases ?

These devices with severely broken DSDTs already need a bunch of
other acpi handling quirks. So the idea is to re-use the existing
quirk mechanism for these to avoid having to have DMI match table
entries for a single model in various different places.

> P.S. Why do we lock an IRQ before checking acpi_gpio_in_ignore_list() and
>      why do we not free that if the IRQ is in ignore list?

The idea was to do the test after other things which can fail, so that
if there are other reasons to skip the GPIO we don't do the test +
dev_xxx msg.  But you are right, we should either unlock it when ignoring
it, or move the acpi_gpio_in_ignore_list() list check up.

I guess just moving the check up is better, shall I prepare a patch for this?

Regards,

Hans


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] ACPI: x86: Introduce an acpi_quirk_skip_gpio_event_handlers() helper
  2023-02-20 15:23     ` Hans de Goede
@ 2023-02-20 15:35       ` Andy Shevchenko
  2023-02-20 15:51         ` Hans de Goede
  0 siblings, 1 reply; 8+ messages in thread
From: Andy Shevchenko @ 2023-02-20 15:35 UTC (permalink / raw)
  To: Hans de Goede
  Cc: Rafael J . Wysocki, Sebastian Reichel, Mika Westerberg,
	Linus Walleij, Len Brown, linux-acpi, linux-pm, linux-gpio

On Mon, Feb 20, 2023 at 04:23:33PM +0100, Hans de Goede wrote:
> On 2/20/23 14:34, Andy Shevchenko wrote:
> > On Sat, Feb 18, 2023 at 11:32:33AM +0100, Hans de Goede wrote:
> >> x86 ACPI boards which ship with only Android as their factory image usually
> >> have pretty broken ACPI tables, relying on everything being hardcoded in
> >> the factory kernel image and often disabling parts of the ACPI enumeration
> >> kernel code to avoid the broken tables causing issues.
> >>
> >> Part of this broken ACPI code is that sometimes these boards have _AEI
> >> ACPI GPIO event handlers which are broken.
> >>
> >> So far this has been dealt with in the platform/x86/x86-android-tablets.c
> >> module, which contains various workarounds for these devices, by it calling
> >> acpi_gpiochip_free_interrupts() on gpiochip-s with troublesome handlers to
> >> disable the handlers.
> >>
> >> But in some cases this is too late, if the handlers are of the edge type
> >> then gpiolib-acpi.c's code will already have run them at boot.
> >> This can cause issues such as GPIOs ending up as owned by "ACPI:OpRegion",
> >> making them unavailable for drivers which actually need them.
> >>
> >> Boards with these broken ACPI tables are already listed in
> >> drivers/acpi/x86/utils.c for e.g. acpi_quirk_skip_i2c_client_enumeration().
> >> Extend the quirks mechanism for a new acpi_quirk_skip_gpio_event_handlers()
> >> helper, this re-uses the DMI-ids rather then having to duplicate the same
> >> DMI table in gpiolib-acpi.c .
> >>
> >> Also add the new ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS quirk to existing
> >> boards with troublesome ACPI gpio event handlers, so that the current
> >> acpi_gpiochip_free_interrupts() hack can be removed from
> >> x86-android-tablets.c .
> > 
> > I'm wondering if we can teach acpi_gpio_in_ignore_list() to handle this.
> 
> You mean have it call acpi_quirk_skip_gpio_event_handlers(), or you mean
> extend the DMI matchs inside drivers/gpio/gpiolib-acpi.c to cover these
> cases ?
> 
> These devices with severely broken DSDTs already need a bunch of
> other acpi handling quirks. So the idea is to re-use the existing
> quirk mechanism for these to avoid having to have DMI match table
> entries for a single model in various different places.

I don't like growing amount of compile dependencies between these modules.
(Yes, I'm aware about stubs.)

Can we maybe move other quirks out from gpiolib-acpi.c to something like
PDx86 or another existing board files (with quirks)?

> > P.S. Why do we lock an IRQ before checking acpi_gpio_in_ignore_list() and
> >      why do we not free that if the IRQ is in ignore list?
> 
> The idea was to do the test after other things which can fail, so that
> if there are other reasons to skip the GPIO we don't do the test +
> dev_xxx msg.  But you are right, we should either unlock it when ignoring
> it, or move the acpi_gpio_in_ignore_list() list check up.
> 
> I guess just moving the check up is better, shall I prepare a patch for this?

Yes, please.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] ACPI: x86: Introduce an acpi_quirk_skip_gpio_event_handlers() helper
  2023-02-20 15:35       ` Andy Shevchenko
@ 2023-02-20 15:51         ` Hans de Goede
  0 siblings, 0 replies; 8+ messages in thread
From: Hans de Goede @ 2023-02-20 15:51 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Rafael J . Wysocki, Sebastian Reichel, Mika Westerberg,
	Linus Walleij, Len Brown, linux-acpi, linux-pm, linux-gpio

Hi,

On 2/20/23 16:35, Andy Shevchenko wrote:
> On Mon, Feb 20, 2023 at 04:23:33PM +0100, Hans de Goede wrote:
>> On 2/20/23 14:34, Andy Shevchenko wrote:
>>> On Sat, Feb 18, 2023 at 11:32:33AM +0100, Hans de Goede wrote:
>>>> x86 ACPI boards which ship with only Android as their factory image usually
>>>> have pretty broken ACPI tables, relying on everything being hardcoded in
>>>> the factory kernel image and often disabling parts of the ACPI enumeration
>>>> kernel code to avoid the broken tables causing issues.
>>>>
>>>> Part of this broken ACPI code is that sometimes these boards have _AEI
>>>> ACPI GPIO event handlers which are broken.
>>>>
>>>> So far this has been dealt with in the platform/x86/x86-android-tablets.c
>>>> module, which contains various workarounds for these devices, by it calling
>>>> acpi_gpiochip_free_interrupts() on gpiochip-s with troublesome handlers to
>>>> disable the handlers.
>>>>
>>>> But in some cases this is too late, if the handlers are of the edge type
>>>> then gpiolib-acpi.c's code will already have run them at boot.
>>>> This can cause issues such as GPIOs ending up as owned by "ACPI:OpRegion",
>>>> making them unavailable for drivers which actually need them.
>>>>
>>>> Boards with these broken ACPI tables are already listed in
>>>> drivers/acpi/x86/utils.c for e.g. acpi_quirk_skip_i2c_client_enumeration().
>>>> Extend the quirks mechanism for a new acpi_quirk_skip_gpio_event_handlers()
>>>> helper, this re-uses the DMI-ids rather then having to duplicate the same
>>>> DMI table in gpiolib-acpi.c .
>>>>
>>>> Also add the new ACPI_QUIRK_SKIP_GPIO_EVENT_HANDLERS quirk to existing
>>>> boards with troublesome ACPI gpio event handlers, so that the current
>>>> acpi_gpiochip_free_interrupts() hack can be removed from
>>>> x86-android-tablets.c .
>>>
>>> I'm wondering if we can teach acpi_gpio_in_ignore_list() to handle this.
>>
>> You mean have it call acpi_quirk_skip_gpio_event_handlers(), or you mean
>> extend the DMI matchs inside drivers/gpio/gpiolib-acpi.c to cover these
>> cases ?
>>
>> These devices with severely broken DSDTs already need a bunch of
>> other acpi handling quirks. So the idea is to re-use the existing
>> quirk mechanism for these to avoid having to have DMI match table
>> entries for a single model in various different places.
> 
> I don't like growing amount of compile dependencies between these modules.
> (Yes, I'm aware about stubs.)

gpiolib-acpi.c already depends on CONFIG_ACPI and is not build when this
is not set. So this does not add any new dependencies. IOW I don't see
the problem here ?

(also for this reason there is no stub for the new
acpi_quirk_skip_gpio_event_handlers() helper)

> Can we maybe move other quirks out from gpiolib-acpi.c to something like
> PDx86 or another existing board files (with quirks)?

I don't really see a clean way to move these.
 
>>> P.S. Why do we lock an IRQ before checking acpi_gpio_in_ignore_list() and
>>>      why do we not free that if the IRQ is in ignore list?
>>
>> The idea was to do the test after other things which can fail, so that
>> if there are other reasons to skip the GPIO we don't do the test +
>> dev_xxx msg.  But you are right, we should either unlock it when ignoring
>> it, or move the acpi_gpio_in_ignore_list() list check up.
>>
>> I guess just moving the check up is better, shall I prepare a patch for this?
> 
> Yes, please.

Ok will do.

Regards,

hans


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-02-20 15:53 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-18 10:32 [PATCH 0/3] ACPI: x86: Introduce an acpi_quirk_skip_gpio_event_handlers() helper Hans de Goede
2023-02-18 10:32 ` [PATCH 1/3] " Hans de Goede
2023-02-20 13:34   ` Andy Shevchenko
2023-02-20 15:23     ` Hans de Goede
2023-02-20 15:35       ` Andy Shevchenko
2023-02-20 15:51         ` Hans de Goede
2023-02-18 10:32 ` [PATCH 2/3] ACPI: x86: Add skip i2c clients quirk for Acer Iconia One 7 B1-750 Hans de Goede
2023-02-18 10:32 ` [PATCH 3/3] power: supply: axp288_charger: Use alt usb-id extcon on some x86 android tablets Hans de Goede

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).