Re: [PATCH v5 4/4] PCI: Allow PCI bridges to go to D3Hot on all Devicetree based platforms

[Lukas Wunner <lukas@wunner.de>]

On Fri, Aug 02, 2024 at 11:25:03AM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> Unlike ACPI based platforms, there are no known issues with D3Hot for the
> PCI bridges in the Devicetree based platforms. So let's allow the PCI
> bridges to go to D3Hot during runtime. It should be noted that the bridges
> need to be defined in Devicetree for this to work.
[...]
> +		if (state == PCI_D3hot && dev_of_node(&bridge->dev))
> +			return true;

For such a simple change which several parties are interested in,
I think it would be better to move it to the front of the series.

Patch [1/4] looks like an optimization (using a cached value)
which this patch doesn't depend on.  Patch [2/4] looks like a
change of bikeshed color which isn't strictly necessary for
this fourth patch either.  If you want to propose those changes,
fine, but by making this fourth patch depend on them, you risk
delaying its acceptance.  As an upstreaming strategy it's usually
smarter to move potentially controversial or unnecessary material
to the back of the series, or submit it separately if it can be
applied standalone.


> Currently, D3Cold is not allowed since Vcc supply which is required for
> transitioning the device to D3Cold is not exposed on all Devicetree based
> platforms.

The PCI core cannot put devices into D3cold without help from the
platform.  Checking whether D3cold is possible (or allowed or
whatever) thus requires asking platform support code via
platform_pci_power_manageable(), platform_pci_choose_state() etc.

I think patch [3/4] is a little confusing because it creates
infrastructure to decide whether D3cold is supported (allowed?)
but we already have that in the platform_pci_*() functions.
So I'm not sure if patch [3/4] adds value.  I think generally
speaking if D3hot isn't possible (allowed?), D3cold is assumed
to not be possible either.

Thanks,

Lukas