@@ -233,6 +234,12 @@ static int snd_rn_acp_probe(struct pci_dev *pci, ret = PTR_ERR(adata->pdev); goto unregister_devs; }
- pm_runtime_set_autosuspend_delay(&pci->dev,
ACP_SUSPEND_DELAY_MS);
- pm_runtime_use_autosuspend(&pci->dev);
- pm_runtime_set_active(&pci->dev);
is the set_active() needed? I haven't seen this in the other PCI audio drivers? >
We have similar implementation in our Raven ACP PCI driver as well which got up streamed. I will give a try by modifying this sequence. Could you please point me , what's exactly wrong with this code?
you would use pm_runtime_set_active() if the device was suspended. I don't think this can possibly happen since there is a _get done by the PCI core, which you compensate for in the line below.
Also look at drivers/pci/pci.c, the core already does this set_active() and _enable().
void pci_pm_init(struct pci_dev *dev) { ...
pm_runtime_forbid(&dev->dev); pm_runtime_set_active(&dev->dev); pm_runtime_enable(&dev->dev);
- pm_runtime_put_noidle(&pci->dev);
- pm_runtime_enable(&pci->dev);
same, is the _enable() needed()?
We have similar implementation in Raven ACP PCI driver as well.
It's quite common unfortunately that extended pm_runtime sequences are used without checking what's necessary - it took Intel some time to clearly define what we needed and what was redundant/noop.
pm_runtime_allow(&pci->dev); return 0;
unregister_devs:
@@ -250,6 +257,42 @@ static int snd_rn_acp_probe(struct pci_dev *pci, return ret; }