Date:   Thu, 9 Feb 2023 12:12:27 +0100
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Petlozu Pravareshwar <petlozup@nvidia.com>
Cc:     Manikanta Maddireddy <mmaddireddy@nvidia.com>,
        "bhelgaas@google.com" <bhelgaas@google.com>,
        "rafael.j.wysocki@intel.com" <rafael.j.wysocki@intel.com>,
        "lpieralisi@kernel.org" <lpieralisi@kernel.org>,
        "robh@kernel.org" <robh@kernel.org>,
        "jeffy.chen@rock-chips.com" <jeffy.chen@rock-chips.com>,
        "krzysztof.kozlowski+dt@linaro.org" 
        <krzysztof.kozlowski+dt@linaro.org>,
        Jonathan Hunter <jonathanh@nvidia.com>,
        "dmitry.osipenko@collabora.com" <dmitry.osipenko@collabora.com>,
        "viresh.kumar@linaro.org" <viresh.kumar@linaro.org>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "steven.price@arm.com" <steven.price@arm.com>,
        "kw@linux.com" <kw@linux.com>,
        "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
        "devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>,
        "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
        Vidya Sagar <vidyas@nvidia.com>
Subject: Re: [RFC,v14 4/5] arm64: tegra: Add PCIe port node with PCIe WAKE#
 for C1 controller
Message-ID: <Y+TVG4gUbbw5Vehi@orome>
References: <20230208111645.3863534-1-mmaddireddy@nvidia.com>
 <20230208111645.3863534-5-mmaddireddy@nvidia.com>
 <Y+OJaGY6mcxM0JOF@orome>
 <1b24e9f5-539a-dd0f-6485-5dbf3757ef27@nvidia.com>
 <Y+PKTNEAuPHBdwqX@orome>
 <DM6PR12MB316420D07FBB9F8CE9AF5CD3B5D99@DM6PR12MB3164.namprd12.prod.outlook.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="oI+597r9ve6kB/tz"
Content-Disposition: inline
In-Reply-To: <DM6PR12MB316420D07FBB9F8CE9AF5CD3B5D99@DM6PR12MB3164.namprd12.prod.outlook.com>
User-Agent: Mutt/2.2.9 (2022-11-12)
Precedence: bulk


--oI+597r9ve6kB/tz
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Feb 09, 2023 at 10:53:25AM +0000, Petlozu Pravareshwar wrote:
> >=20
> > On Wed, Feb 08, 2023 at 05:43:35PM +0530, Manikanta Maddireddy wrote:
> > >
> > > On 2/8/2023 5:07 PM, Thierry Reding wrote:
> > > > On Wed, Feb 08, 2023 at 04:46:44PM +0530, Manikanta Maddireddy
> > wrote:
> > > > > Add PCIe port node under the PCIe controller-1 device tree node to
> > > > > support PCIe WAKE# interrupt for WiFi.
> > > > >
> > > > > Signed-off-by: Manikanta Maddireddy <mmaddireddy@nvidia.com>
> > > > > ---
> > > > >
> > > > > Changes in v14:
> > > > > New patch in the series to support PCIe WAKE# in NVIDIA Jetson AGX
> > Orin.
> > > > >
> > > > >   .../dts/nvidia/tegra234-p3737-0000+p3701-0000.dts     | 11
> > +++++++++++
> > > > >   1 file changed, 11 insertions(+)
> > > > >
> > > > > diff --git
> > > > > a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dts
> > > > > b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dts
> > > > > index 8a9747855d6b..9c89be263141 100644
> > > > > ---
> > > > > a/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-0000.dts
> > > > > +++ b/arch/arm64/boot/dts/nvidia/tegra234-p3737-0000+p3701-
> > 0000.dt
> > > > > +++ s
> > > > > @@ -2147,6 +2147,17 @@ pcie@14100000 {
> > > > >   			phys =3D <&p2u_hsio_3>;
> > > > >   			phy-names =3D "p2u-0";
> > > > > +
> > > > > +			pci@0,0 {
> > > > > +				reg =3D <0x0000 0 0 0 0>;
> > > > > +				#address-cells =3D <3>;
> > > > > +				#size-cells =3D <2>;
> > > > > +				ranges;
> > > > > +
> > > > > +				interrupt-parent =3D <&gpio>;
> > > > > +				interrupts =3D <TEGRA234_MAIN_GPIO(L, 2)
> > IRQ_TYPE_LEVEL_LOW>;
> > > > > +				interrupt-names =3D "wakeup";
> > > > > +			};
> > > > Don't we need to wire this to the PMC interrupt controller and the
> > > > wake event corresponding to the L2 GPIO? Otherwise none of the wake
> > > > logic in PMC will get invoked.
> > > >
> > > > Thierry
> > > PCIe wake is gpio based not pmc, only wake support is provided by PMC
> > > controller.
> > > I verified this patch and able to wake up Tegra from suspend.
> > > Petlozu, correct me if my understanding is wrong.
> >=20
> > The way that this usually works is that you need to use something like
> > this:
> >=20
> > 	interrupt-parent =3D <&pmc>;
> > 	interrupts =3D <1 IRQ_TYPE_LEVEL_LOW>;
> > 	interrupt-names =3D "wakeup";
> >=20
> > This will then cause the PMC's interrupt chip callbacks to setup all th=
e wake-
> > related interrupts and use the internal wake event tables to forward the
> > GPIO/IRQ corresponding to the PMC wake event to the GPIO controller or
> > GIC, respectively.
> >=20
> > If you use &gpio as the interrupt parent, none of the PMC logic will be
> > invoked, so unless this is somehow set up correctly by default, the PMC
> > wouldn't be able to wake up the system.
> >=20
> > Thierry
> Thierry,
> Since PMC's IRQ domain is made as parent of GPIO controller's IRQ domain,
> I think, for GPIO based wakes setting &gpio as the interrupt parent can s=
till
> invoke PMC logic to program the required registers to enable such wakes.
> Related commit in this regard:
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commi=
t/drivers/gpio/gpio-tegra186.c?id=3D2a36550567307b881ce570a81189682ae1c9d08d

Heh... nicely self-owned =3D). You're right, no need for the detour in DT
with those, the GPIO driver will hook up the IRQ hierarchy itself. We
already do this for the "power" key in the various gpio-keys, so it
should work fine.

Sorry for the noise,
Thierry

--oI+597r9ve6kB/tz
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAmPk1RsACgkQ3SOs138+
s6HqwQ//Zpt5GNboNT6FQXO5Dl4h4mISvUB+mcMRdY9LdmHEZs4O8Xp85ftbrubB
uHR7S0Fvqt1EDlaPg+tzUYMBGtq/QpUy46Pi69fqHLUsTp8HnrKnIXbQ7KwEpR0z
RDrfO7dhYMo7Zv4KxuC3H8LmRIqJ54HevvW3sAx/mA2G9Et5CGcWCfUpTYnE8M6P
DFe4aSPVuetKatXz1oRz7NWR4d2S2ZsEfi7tNqhLnnSuPlJm+400Zz46+k/fmjzt
m+BKqyTjVwzvNfDdzryZiMMaC9tJMRaU9mAM4KtKwTcbQF78aHmizGdLwcegzCaj
9kCvG5tnaPhrfm6HZ1mQvgW4GgpXLola7Jpm58WoxuAGt9rjVSek1+taji44dGwV
N1Q5YrAqBzUTE6Pqm8CcaGP2LR02Jf8d2kJ0HMd7L+/OM1qCoepAepprllBW4hjM
AbHSu3hLBNt3h9IdYOLMyuTp9HqblZXaUcTsO8YNOTWcNjTSU+NT8BHmmrdOWrHj
emjW6zsMf0s3aZci5lCDa1MA6IjIjZciEPR1R6+NCKWIoccKCsqhNyAQyKblAX6L
fJFFg4bBGcy665BGBPogy9jH7L8wstPU+CItRuGrLNIfzjOhIYS6uRmP0gMCFBLS
34r9vuyy0mx7nApzj1XU4Yaoq8Gt5f9afCgxHdD+T0qCvFzuTxM=
=0MpZ
-----END PGP SIGNATURE-----

--oI+597r9ve6kB/tz--