[PATCH] HID: intel-ish-hid: ipc: Disable and reenable ACPI GPE bit

[PATCH] HID: intel-ish-hid: ipc: Disable and reenable ACPI GPE bit

[Srinivas Pandruvada]

The EHL (Elkhart Lake) based platforms provide a OOB (Out of band)
service, which allows to wakup device when the system is in S5 (Soft-Off
state). This OOB service can be enabled/disabled from BIOS settings. When
enabled, the ISH device gets PME wake capability. To enable PME wakeup,
driver also needs to enable ACPI GPE bit.

On resume, BIOS will clear the wakeup bit. So driver need to re-enable it
in resume function to keep the next wakeup capability. But this BIOS
clearing of wakeup bit doesn't decrement internal OS GPE reference count,
so this reenabling on every resume will cause reference count to overflow.

So first disable and reenable ACPI GPE bit using acpi_disable_gpe().

Fixes: 2e23a70edabe ("HID: intel-ish-hid: ipc: finish power flow for EHL OOB")
Reported-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Closes: https://lore.kernel.org/lkml/CAAd53p4=oLYiH2YbVSmrPNj1zpMcfp=Wxbasb5vhMXOWCArLCg@mail.gmail.com/T/
Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
---
 drivers/hid/intel-ish-hid/ipc/pci-ish.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
index 55cb25038e63..710fda5f19e1 100644
--- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
+++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
@@ -133,6 +133,14 @@ static int enable_gpe(struct device *dev)
 	}
 	wakeup = &adev->wakeup;
 
+	/*
+	 * Call acpi_disable_gpe(), so that reference count
+	 * gpe_event_info->runtime_count doesn't overflow.
+	 * When gpe_event_info->runtime_count = 0, the call
+	 * to acpi_disable_gpe() simply return.
+	 */
+	acpi_disable_gpe(wakeup->gpe_device, wakeup->gpe_number);
+
 	acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup->gpe_number);
 	if (ACPI_FAILURE(acpi_sts)) {
 		dev_err(dev, "enable ose_gpe failed\n");
-- 
2.41.0

Re: [PATCH] HID: intel-ish-hid: ipc: Disable and reenable ACPI GPE bit

[Jiri Kosina]

On Tue, 3 Oct 2023, Srinivas Pandruvada wrote:

> The EHL (Elkhart Lake) based platforms provide a OOB (Out of band)
> service, which allows to wakup device when the system is in S5 (Soft-Off
> state). This OOB service can be enabled/disabled from BIOS settings. When
> enabled, the ISH device gets PME wake capability. To enable PME wakeup,
> driver also needs to enable ACPI GPE bit.
> 
> On resume, BIOS will clear the wakeup bit. So driver need to re-enable it
> in resume function to keep the next wakeup capability. But this BIOS
> clearing of wakeup bit doesn't decrement internal OS GPE reference count,
> so this reenabling on every resume will cause reference count to overflow.
> 
> So first disable and reenable ACPI GPE bit using acpi_disable_gpe().
> 
> Fixes: 2e23a70edabe ("HID: intel-ish-hid: ipc: finish power flow for EHL OOB")
> Reported-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Closes: https://lore.kernel.org/lkml/CAAd53p4=oLYiH2YbVSmrPNj1zpMcfp=Wxbasb5vhMXOWCArLCg@mail.gmail.com/T/
> Tested-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
> ---
>  drivers/hid/intel-ish-hid/ipc/pci-ish.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> index 55cb25038e63..710fda5f19e1 100644
> --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c
> @@ -133,6 +133,14 @@ static int enable_gpe(struct device *dev)
>  	}
>  	wakeup = &adev->wakeup;
>  
> +	/*
> +	 * Call acpi_disable_gpe(), so that reference count
> +	 * gpe_event_info->runtime_count doesn't overflow.
> +	 * When gpe_event_info->runtime_count = 0, the call
> +	 * to acpi_disable_gpe() simply return.
> +	 */
> +	acpi_disable_gpe(wakeup->gpe_device, wakeup->gpe_number);
> +
>  	acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup->gpe_number);
>  	if (ACPI_FAILURE(acpi_sts)) {
>  		dev_err(dev, "enable ose_gpe failed\n");

Applied, thanks Srinivas.

-- 
Jiri Kosina
SUSE Labs