On Fri, Aug 02, 2024 at 10:07:39PM +0200, Lukas Wunner wrote: > On Fri, Aug 02, 2024 at 01:19:24PM +0200, Rafael J. Wysocki wrote: > > On Fri, Aug 2, 2024 at 11:49AM Lukas Wunner <lukas@xxxxxxxxx> wrote: > > > > > > On Fri, Aug 02, 2024 at 11:25:00AM +0530, Manivannan Sadhasivam via B4 Relay wrote: > > > > PCI core is already caching the value of pci_bridge_d3_possible() in > > > > pci_dev::bridge_d3 during pci_pm_init(). Since the value is not going to > > > > change, > > > > Is that really the case? > > > > Have you seen pci_bridge_d3_update()? > > Okay the value may change at runtime, e.g. due to user space > manipulating d3cold_allowed in sysfs. > The last part of the commit message is wrong, but pci_bridge_d3_update() is already updating pci_dev::bridge_d3. And pci_bridge_d3_possible() is not making use of any checks that could change dynamically IIUC. So what is wrong in using pci_dev::bridge_d3? Even more, if the client drivers have changed the state of pci_dev::bridge_d3 during runtime, then pci_bridge_d3_possible() won't catch that. Or is there a reason to not do so purposefully? > > > I don't know if there was a reason to call pci_bridge_d3_possible() > > > (instead of using the cached value) on probe, remove and shutdown. > > > > > > The change is probably safe but it would still be good to get some > > > positive test results with Thunderbolt laptops etc to raise the > > > confidence. > > > > If I'm not mistaken, the change is not correct. > > You're right. Because the value may change, different code paths > may be chosen on probe, remove and shutdown. Sorry for missing that. > Again, pci_bridge_d3_possible() is not making use of values that could change dynamically. - Mani -- மணிவண்ணன் சதாசிவம்
