| Message ID | 20230208111645.3863534-5-mmaddireddy@nvidia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add DT based PCIe wake support in PCI core driver | expand |
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.dts > @@ -2147,6 +2147,17 @@ pcie@14100000 { > > phys = <&p2u_hsio_3>; > phy-names = "p2u-0"; > + > + pci@0,0 { > + reg = <0x0000 0 0 0 0>; > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + > + interrupt-parent = <&gpio>; > + interrupts = <TEGRA234_MAIN_GPIO(L, 2) IRQ_TYPE_LEVEL_LOW>; > + interrupt-names = "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
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.dts >> @@ -2147,6 +2147,17 @@ pcie@14100000 { >> >> phys = <&p2u_hsio_3>; >> phy-names = "p2u-0"; >> + >> + pci@0,0 { >> + reg = <0x0000 0 0 0 0>; >> + #address-cells = <3>; >> + #size-cells = <2>; >> + ranges; >> + >> + interrupt-parent = <&gpio>; >> + interrupts = <TEGRA234_MAIN_GPIO(L, 2) IRQ_TYPE_LEVEL_LOW>; >> + interrupt-names = "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. Manikanta
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.dts > > > @@ -2147,6 +2147,17 @@ pcie@14100000 { > > > phys = <&p2u_hsio_3>; > > > phy-names = "p2u-0"; > > > + > > > + pci@0,0 { > > > + reg = <0x0000 0 0 0 0>; > > > + #address-cells = <3>; > > > + #size-cells = <2>; > > > + ranges; > > > + > > > + interrupt-parent = <&gpio>; > > > + interrupts = <TEGRA234_MAIN_GPIO(L, 2) IRQ_TYPE_LEVEL_LOW>; > > > + interrupt-names = "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. The way that this usually works is that you need to use something like this: interrupt-parent = <&pmc>; interrupts = <1 IRQ_TYPE_LEVEL_LOW>; interrupt-names = "wakeup"; This will then cause the PMC's interrupt chip callbacks to setup all the 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. 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. Thierry
> > 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 = <&p2u_hsio_3>; > > > > phy-names = "p2u-0"; > > > > + > > > > + pci@0,0 { > > > > + reg = <0x0000 0 0 0 0>; > > > > + #address-cells = <3>; > > > > + #size-cells = <2>; > > > > + ranges; > > > > + > > > > + interrupt-parent = <&gpio>; > > > > + interrupts = <TEGRA234_MAIN_GPIO(L, 2) > IRQ_TYPE_LEVEL_LOW>; > > > > + interrupt-names = "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. > > The way that this usually works is that you need to use something like > this: > > interrupt-parent = <&pmc>; > interrupts = <1 IRQ_TYPE_LEVEL_LOW>; > interrupt-names = "wakeup"; > > This will then cause the PMC's interrupt chip callbacks to setup all the 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. > > 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. > > 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 still 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/commit/drivers/gpio/gpio-tegra186.c?id=2a36550567307b881ce570a81189682ae1c9d08d Thanks.
On Thu, Feb 09, 2023 at 10:53:25AM +0000, Petlozu Pravareshwar wrote: > > > > 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 = <&p2u_hsio_3>; > > > > > phy-names = "p2u-0"; > > > > > + > > > > > + pci@0,0 { > > > > > + reg = <0x0000 0 0 0 0>; > > > > > + #address-cells = <3>; > > > > > + #size-cells = <2>; > > > > > + ranges; > > > > > + > > > > > + interrupt-parent = <&gpio>; > > > > > + interrupts = <TEGRA234_MAIN_GPIO(L, 2) > > IRQ_TYPE_LEVEL_LOW>; > > > > > + interrupt-names = "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. > > > > The way that this usually works is that you need to use something like > > this: > > > > interrupt-parent = <&pmc>; > > interrupts = <1 IRQ_TYPE_LEVEL_LOW>; > > interrupt-names = "wakeup"; > > > > This will then cause the PMC's interrupt chip callbacks to setup all the 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. > > > > 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. > > > > 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 still > 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/commit/drivers/gpio/gpio-tegra186.c?id=2a36550567307b881ce570a81189682ae1c9d08d Heh... nicely self-owned =). 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
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.dts > @@ -2147,6 +2147,17 @@ pcie@14100000 { > > phys = <&p2u_hsio_3>; > phy-names = "p2u-0"; > + > + pci@0,0 { > + reg = <0x0000 0 0 0 0>; > + #address-cells = <3>; > + #size-cells = <2>; > + ranges; > + > + interrupt-parent = <&gpio>; > + interrupts = <TEGRA234_MAIN_GPIO(L, 2) IRQ_TYPE_LEVEL_LOW>; > + interrupt-names = "wakeup"; WAKE# should be part of the PCIe controller, not device. And the interrupt name should be "wake". - Mani > + }; > }; > > pcie@14160000 { > -- > 2.25.1 >
On 06-12-2023 21:06, Manivannan Sadhasivam wrote: > External email: Use caution opening links or attachments > > > 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.dts >> @@ -2147,6 +2147,17 @@ pcie@14100000 { >> >> phys = <&p2u_hsio_3>; >> phy-names = "p2u-0"; >> + >> + pci@0,0 { >> + reg = <0x0000 0 0 0 0>; >> + #address-cells = <3>; >> + #size-cells = <2>; >> + ranges; >> + >> + interrupt-parent = <&gpio>; >> + interrupts = <TEGRA234_MAIN_GPIO(L, 2) IRQ_TYPE_LEVEL_LOW>; >> + interrupt-names = "wakeup"; > WAKE# should be part of the PCIe controller, not device. And the interrupt name > should be "wake". > > - Mani Hi, Please refer to the discussion in below link, WAKE# is per PCI bridge. https://patchwork.ozlabs.org/project/linux-pci/patch/20171226020806.32710-2-jeffy.chen@rock-chips.com/ I carried wakeup name defined in previous version, but wake seems to be sufficient. Thanks, Manikanta > >> + }; >> }; >> >> pcie@14160000 { >> -- >> 2.25.1 >> > -- > மணிவண்ணன் சதாசிவம்
On Thu, Dec 07, 2023 at 12:54:04PM +0530, Manikanta Maddireddy wrote: > > On 06-12-2023 21:06, Manivannan Sadhasivam wrote: > > External email: Use caution opening links or attachments > > > > > > 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.dts > > > @@ -2147,6 +2147,17 @@ pcie@14100000 { > > > > > > phys = <&p2u_hsio_3>; > > > phy-names = "p2u-0"; > > > + > > > + pci@0,0 { > > > + reg = <0x0000 0 0 0 0>; > > > + #address-cells = <3>; > > > + #size-cells = <2>; > > > + ranges; > > > + > > > + interrupt-parent = <&gpio>; > > > + interrupts = <TEGRA234_MAIN_GPIO(L, 2) IRQ_TYPE_LEVEL_LOW>; > > > + interrupt-names = "wakeup"; > > WAKE# should be part of the PCIe controller, not device. And the interrupt name > > should be "wake". > > > > - Mani > Hi, > > Please refer to the discussion in below link, WAKE# is per PCI bridge. > https://patchwork.ozlabs.org/project/linux-pci/patch/20171226020806.32710-2-jeffy.chen@rock-chips.com/ > PCIe Host controller (RC) usually represents host bridge + PCI-PCI bridge. We do not represent the PCI-PCI bridge in devicetree for any platforms, but only RC as a whole. Moreover, PERST# is already defined in RC node. So it becomes confusing if WAKE# is defined in a child node representing bridge. So please move WAKE# to RC node. - Mani > I carried wakeup name defined in previous version, but wake seems to be > sufficient. > > Thanks, > Manikanta > > > > > + }; > > > }; > > > > > > pcie@14160000 { > > > -- > > > 2.25.1 > > > > > -- > > மணிவண்ணன் சதாசிவம்
On 07-12-2023 13:29, Manivannan Sadhasivam wrote: > External email: Use caution opening links or attachments > > > On Thu, Dec 07, 2023 at 12:54:04PM +0530, Manikanta Maddireddy wrote: >> On 06-12-2023 21:06, Manivannan Sadhasivam wrote: >>> External email: Use caution opening links or attachments >>> >>> >>> 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.dts >>>> @@ -2147,6 +2147,17 @@ pcie@14100000 { >>>> >>>> phys = <&p2u_hsio_3>; >>>> phy-names = "p2u-0"; >>>> + >>>> + pci@0,0 { >>>> + reg = <0x0000 0 0 0 0>; >>>> + #address-cells = <3>; >>>> + #size-cells = <2>; >>>> + ranges; >>>> + >>>> + interrupt-parent = <&gpio>; >>>> + interrupts = <TEGRA234_MAIN_GPIO(L, 2) IRQ_TYPE_LEVEL_LOW>; >>>> + interrupt-names = "wakeup"; >>> WAKE# should be part of the PCIe controller, not device. And the interrupt name >>> should be "wake". >>> >>> - Mani >> Hi, >> >> Please refer to the discussion in below link, WAKE# is per PCI bridge. >> https://patchwork.ozlabs.org/project/linux-pci/patch/20171226020806.32710-2-jeffy.chen@rock-chips.com/ >> > PCIe Host controller (RC) usually represents host bridge + PCI-PCI bridge. We do > not represent the PCI-PCI bridge in devicetree for any platforms, but only RC as > a whole. > > Moreover, PERST# is already defined in RC node. So it becomes confusing if > WAKE# is defined in a child node representing bridge. > > So please move WAKE# to RC node. > > - Mani Hi, We can define PCI-PCI bridge in device tree, refer to below device tree which has 3 ports under a controller, with PERST#(reset-gpios) defined per port. https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/apple/t8103.dtsi#n749 Also, of_pci_setup_wake_irq() in below patch is parsing "wakeup" from PCI bridge, not from the host bridge. https://patchwork.ozlabs.org/project/linux-pci/patch/20230208111645.3863534-4-mmaddireddy@nvidia.com/ If a controller has only one port it has to define a PCI bridge under controller device tree node and add wakeup interrupt property, refer to below patch from original author. https://www.spinics.net/lists/linux-pci/msg135569.html Thanks, Manikanta > >> I carried wakeup name defined in previous version, but wake seems to be >> sufficient. >> >> Thanks, >> Manikanta >>>> + }; >>>> }; >>>> >>>> pcie@14160000 { >>>> -- >>>> 2.25.1 >>>> >>> -- >>> மணிவண்ணன் சதாசிவம் > -- > மணிவண்ணன் சதாசிவம்
On Thu, Dec 07, 2023 at 02:23:46PM +0530, Manikanta Maddireddy wrote: > > On 07-12-2023 13:29, Manivannan Sadhasivam wrote: > > External email: Use caution opening links or attachments > > > > > > On Thu, Dec 07, 2023 at 12:54:04PM +0530, Manikanta Maddireddy wrote: > > > On 06-12-2023 21:06, Manivannan Sadhasivam wrote: > > > > External email: Use caution opening links or attachments > > > > > > > > > > > > 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.dts > > > > > @@ -2147,6 +2147,17 @@ pcie@14100000 { > > > > > > > > > > phys = <&p2u_hsio_3>; > > > > > phy-names = "p2u-0"; > > > > > + > > > > > + pci@0,0 { > > > > > + reg = <0x0000 0 0 0 0>; > > > > > + #address-cells = <3>; > > > > > + #size-cells = <2>; > > > > > + ranges; > > > > > + > > > > > + interrupt-parent = <&gpio>; > > > > > + interrupts = <TEGRA234_MAIN_GPIO(L, 2) IRQ_TYPE_LEVEL_LOW>; > > > > > + interrupt-names = "wakeup"; > > > > WAKE# should be part of the PCIe controller, not device. And the interrupt name > > > > should be "wake". > > > > > > > > - Mani > > > Hi, > > > > > > Please refer to the discussion in below link, WAKE# is per PCI bridge. > > > https://patchwork.ozlabs.org/project/linux-pci/patch/20171226020806.32710-2-jeffy.chen@rock-chips.com/ > > > > > PCIe Host controller (RC) usually represents host bridge + PCI-PCI bridge. We do > > not represent the PCI-PCI bridge in devicetree for any platforms, but only RC as > > a whole. > > > > Moreover, PERST# is already defined in RC node. So it becomes confusing if > > WAKE# is defined in a child node representing bridge. > > > > So please move WAKE# to RC node. > > > > - Mani > > Hi, > > We can define PCI-PCI bridge in device tree, refer to below device tree > which has 3 ports under a controller, > with PERST#(reset-gpios) defined per port. > > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/arch/arm64/boot/dts/apple/t8103.dtsi#n749 > Hmm. For RCs with single bridge, we never defined a DT node (atleast on Qcom platforms). But I think it is the time to fix them. > Also, of_pci_setup_wake_irq() in below patch is parsing "wakeup" from PCI > bridge, not from the host bridge. > https://patchwork.ozlabs.org/project/linux-pci/patch/20230208111645.3863534-4-mmaddireddy@nvidia.com/ > I didn't say that WAKE# should be parsed from host bridge, it doesn't make sense. But I get your point. > If a controller has only one port it has to define a PCI bridge under > controller device tree node and > add wakeup interrupt property, refer to below patch from original author. > > https://www.spinics.net/lists/linux-pci/msg135569.html > Yes, I agree. Thanks for the clarification. - Mani > Thanks, > Manikanta > > > > > I carried wakeup name defined in previous version, but wake seems to be > > > sufficient. > > > > > > Thanks, > > > Manikanta > > > > > + }; > > > > > }; > > > > > > > > > > pcie@14160000 { > > > > > -- > > > > > 2.25.1 > > > > > > > > > -- > > > > மணிவண்ணன் சதாசிவம் > > -- > > மணிவண்ணன் சதாசிவம்
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.dts @@ -2147,6 +2147,17 @@ pcie@14100000 { phys = <&p2u_hsio_3>; phy-names = "p2u-0"; + + pci@0,0 { + reg = <0x0000 0 0 0 0>; + #address-cells = <3>; + #size-cells = <2>; + ranges; + + interrupt-parent = <&gpio>; + interrupts = <TEGRA234_MAIN_GPIO(L, 2) IRQ_TYPE_LEVEL_LOW>; + interrupt-names = "wakeup"; + }; }; pcie@14160000 {
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(+)