On Fri, Aug 02, 2024 at 12:13:31PM +0200, Lukas Wunner wrote: > 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. > Agree with you! Even after doing upstreaming for this much time, I tend to ignore this... > > > 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. > Why? D3Hot is useful for runtime PM and if the platform doesn't want to do runtime PM, it can always skip D3Hot (not ideal though). But D3Cold is a power off state, and the platform may choose to use it for the case of system suspend. So I still feel that decoupling D3Hot and D3Cold is necessary. - Mani -- மணிவண்ணன் சதாசிவம்
