Message ID | 200909212337.01812.rjw@sisk.pl (mailing list archive) |
---|---|
State | Superseded, archived |
Headers | show |
On Mon, 21 Sep 2009 23:37:01 +0200 "Rafael J. Wysocki" <rjw@sisk.pl> wrote: > > - dev->current_state = state; > + pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); > + dev->current_state = (pmcsr & PCI_PM_CTRL_STATE_MASK); > + /* Return error code if we have failed to change the state */ > + if (dev->current_state != state) > + dev_info(&dev->dev, "Refused to change power state, " > + "currently in D%d\n", dev->current_state); I would suspect that you want this message only once... to avoid it flooding logs and such or at least ratelimiting it
Hi Rafael, first you say "returning an error code in such cases would..." and then the patch content has...? Andreas Mohr -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 22 September 2009, Andreas Mohr wrote: > Hi Rafael, Hi, > first you say "returning an error code in such cases would..." > and then the patch content has...? I don't quite understand your comment, care to elaborate? Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 22 September 2009, Arjan van de Ven wrote: > On Mon, 21 Sep 2009 23:37:01 +0200 > "Rafael J. Wysocki" <rjw@sisk.pl> wrote: > > > > - dev->current_state = state; > > + pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); > > + dev->current_state = (pmcsr & PCI_PM_CTRL_STATE_MASK); > > + /* Return error code if we have failed to change the state */ > > + if (dev->current_state != state) > > + dev_info(&dev->dev, "Refused to change power state, " > > + "currently in D%d\n", dev->current_state); > > I would suspect that you want this message only once... > to avoid it flooding logs and such Hmm? I don't think we use pci_set_power_state() _that_ often. > or at least ratelimiting it That sounds like a good idea, though, will respin. -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tuesday 22 September 2009, kevin granade wrote: > On Tue, Sep 22, 2009 at 2:50 PM, Rafael J. Wysocki <rjw@sisk.pl> wrote: > > > On Tuesday 22 September 2009, Andreas Mohr wrote: > > > Hi Rafael, > > > > Hi, > > > > > first you say "returning an error code in such cases would..." > > > and then the patch content has...? > > > > I don't quite understand your comment, care to elaborate? > > > > > I think this is what was being referenced: > > + /* Return error code if we have failed to change the state */ > + if (dev->current_state != state) > + dev_info(&dev->dev, "Refused to change power state, " > + "currently in D%d\n", dev->current_state); > > The comment says, "Return error code", where it should probably say, "Log > error". Or possibly no comment at all. Ah, thanks. Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: linux-2.6/drivers/pci/pci.c =================================================================== --- linux-2.6.orig/drivers/pci/pci.c +++ linux-2.6/drivers/pci/pci.c @@ -513,7 +513,12 @@ static int pci_raw_set_power_state(struc else if (state == PCI_D2 || dev->current_state == PCI_D2) udelay(PCI_PM_D2_DELAY); - dev->current_state = state; + pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); + dev->current_state = (pmcsr & PCI_PM_CTRL_STATE_MASK); + /* Return error code if we have failed to change the state */ + if (dev->current_state != state) + dev_info(&dev->dev, "Refused to change power state, " + "currently in D%d\n", dev->current_state); /* According to section 5.4.1 of the "PCI BUS POWER MANAGEMENT * INTERFACE SPECIFICATION, REV. 1.2", a device transitioning