RE: [PATCH] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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 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,
>  };
>  





[Index of Archives]     [Linux Media Devel]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Linux Wireless Networking]     [Linux Omap]

  Powered by Linux