Hi Even, On Mon, Sep 18, 2023 at 8:33 AM Xu, Even <even.xu@xxxxxxxxx> wrote: > > 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. The patch is to solve the GPE refcount overflow, while maintaining S5 wakeup. I don't have any mean to test S5 wake. So if you also don't have ways to verify S5 wake functionality, maybe we can simply revert 2e23a70edabe ("HID: intel-ish-hid: ipc: finish power flow for EHL OOB") as alternative? Kai-Heng > 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, > > > }; > > > > >
