Messages in this thread | | | From | "Rafael J. Wysocki" <> | Subject | Re: [PATCH -v5 3/3] PCIe, PM, Add PCIe runtime D3cold support | Date | Sat, 16 Jun 2012 23:09:01 +0200 |
| |
On Saturday, June 16, 2012, huang ying wrote: > On Sat, Jun 16, 2012 at 5:21 AM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > [snip] > > > > > @@ -731,9 +791,13 @@ int pci_set_power_state(struct pci_dev * > > > { > > > int error; > > > > > > - /* bound the state we're entering */ > > > - if (state > PCI_D3hot) > > > + /* D3cold during system suspend/hibernate is not supported */ > > > + if (!dev->runtime_d3cold && state > PCI_D3hot) > > > > I don't think this is the right place for the dev->runtime_d3cold check, > > I've already told you that and haven't heard anything from you causing me > > to change my mind. > > > > If you don't want D3cold to be used during system sleep transitions, > > please make sure that D3cold is not passed to pci_set_power_state() in > > those cases. > > > > OK. Checked pci_set_power_state usage with following command line: > > $ grep 'pci_set_power_state' -r . | grep -v PCI_D3hot | grep -v PCI_D0 | > grep -v pci_choose_state > $ grep 'pci_set_power_state' -r . | grep -v PCI_D3hot | grep -v PCI_D0 | > grep -v pci_choose_state | grep -i d3cold > > Found that, almost all pci_set_power_state usage is: > > 1) set to PCI_Dx (not PCI_D3cold) > 2) set to the state that choosed by pci_choose_state() > 3) driver use pci_prepare_to_sleep(), which will call pci_set_power_state() > > The only two exceptions are as follow: > > ./drivers/misc/cb710/core.c: pci_set_power_state(pdev, > PCI_D3cold); > ./arch/x86/pci/mrst.c: pci_set_power_state(dev, PCI_D3cold);
Well, no one is supposed to pass PCI_D3cold to pci_set_power_state() before the $subject patch, so they should be fixed anyway.
> And pci_choose_state() will not return PCI_D3cold for system suspend. > > So what do you think about the following solution? > > a) Change pci_prepare_to_sleep(), so that it will not put device into > PCI_D3cold. > b) Change PCI_D3cold to PCI_D3hot in the following two places, > > ./drivers/misc/cb710/core.c: pci_set_power_state(pdev, > PCI_D3cold); > ./arch/x86/pci/mrst.c: pci_set_power_state(dev, PCI_D3cold);
Yes, that will do.
Thanks, Rafael
| |