Re: [PATCH 0/2] Enable D3 support for Qualcomm bridges

[Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org>]

On Fri, Feb 02, 2024 at 10:00:33AM +0100, Lukas Wunner wrote:
> On Fri, Feb 02, 2024 at 12:24:16PM +0530, Manivannan Sadhasivam wrote:
> > This series enables D3 support for PCI bridges found in Qcom SoCs. Currently,
> > PCI core will enable D3 support for PCI bridges only when the following
> > conditions are met:
> > 
> > 1. Platform is ACPI based
> > 2. Thunderbolt controller is used
> > 3. pcie_port_pm=force passed in cmdline
> > 
> > While options 1 and 2 do not apply to Qcom SoCs, option 3 will make the life
> > harder for distro maintainers. Due to this, runtime PM is also not getting
> > enabled for the bridges.
> > 
> > Ideally, D3 support should be enabled by default for the recent PCI bridges,
> > but we do not have a sane way to detect them. So this series adds a new flag
> > "bridge_d3_capable" to "struct pci_dev" which could be set by the bridge
> > drivers for capable devices. This will allow the PCI core to enable D3
> > support for the bridges during enumeration.
> 
> I think the right way to do this is to use the existing call to
> platform_pci_bridge_d3() in pci_bridge_d3_possible().
> 
> Please amend platform_pci_bridge_d3() to call a new of_pci_bridge_d3()
> function which determines whether D3 is supported by the platform.
> 
> E.g. of_pci_bridge_d3() could contain a whitelist of supported VID/DID
> tuples.  Or it could be defined as a __weak function which always
> returns false but can be overridden at link time by a function
> defined somewhere in arch/arm/, arch/arm64/ or in some driver
> whose Kconfig option is enabled in Qualcomm platforms.
> 

Hmm. If we go with a DT based solution, then introducing a new property like
"d3-support" in the PCI bridge node would be the right approach. But then, it
also requires defining the PCI bridge node in all the DTs. But that should be
fine since it will help us to support WAKE# (per bridge) in the future.

Thanks for the review.

- Mani

-- 
மணிவண்ணன் சதாசிவம்