Hi, Kai-Heng, I just got feedback, for testing EHL S5 wakeup feature, you need several steps to setup and access "https://portal.devicewise.com/things/browse"; to trigger wake. But currently, our test account of this website are all out of data. So maybe you need double check with the team who required you preparing the patch for the verification. Thanks! Best Regards, Even Xu -----Original Message----- From: Xu, Even Sent: Friday, September 15, 2023 3:27 PM To: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> Cc: srinivas pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>; jikos@xxxxxxxxxx; benjamin.tissoires@xxxxxxxxxx; linux-pm@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; Lee, Jian Hui <jianhui.lee@xxxxxxxxxxxxx>; Zhang, Lixu <Lixu.Zhang@xxxxxxxxx>; Ba, Najumon <najumon.ba@xxxxxxxxx>; linux-input@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx Subject: RE: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup Hi, Kai-Heng, I am also not familiar with this S5 wakeup test case. I already sent out mails to ask for help on it. Will come back to you once I get feedback. Thanks! Best Regards, Even Xu -----Original Message----- From: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> Sent: Friday, September 15, 2023 2:01 PM To: Xu, Even <even.xu@xxxxxxxxx> Cc: srinivas pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx>; jikos@xxxxxxxxxx; benjamin.tissoires@xxxxxxxxxx; linux-pm@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; Lee, Jian Hui <jianhui.lee@xxxxxxxxxxxxx>; Zhang, Lixu <lixu.zhang@xxxxxxxxx>; Ba, Najumon <najumon.ba@xxxxxxxxx>; linux-input@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx Subject: Re: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup Hi Even, On Fri, Sep 15, 2023 at 1:31 PM Xu, Even <even.xu@xxxxxxxxx> wrote: > > Hi, Srinivas, > > Sure, I will test it. > As long term not working on EHL, I doesn't have EHL board on hand right now, I can test this patch on other ISH related platforms. > From the patch, it's focus on EHL platform, I assume Kai-Heng already verified the function on EHL board. I only made sure the GPE overflow issue is fixed by the patch, but I didn't test the S5 wakeup. That's because I don't know how to test it on the EHL system I have. I'll test it if you can let me know how to test the S5 wakeup. Kai-Heng > I don't think it will take effect on other platforms, anyway, I will test it on the platforms I have to provide cross platform verification. > > Thanks! > > Best Regards, > Even Xu > > -----Original Message----- > From: srinivas pandruvada <srinivas.pandruvada@xxxxxxxxxxxxxxx> > Sent: Friday, September 15, 2023 12:11 AM > To: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx>; jikos@xxxxxxxxxx; > benjamin.tissoires@xxxxxxxxxx > Cc: linux-pm@xxxxxxxxxxxxxxx; linux-pci@xxxxxxxxxxxxxxx; Lee, Jian Hui > <jianhui.lee@xxxxxxxxxxxxx>; Xu, Even <even.xu@xxxxxxxxx>; Zhang, Lixu > <lixu.zhang@xxxxxxxxx>; Ba, Najumon <najumon.ba@xxxxxxxxx>; > linux-input@xxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup > > Hi Even, > > On Thu, 2023-09-14 at 12:18 +0800, Kai-Heng Feng wrote: > > System cannot suspend more than 255 times because the driver doesn't > > have corresponding acpi_disable_gpe() for acpi_enable_gpe(), so the > > GPE refcount overflows. > > > > Since PCI core and ACPI core already handles PCI PME wake and GPE > > wake when the device has wakeup capability, use device_init_wakeup() > > to let them do the wakeup setting work. > > > > Also add a shutdown callback which uses pci_prepare_to_sleep() to > > let PCI and ACPI set OOB wakeup for S5. > > > Please test this change. > > Thanks, > Srinivas > > > Fixes: 2e23a70edabe ("HID: intel-ish-hid: ipc: finish power flow for > > EHL OOB") > > Cc: Jian Hui Lee <jianhui.lee@xxxxxxxxxxxxx> > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@xxxxxxxxxxxxx> > > --- > > drivers/hid/intel-ish-hid/ipc/pci-ish.c | 59 > > +++++++---------------- > > -- > > 1 file changed, 15 insertions(+), 44 deletions(-) > > > > diff --git a/drivers/hid/intel-ish-hid/ipc/pci-ish.c > > b/drivers/hid/intel-ish-hid/ipc/pci-ish.c > > index 55cb25038e63..65e7eeb2fa64 100644 > > --- a/drivers/hid/intel-ish-hid/ipc/pci-ish.c > > +++ b/drivers/hid/intel-ish-hid/ipc/pci-ish.c > > @@ -119,42 +119,6 @@ static inline bool ish_should_leave_d0i3(struct > > pci_dev *pdev) > > return !pm_resume_via_firmware() || pdev->device == > > CHV_DEVICE_ID; } > > > > -static int enable_gpe(struct device *dev) -{ -#ifdef CONFIG_ACPI > > - acpi_status acpi_sts; > > - struct acpi_device *adev; > > - struct acpi_device_wakeup *wakeup; > > - > > - adev = ACPI_COMPANION(dev); > > - if (!adev) { > > - dev_err(dev, "get acpi handle failed\n"); > > - return -ENODEV; > > - } > > - wakeup = &adev->wakeup; > > - > > - acpi_sts = acpi_enable_gpe(wakeup->gpe_device, wakeup- > > >gpe_number); > > - if (ACPI_FAILURE(acpi_sts)) { > > - dev_err(dev, "enable ose_gpe failed\n"); > > - return -EIO; > > - } > > - > > - return 0; > > -#else > > - return -ENODEV; > > -#endif > > -} > > - > > -static void enable_pme_wake(struct pci_dev *pdev) -{ > > - if ((pci_pme_capable(pdev, PCI_D0) || > > - pci_pme_capable(pdev, PCI_D3hot) || > > - pci_pme_capable(pdev, PCI_D3cold)) && !enable_gpe(&pdev- > > >dev)) { > > - pci_pme_active(pdev, true); > > - dev_dbg(&pdev->dev, "ish ipc driver pme wake > > enabled\n"); > > - } > > -} > > - > > /** > > * ish_probe() - PCI driver probe callback > > * @pdev: pci device > > @@ -225,7 +189,7 @@ static int ish_probe(struct pci_dev *pdev, const > > struct pci_device_id *ent) > > > > /* Enable PME for EHL */ > > if (pdev->device == EHL_Ax_DEVICE_ID) > > - enable_pme_wake(pdev); > > + device_init_wakeup(dev, true); > > > > ret = ish_init(ishtp); > > if (ret) > > @@ -248,6 +212,19 @@ static void ish_remove(struct pci_dev *pdev) > > ish_device_disable(ishtp_dev); } > > > > + > > +/** > > + * ish_shutdown() - PCI driver shutdown callback > > + * @pdev: pci device > > + * > > + * This function sets up wakeup for S5 */ static void > > +ish_shutdown(struct pci_dev *pdev) { > > + if (pdev->device == EHL_Ax_DEVICE_ID) > > + pci_prepare_to_sleep(pdev); } > > + > > static struct device __maybe_unused *ish_resume_device; > > > > /* 50ms to get resume response */ > > @@ -370,13 +347,6 @@ static int __maybe_unused ish_resume(struct > > device *device) > > struct pci_dev *pdev = to_pci_dev(device); > > struct ishtp_device *dev = pci_get_drvdata(pdev); > > > > - /* add this to finish power flow for EHL */ > > - if (dev->pdev->device == EHL_Ax_DEVICE_ID) { > > - pci_set_power_state(pdev, PCI_D0); > > - enable_pme_wake(pdev); > > - dev_dbg(dev->devc, "set power state to D0 for ehl\n"); > > - } > > - > > ish_resume_device = device; > > dev->resume_flag = 1; > > > > @@ -392,6 +362,7 @@ static struct pci_driver ish_driver = { > > .id_table = ish_pci_tbl, > > .probe = ish_probe, > > .remove = ish_remove, > > + .shutdown = ish_shutdown, > > .driver.pm = &ish_pm_ops, > > }; > > >
