On Mon, Nov 18, 2024 at 01:58:17PM +0100, Christoph Hellwig wrote: > On Mon, Nov 18, 2024 at 01:53:44PM +0530, Manivannan Sadhasivam wrote: > > PCI core allows users to configure the D3Cold state for each PCI device > > through the sysfs attribute '/sys/bus/pci/devices/.../d3cold_allowed'. This > > attribute sets the 'pci_dev:d3cold_allowed' flag and could be used by users > > to allow/disallow the PCI devices to enter D3Cold during system suspend. > > > > So make use of this flag in the NVMe driver to shutdown the NVMe device > > during system suspend if the user has allowed D3Cold for the device. > > Existing checks in the NVMe driver decide whether to shut down the device > > (based on platform/device limitations), so use this flag as the last resort > > to keep the existing behavior. > > Umm, what? The documentation of this attribute says: > > "d3cold_allowed is bit to control whether the corresponding PCI > device can be put into D3Cold state. If it is cleared, the > device will never be put into D3Cold state. If it is set, the > device may be put into D3Cold state if other requirements are > satisfied too. Reading this attribute will show the current > value of d3cold_allowed bit. Writing this attribute will > the value of d3cold_allowed bit." > > Which honestly already sounds rather non-specific, but everything but > a mandate for drivers to act on it. > > The only place currently checking it is pci_dev_check_d3cold in the > PCI core, which is used to set the bridge_d3 attibute. > Yeah, it is pretty much used internally up until now. But the attribute looks like a close match of what I could find for this usecase and that's why I used it. > So blindly using it in a driver to force a different PM strategy feels > completely wrong. Even if the attrite should have that effect it > needs to happen through a well documented PCI or PM layer helper and > open coded like this. > Ok. I'd like to get some feedback from Bjorn H (PCI maintainer) about using this attribute before moving forward with a helper. Thanks! - Mani -- மணிவண்ணன் சதாசிவம்
