Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34;
Message-ID: <925fd50bd609f99f0eabf93d1163106f6118f139.camel@linux.intel.com>
Subject: Re: [PATCH v2] HID: intel-ish-hid: ipc: Rework EHL OOB wakeup
From:   srinivas pandruvada <srinivas.pandruvada@linux.intel.com>
To:     Kai-Heng Feng <kai.heng.feng@canonical.com>
Cc:     jikos@kernel.org, benjamin.tissoires@redhat.com,
        linux-pm@vger.kernel.org, linux-pci@vger.kernel.org,
        Jian Hui Lee <jianhui.lee@canonical.com>,
        Even Xu <even.xu@intel.com>, linux-input@vger.kernel.org,
        linux-kernel@vger.kernel.org
Date:   Thu, 07 Dec 2023 08:48:30 -0500
In-Reply-To: <CAAd53p7do2rrB=qbpWKbNCWB_qfZ2YZPtB_55VcfGjDYXgLfzA@mail.gmail.com>
References: <20231108121940.288005-1-kai.heng.feng@canonical.com>
         <cc6c162407c69c53ec256bf887a0384361dd0516.camel@linux.intel.com>
         <CAAd53p7do2rrB=qbpWKbNCWB_qfZ2YZPtB_55VcfGjDYXgLfzA@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
User-Agent: Evolution 3.48.4 (3.48.4-1.fc38) 
MIME-Version: 1.0
Precedence: bulk

On Thu, 2023-12-07 at 10:44 +0800, Kai-Heng Feng wrote:
> On Tue, Dec 5, 2023 at 1:50=E2=80=AFAM srinivas pandruvada
> <srinivas.pandruvada@linux.intel.com> wrote:
> >=20
> > Hi Kai,
> >=20
> > Sorry for he delay in getting back on this. I have a question
> > below:
> >=20
> > On Wed, 2023-11-08 at 14:19 +0200, Kai-Heng Feng wrote:
> > > 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.
> > >=20
> > > Also add a shutdown callback which uses pci_prepare_to_sleep() to
> > > let
> > > PCI and ACPI set OOB wakeup for S5.
> > >=20
> > > Cc: Jian Hui Lee <jianhui.lee@canonical.com>
> > > Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
> > > ---
> > > v2:
> > > =C2=A0Rebase on ("HID: intel-ish-hid: ipc: Disable and reenable ACPI
> > > GPE
> > > bit")
> > >=20
> > > =C2=A0drivers/hid/intel-ish-hid/ipc/pci-ish.c | 67 ++++++------------=
-
> > > ----
> > > --
> > >=20
...
...

> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 device_init_wakeup(dev, true);
> >=20
> > For apple to apple comparison, which path will call
> > https://elixir.bootlin.com/linux/latest/C/ident/__pci_enable_wake
> > which will call pci_pme_active()?
>=20
> Here's the flow:
> device_shutdown()
> =C2=A0 pci_device_shutdown()
> =C2=A0=C2=A0=C2=A0 ish_shutdown()
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 pci_prepare_to_sleep()
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 __pci_enable_wake()
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 pci_pme_active()
> =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 __pci_=
pme_active()

Thanks. I will send my ACK to the original patch.

Thanks,
Srinivas

>=20
> Kai-Heng
>=20
> >=20
> > Thanks,
> > Srinivas
> >=20
> > >=20
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ret =3D ish_init(ishtp);
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (ret)
> > > @@ -256,6 +212,19 @@ static void ish_remove(struct pci_dev *pdev)
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ish_device_disable(ishtp_d=
ev);
> > > =C2=A0}
> > >=20
> > > +
> > > +/**
> > > + * ish_shutdown() - PCI driver shutdown callback
> > > + * @pdev:=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 pci device
> > > + *
> > > + * This function sets up wakeup for S5
> > > + */
> > > +static void ish_shutdown(struct pci_dev *pdev)
> > > +{
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (pdev->device =3D=3D EHL_Ax_=
DEVICE_ID)
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 pci_prepare_to_sleep(pdev);
> > > +}
> > > +
> > > =C2=A0static struct device __maybe_unused *ish_resume_device;
> > >=20
> > > =C2=A0/* 50ms to get resume response */
> > > @@ -378,13 +347,6 @@ static int __maybe_unused ish_resume(struct
> > > device *device)
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct pci_dev *pdev =3D t=
o_pci_dev(device);
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 struct ishtp_device *dev =
=3D pci_get_drvdata(pdev);
> > >=20
> > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 /* add this to finish power flo=
w for EHL */
> > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 if (dev->pdev->device =3D=3D EH=
L_Ax_DEVICE_ID) {
> > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 pci_set_power_state(pdev, PCI_D0);
> > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 enable_pme_wake(pdev);
> > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=
=C2=A0=C2=A0=C2=A0 dev_dbg(dev->devc, "set power state to D0 for
> > > ehl\n");
> > > -=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 }
> > > -
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ish_resume_device =3D devi=
ce;
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 dev->resume_flag =3D 1;
> > >=20
> > > @@ -400,6 +362,7 @@ static struct pci_driver ish_driver =3D {
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 .id_table =3D ish_pci_tbl,
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 .probe =3D ish_probe,
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 .remove =3D ish_remove,
> > > +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 .shutdown =3D ish_shutdown,
> > > =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 .driver.pm =3D &ish_pm_ops=
,
> > > =C2=A0};
> > >=20
> >=20