On Thu, 18 Jan 2024 15:29:01 +0100, Rob Herring <robh+dt@xxxxxxxxxx> said: > On Wed, Jan 17, 2024 at 10:08 AM Bartosz Golaszewski <brgl@xxxxxxxx> wrote: > [snip] > >> The general idea is to instantiate platform devices for child nodes of >> the PCIe port DT node. For those nodes for which a power-sequencing >> driver exists, we bind it and let it probe. The driver then triggers a >> rescan of the PCI bus with the aim of detecting the now powered-on >> device. The device will consume the same DT node as the platform, >> power-sequencing device. We use device links to make the latter become >> the parent of the former. >> >> The main advantage of this approach is not modifying the existing DT in >> any way and especially not adding any "fake" platform devices. > > Suspend/resume has been brought up already, but I disagree we can > worry about that later unless there is and always will be no power > sequencing during suspend/resume for all devices ever. Given the > supplies aren't standard, it wouldn't surprise me if standard PCI > power management isn't either. The primary issue I see with this > design is we will end up with 2 drivers doing the same power > sequencing: the platform driver for initial power on and the device's > PCI driver for suspend/resume. > > Rob > I admit that I don't have any HW where I could test it but I my thinking was that with the following relationships between the devices: ┌─────────────────────┐ │ │ │ PCI Port device │ │ │ └───┬───────────┬─────┘ │ │ │ │ │ │ ┌─────────────────────▼─────┐ │ │ │ │ │ QCA6390 pwrseq device │ │ │ │ │ └─────────────────────┬─────┘ │ │ │ │ │ │ │ ┌─────▼───────────▼───┐ │ │ │ ath11k_pci device │ │ │ └─────────────────────┘ the PM subsystem would handle the dependencies automatically and correctly setup the sequence for suspend and resume. Also: the PCI ath11k driver does not deal with the kind of resources that the power sequencing platform driver handles: regulators, GPIOs and clocks. I agree, it would be useful to have a working case of handling suspend/resume with this code though. Bartosz