Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.36 as permitted sender) client-ip=23.128.96.36;
Date:   Wed, 4 Oct 2023 21:14:50 +0200 (CEST)
From:   Jiri Kosina <jikos@kernel.org>
To:     Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
cc:     benjamin.tissoires@redhat.com, linux-input@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Kai-Heng Feng <kai.heng.feng@canonical.com>
Subject: Re: [PATCH] HID: intel-ish-hid: ipc: Disable and reenable ACPI GPE
 bit
In-Reply-To: <20231003155332.1855569-1-srinivas.pandruvada@linux.intel.com>
Message-ID: <nycvar.YFH.7.76.2310042114420.3534@cbobk.fhfr.pm>
References: <20231003155332.1855569-1-srinivas.pandruvada@linux.intel.com>
User-Agent: Alpine 2.21 (LSU 202 2017-01-01)
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Precedence: bulk

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