mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ACPI: GED: Support GpioInt event resources
@ 2026-09-30 11:15 Rahul Pon
  2026-09-30 20:22 ` Linus Walleij
  0 siblings, 1 reply; 6+ messages in thread
From: Rahul Pon @ 2026-09-30 11:15 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Len Brown, Linus Walleij, Bartosz Golaszewski, linux-acpi,
	linux-gpio, linux-kernel

The Generic Event Device accepts only Interrupt and ExtendedInterrupt
resources in _CRS. Some ACPI firmware signals GED events through GPIO
controllers instead: the HP OmniBook 5 16-bf1xxx (Snapdragon X2) describes
its lid (LIGE) and embedded-controller event device (ECGE) as ACPI0013 with
GpioInt resources, and both currently fail to probe with "unable to parse
IRQ resource".

Handle GpioInt resources: get the Linux IRQ with acpi_dev_gpio_irq_get(),
which applies the trigger and polarity and returns -EPROBE_DEFER until the
GPIO controller is registered, and evaluate _EVT with the GPIO pin number,
as done for GSIs above 255. GpioIo resources are skipped. On a failed walk,
free the events already requested and propagate the GPIO error (including
-EPROBE_DEFER) instead of -EINVAL.

Tested on the HP OmniBook 5 16-bf1xxx: lid close/open run _EVT and reach
the ACPI button driver and logind.

Assisted-by: LLM
Signed-off-by: Rahul Pon <theflyingrahul@gmail.com>
---
 drivers/acpi/evged.c | 46 +++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/evged.c b/drivers/acpi/evged.c
index 5c35cbc7f6..478172ede7 100644
--- a/drivers/acpi/evged.c
+++ b/drivers/acpi/evged.c
@@ -43,6 +43,8 @@
 struct acpi_ged_device {
 	struct device *dev;
 	struct list_head event_list;
+	unsigned int gpio_index;
+	int error;
 };
 
 struct acpi_ged_event {
@@ -76,7 +78,7 @@ static acpi_status acpi_ged_request_interrupt(struct acpi_resource *ares,
 	struct device *dev = geddev->dev;
 	acpi_handle handle = ACPI_HANDLE(dev);
 	acpi_handle evt_handle;
-	struct resource r;
+	struct resource r = {};
 	struct acpi_resource_irq *p = &ares->data.irq;
 	struct acpi_resource_extended_irq *pext = &ares->data.extended_irq;
 	char ev_name[5];
@@ -85,6 +87,33 @@ static acpi_status acpi_ged_request_interrupt(struct acpi_resource *ares,
 	if (ares->type == ACPI_RESOURCE_TYPE_END_TAG)
 		return AE_OK;
 
+	if (ares->type == ACPI_RESOURCE_TYPE_GPIO) {
+		struct acpi_resource_gpio *agpio;
+		int ret;
+
+		/* GpioIo resources are not event sources. */
+		if (!acpi_gpio_get_irq_resource(ares, &agpio))
+			return AE_OK;
+
+		/* The GPIO core applies the GpioInt trigger and polarity. */
+		ret = acpi_dev_gpio_irq_get(ACPI_COMPANION(dev), geddev->gpio_index++);
+		if (ret < 0) {
+			geddev->error = ret;
+			return AE_ERROR;
+		}
+		irq = ret;
+		gsi = agpio->pin_table[0];
+		if (agpio->shareable == ACPI_SHARED)
+			r.flags |= IORESOURCE_IRQ_SHAREABLE;
+
+		/* As for GSIs above 255, GPIO-signaled events use _EVT. */
+		if (ACPI_FAILURE(acpi_get_handle(handle, "_EVT", &evt_handle))) {
+			dev_err(dev, "cannot locate _EVT method\n");
+			return AE_ERROR;
+		}
+		goto request;
+	}
+
 	if (!acpi_dev_resource_interrupt(ares, 0, &r)) {
 		dev_err(dev, "unable to parse IRQ resource\n");
 		return AE_ERROR;
@@ -115,6 +144,7 @@ static acpi_status acpi_ged_request_interrupt(struct acpi_resource *ares,
 		return AE_ERROR;
 	}
 
+request:
 	event = devm_kzalloc(dev, sizeof(*event), GFP_KERNEL);
 	if (!event)
 		return AE_ERROR;
@@ -138,6 +168,8 @@ static acpi_status acpi_ged_request_interrupt(struct acpi_resource *ares,
 	return AE_OK;
 }
 
+static void acpi_ged_free_events(struct acpi_ged_device *geddev);
+
 static int ged_probe(struct platform_device *pdev)
 {
 	struct acpi_ged_device *geddev;
@@ -152,6 +184,10 @@ static int ged_probe(struct platform_device *pdev)
 	acpi_ret = acpi_walk_resources(ACPI_HANDLE(&pdev->dev), "_CRS",
 				       acpi_ged_request_interrupt, geddev);
 	if (ACPI_FAILURE(acpi_ret)) {
+		acpi_ged_free_events(geddev);
+		if (geddev->error)
+			return dev_err_probe(&pdev->dev, geddev->error,
+					     "unable to get GPIO event IRQ\n");
 		dev_err(&pdev->dev, "unable to parse the _CRS record\n");
 		return -EINVAL;
 	}
@@ -160,9 +196,8 @@ static int ged_probe(struct platform_device *pdev)
 	return 0;
 }
 
-static void ged_shutdown(struct platform_device *pdev)
+static void acpi_ged_free_events(struct acpi_ged_device *geddev)
 {
-	struct acpi_ged_device *geddev = platform_get_drvdata(pdev);
 	struct acpi_ged_event *event, *next;
 
 	list_for_each_entry_safe(event, next, &geddev->event_list, node) {
@@ -173,6 +208,11 @@ static void ged_shutdown(struct platform_device *pdev)
 	}
 }
 
+static void ged_shutdown(struct platform_device *pdev)
+{
+	acpi_ged_free_events(platform_get_drvdata(pdev));
+}
+
 static void ged_remove(struct platform_device *pdev)
 {
 	ged_shutdown(pdev);

base-commit: 551c722f40809618230001baccf219193e22fc5a
-- 
2.53.0


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

* Re: [PATCH] ACPI: GED: Support GpioInt event resources
  2026-09-30 11:15 [PATCH] ACPI: GED: Support GpioInt event resources Rahul Pon
@ 2026-09-30 20:22 ` Linus Walleij
  2026-10-01  4:04   ` Mika Westerberg
  2026-10-01  4:18   ` Rahul Pon
  0 siblings, 2 replies; 6+ messages in thread
From: Linus Walleij @ 2026-09-30 20:22 UTC (permalink / raw)
  To: Rahul Pon, Andy Shevchenko, Mika Westerberg
  Cc: Rafael J. Wysocki, Len Brown, Bartosz Golaszewski, linux-acpi,
	linux-gpio, linux-kernel

On Wed, Sep 30, 2026 at 1:16 PM Rahul Pon <theflyingrahul@gmail.com> wrote:

> The Generic Event Device accepts only Interrupt and ExtendedInterrupt
> resources in _CRS. Some ACPI firmware signals GED events through GPIO
> controllers instead: the HP OmniBook 5 16-bf1xxx (Snapdragon X2) describes
> its lid (LIGE) and embedded-controller event device (ECGE) as ACPI0013 with
> GpioInt resources, and both currently fail to probe with "unable to parse
> IRQ resource".
>
> Handle GpioInt resources: get the Linux IRQ with acpi_dev_gpio_irq_get(),
> which applies the trigger and polarity and returns -EPROBE_DEFER until the
> GPIO controller is registered, and evaluate _EVT with the GPIO pin number,
> as done for GSIs above 255. GpioIo resources are skipped. On a failed walk,
> free the events already requested and propagate the GPIO error (including
> -EPROBE_DEFER) instead of -EINVAL.
>
> Tested on the HP OmniBook 5 16-bf1xxx: lid close/open run _EVT and reach
> the ACPI button driver and logind.
>
> Assisted-by: LLM
> Signed-off-by: Rahul Pon <theflyingrahul@gmail.com>

I don't understand the patch at all, but I understand that Andy or Mika must
review it otherwise it's not going anywhere, so looping them in.

Yours,
Linus Walleij

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

* Re: [PATCH] ACPI: GED: Support GpioInt event resources
  2026-09-30 20:22 ` Linus Walleij
@ 2026-10-01  4:04   ` Mika Westerberg
  2026-10-01  4:27     ` Rahul Pon
  2026-10-01  4:18   ` Rahul Pon
  1 sibling, 1 reply; 6+ messages in thread
From: Mika Westerberg @ 2026-10-01  4:04 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Rahul Pon, Andy Shevchenko, Rafael J. Wysocki, Len Brown,
	Bartosz Golaszewski, linux-acpi, linux-gpio, linux-kernel

Hi,

On Wed, Sep 30, 2026 at 10:22:22PM +0200, Linus Walleij wrote:
> On Wed, Sep 30, 2026 at 1:16 PM Rahul Pon <theflyingrahul@gmail.com> wrote:
> 
> > The Generic Event Device accepts only Interrupt and ExtendedInterrupt
> > resources in _CRS. Some ACPI firmware signals GED events through GPIO
> > controllers instead: the HP OmniBook 5 16-bf1xxx (Snapdragon X2) describes
> > its lid (LIGE) and embedded-controller event device (ECGE) as ACPI0013 with
> > GpioInt resources, and both currently fail to probe with "unable to parse
> > IRQ resource".
> >
> > Handle GpioInt resources: get the Linux IRQ with acpi_dev_gpio_irq_get(),
> > which applies the trigger and polarity and returns -EPROBE_DEFER until the
> > GPIO controller is registered, and evaluate _EVT with the GPIO pin number,
> > as done for GSIs above 255. GpioIo resources are skipped. On a failed walk,
> > free the events already requested and propagate the GPIO error (including
> > -EPROBE_DEFER) instead of -EINVAL.
> >
> > Tested on the HP OmniBook 5 16-bf1xxx: lid close/open run _EVT and reach
> > the ACPI button driver and logind.
> >
> > Assisted-by: LLM
> > Signed-off-by: Rahul Pon <theflyingrahul@gmail.com>
> 
> I don't understand the patch at all, but I understand that Andy or Mika must
> review it otherwise it's not going anywhere, so looping them in.

ACPI GED specifically is supposed to use interrupts only (hence the name,
Interrupt -signaled ACPI events). I think this one should really use the
GPIO signaled ACPI events instead and probably just has the ACPI0013 there
by accident. Can you share the ASL around this device?

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

* Re: [PATCH] ACPI: GED: Support GpioInt event resources
  2026-09-30 20:22 ` Linus Walleij
  2026-10-01  4:04   ` Mika Westerberg
@ 2026-10-01  4:18   ` Rahul Pon
  2026-10-01 18:15     ` Andy Shevchenko
  1 sibling, 1 reply; 6+ messages in thread
From: Rahul Pon @ 2026-10-01  4:18 UTC (permalink / raw)
  To: Linus Walleij
  Cc: Andy Shevchenko, Mika Westerberg, Mika Westerberg,
	Rafael J. Wysocki, Len Brown, Bartosz Golaszewski, linux-acpi,
	linux-gpio, linux-kernel

On Thu, Oct 1, 2026 at 1:52 AM Linus Walleij <linusw@kernel.org> wrote:
> I don't understand the patch at all, but I understand that Andy or Mika must
> review it otherwise it's not going anywhere, so looping them in.

Thanks, Linus.

Andy, Mika: some context that should have been in the commit message.

ACPI 6.6 section 5.6.9.2 says a GED _CRS contains only Interrupt
resource descriptors, so this firmware is outside the spec. It is still
the only source of lid events here: LIGE's _CRS is a single GpioInt on
the GPIO controller (pin 960), and the controller's _AEI lists one
other, unrelated pin. ECGE is the same, with pin 768. On Windows, the
in-box ACPI device driver (acpidev.inf) binds both GED devices, and the
lid works there.

For _EVT, section 5.6.9.3 passes the GSI number, which a GpioInt does
not have. The patch passes the pin number from the GpioInt descriptor,
as section 5.6.5.3 does for _EVT under _AEI. Both _EVT methods on this
laptop ignore Arg0.

If the approach is acceptable, v2 will state the spec deviation in the
commit message.

Thanks,
Rahul

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

* Re: [PATCH] ACPI: GED: Support GpioInt event resources
  2026-10-01  4:04   ` Mika Westerberg
@ 2026-10-01  4:27     ` Rahul Pon
  0 siblings, 0 replies; 6+ messages in thread
From: Rahul Pon @ 2026-10-01  4:27 UTC (permalink / raw)
  To: Mika Westerberg
  Cc: Linus Walleij, Mika Westerberg, Andy Shevchenko,
	Rafael J. Wysocki, Len Brown, Bartosz Golaszewski, linux-acpi,
	linux-gpio, linux-kernel

Hi Mika,

On Thu, Oct 01, 2026 at 06:04:20AM +0200, Mika Westerberg wrote:
> ACPI GED specifically is supposed to use interrupts only (hence the name,
> Interrupt -signaled ACPI events). I think this one should really use the
> GPIO signaled ACPI events instead and probably just has the ACPI0013 there
> by accident. Can you share the ASL around this device?

Sure, below. iasl shows the _CRS buffers as raw bytes; the GpioInt
lines are decoded from them and recompile to the same bytes.

The lid device:

    Device (LIGE)
    {
        Name (_HID, "ACPI0013")
        Name (_UID, One)
        Name (_DEP, Package (0x03)
        {
            \_SB.GIO0, ,
            \_SB.LID0, ,
            \_SB.IC10,
        })
        Name (EEDP, Buffer (0x03)
        {
             0xFF
        })
        CreateByteField (EEDP, Zero, STAT)
        CreateField (EEDP, 0x10, 0x08, DBUF)
        Name (LIDG, Zero)
        Method (_CRS, 0, NotSerialized)
        {
            Name (RBUF, Buffer (0x25)
            {
                /* 0000 */  0x8C, 0x20, 0x00, 0x01, 0x00, 0x01, 0x00, 0x1D,
                /* 0008 */  0x00, 0x03, 0x00, 0x00, 0x00, 0x00, 0x17, 0x00,
                /* 0010 */  0x00, 0x19, 0x00, 0x23, 0x00, 0x00, 0x00, 0xC0,
                /* 0018 */  0x03, 0x5C, 0x5F, 0x53, 0x42, 0x2E, 0x47, 0x49,
                /* 0020 */  0x4F, 0x30, 0x00, 0x79, 0x00
            })
            /* RBUF decodes to:
             * GpioInt (Edge, ActiveBoth, SharedAndWake, PullNone, 0x0000,
             *          "\\_SB.GIO0", 0x00, ResourceConsumer, ,) { 960 }
             */
            Return (RBUF)
        }

        Method (_EVT, 1, NotSerialized)
        {
            If ((\_SB.IC10.AVBL == One))
            {
                Sleep (0x05)
                Acquire (\_SB.ECMX, 0xFFFF)
                EEDP = \_SB.IC10.CMB2
                Release (\_SB.ECMX)
                LIDG = DBUF
                If ((LIDG & 0x04))
                {
                    \_SB.LID0.LIDB = One
                    \_SB.GPU0.LIDB = One
                    Notify (\_SB.LID0, 0x80)
                }
                Else
                {
                    \_SB.LID0.LIDB = \_SB.GIO0.LIDR
                    \_SB.GPU0.LIDB = \_SB.GIO0.LIDR
                    Notify (\_SB.LID0, 0x80)
                    If ((\_SB.LID0.LIDB == Zero))
                    {
                        \_SB.WMID.GWMT (0x08, One)
                    }

                    If ((\_SB.LID0.LIDB == One))
                    {
                        \_SB.WMID.GWMT (0x08, 0x02)
                    }
                }
            }
            Else
            {
                \_SB.LID0.LIDB = \_SB.GIO0.LIDR
                \_SB.GPU0.LIDB = \_SB.GIO0.LIDR
                Notify (\_SB.LID0, 0x80)
            }
        }
    }

The EC event device, ECGE, is another ACPI0013 with one GpioInt:

    GpioInt (Edge, ActiveLow, Exclusive, PullUp, 0x0000,
             "\\_SB.GIO0", 0x00, ResourceConsumer, ,) { 768 }

The GPIO controller (GIO0, QCOM0F0C) does have GPIO-signaled events.
Its _AEI lists one pin:

    GpioInt (Edge, ActiveHigh, Exclusive, PullNone, 0x01F4,
             "\\_SB.GIO0", 0x00, ResourceConsumer, ,) { 126 }

Its _EVT handles that pin (0x7E, a Notify to the GPU) and also pin 0x5C
with the same EC read and lid Notify as LIGE's _EVT, but _AEI does not
list 0x5C. So on this firmware the lid is signaled only through LIGE.

Either way, please drop this patch, and disregard the v2 plan in my
other reply. Looking at it again, it has no upstream user: GIO0 has no
ACPI driver in mainline, and my lid test used an out-of-tree bring-up
driver for it. Mainline supports these laptops through the device tree,
where the lid is a gpio-keys switch and no GED is involved.

Sorry for the noise.

Thanks,
Rahul

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

* Re: [PATCH] ACPI: GED: Support GpioInt event resources
  2026-10-01  4:18   ` Rahul Pon
@ 2026-10-01 18:15     ` Andy Shevchenko
  0 siblings, 0 replies; 6+ messages in thread
From: Andy Shevchenko @ 2026-10-01 18:15 UTC (permalink / raw)
  To: Rahul Pon
  Cc: Linus Walleij, Andy Shevchenko, Mika Westerberg, Mika Westerberg,
	Rafael J. Wysocki, Len Brown, Bartosz Golaszewski, linux-acpi,
	linux-gpio, linux-kernel

On Thu, Oct 01, 2026 at 09:48:56AM +0530, Rahul Pon wrote:
> On Thu, Oct 1, 2026 at 1:52 AM Linus Walleij <linusw@kernel.org> wrote:
> > I don't understand the patch at all, but I understand that Andy or Mika must
> > review it otherwise it's not going anywhere, so looping them in.
> 
> Thanks, Linus.
> 
> Andy, Mika: some context that should have been in the commit message.
> 
> ACPI 6.6 section 5.6.9.2 says a GED _CRS contains only Interrupt
> resource descriptors, so this firmware is outside the spec. It is still
> the only source of lid events here: LIGE's _CRS is a single GpioInt on
> the GPIO controller (pin 960), and the controller's _AEI lists one
> other, unrelated pin. ECGE is the same, with pin 768. On Windows, the
> in-box ACPI device driver (acpidev.inf) binds both GED devices, and the
> lid works there.
> 
> For _EVT, section 5.6.9.3 passes the GSI number, which a GpioInt does
> not have. The patch passes the pin number from the GpioInt descriptor,
> as section 5.6.5.3 does for _EVT under _AEI. Both _EVT methods on this
> laptop ignore Arg0.
> 
> If the approach is acceptable, v2 will state the spec deviation in the
> commit message.

Is vendor of the firmware / platform being informed? Do they accept their
mistake? Are they going to fix their crap?

-- 
With Best Regards,
Andy Shevchenko



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

end of thread, other threads:[~2026-10-01 18:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-30 11:15 [PATCH] ACPI: GED: Support GpioInt event resources Rahul Pon
2026-09-30 20:22 ` Linus Walleij
2026-10-01  4:04   ` Mika Westerberg
2026-10-01  4:27     ` Rahul Pon
2026-10-01  4:18   ` Rahul Pon
2026-10-01 18:15     ` Andy Shevchenko

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®