| Message ID | 9bc855a01d0f29cc8425cf50dce9812062309cfb.1473135221.git.lukas@wunner.de (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Tue, Sep 06, 2016 at 06:20:46AM +0200, Lukas Wunner wrote: > Starting with v4.8, we allow a PCIe port to runtime suspend to D3hot if > the port itself and its children satisfy a number of conditions. Once a > child is removed, we recheck those conditions in case the removed device > was blocking the port from suspending. > > The rechecking needs to happen *after* the device has been removed from > the bus it resides on. Otherwise when walking the port's subordinate > bus in pci_bridge_d3_update(), the device being removed would > erroneously still be taken into account. > > However the device is removed from the bus_list in pci_destroy_dev() and > we currently recheck *before* that. Fix it. Yes, makes sense. > Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> -- 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 Wednesday, September 07, 2016 06:47:19 PM Mika Westerberg wrote: > On Tue, Sep 06, 2016 at 06:20:46AM +0200, Lukas Wunner wrote: > > Starting with v4.8, we allow a PCIe port to runtime suspend to D3hot if > > the port itself and its children satisfy a number of conditions. Once a > > child is removed, we recheck those conditions in case the removed device > > was blocking the port from suspending. > > > > The rechecking needs to happen *after* the device has been removed from > > the bus it resides on. Otherwise when walking the port's subordinate > > bus in pci_bridge_d3_update(), the device being removed would > > erroneously still be taken into account. > > > > However the device is removed from the bus_list in pci_destroy_dev() and > > we currently recheck *before* that. Fix it. > > Yes, makes sense. > > > Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > > Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com> Acked-by: Rafael J. Wysocki <mika.westerberg@linux.intel.com> Thanks! -- 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 Tue, Sep 06, 2016 at 06:20:46AM +0200, Lukas Wunner wrote: > Starting with v4.8, we allow a PCIe port to runtime suspend to D3hot if > the port itself and its children satisfy a number of conditions. Once a > child is removed, we recheck those conditions in case the removed device > was blocking the port from suspending. > > The rechecking needs to happen *after* the device has been removed from > the bus it resides on. Otherwise when walking the port's subordinate > bus in pci_bridge_d3_update(), the device being removed would > erroneously still be taken into account. > > However the device is removed from the bus_list in pci_destroy_dev() and > we currently recheck *before* that. Fix it. > > Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > Signed-off-by: Lukas Wunner <lukas@wunner.de> 9d26d3a8f1b0 appeared in v4.8-rc1, so I assume this fix should be merged before v4.8, right? I applied it to for-linus with Mika's reviewed-by and Rafael's acked-by on that assumption. > --- > drivers/pci/remove.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c > index d1ef7ac..f9357e0 100644 > --- a/drivers/pci/remove.c > +++ b/drivers/pci/remove.c > @@ -40,6 +40,7 @@ static void pci_destroy_dev(struct pci_dev *dev) > list_del(&dev->bus_list); > up_write(&pci_bus_sem); > > + pci_bridge_d3_device_removed(dev); > pci_free_resources(dev); > put_device(&dev->dev); > } > @@ -96,8 +97,6 @@ static void pci_remove_bus_device(struct pci_dev *dev) > dev->subordinate = NULL; > } > > - pci_bridge_d3_device_removed(dev); > - > pci_destroy_dev(dev); > } > > -- > 2.9.3 > > -- > 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 -- 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 9/13/2016 11:02 PM, Bjorn Helgaas wrote: > On Tue, Sep 06, 2016 at 06:20:46AM +0200, Lukas Wunner wrote: >> Starting with v4.8, we allow a PCIe port to runtime suspend to D3hot if >> the port itself and its children satisfy a number of conditions. Once a >> child is removed, we recheck those conditions in case the removed device >> was blocking the port from suspending. >> >> The rechecking needs to happen *after* the device has been removed from >> the bus it resides on. Otherwise when walking the port's subordinate >> bus in pci_bridge_d3_update(), the device being removed would >> erroneously still be taken into account. >> >> However the device is removed from the bus_list in pci_destroy_dev() and >> we currently recheck *before* that. Fix it. >> >> Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") >> Cc: Mika Westerberg <mika.westerberg@linux.intel.com> >> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> >> Signed-off-by: Lukas Wunner <lukas@wunner.de> > 9d26d3a8f1b0 appeared in v4.8-rc1, so I assume this fix should be merged > before v4.8, right? Yup. > I applied it to for-linus with Mika's reviewed-by and Rafael's acked-by on > that assumption. Cool, thanks! -- 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 Tue, Sep 13, 2016 at 04:02:31PM -0500, Bjorn Helgaas wrote: > On Tue, Sep 06, 2016 at 06:20:46AM +0200, Lukas Wunner wrote: > > Starting with v4.8, we allow a PCIe port to runtime suspend to D3hot if > > the port itself and its children satisfy a number of conditions. Once a > > child is removed, we recheck those conditions in case the removed device > > was blocking the port from suspending. > > > > The rechecking needs to happen *after* the device has been removed from > > the bus it resides on. Otherwise when walking the port's subordinate > > bus in pci_bridge_d3_update(), the device being removed would > > erroneously still be taken into account. > > > > However the device is removed from the bus_list in pci_destroy_dev() and > > we currently recheck *before* that. Fix it. > > > > Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") > > Cc: Mika Westerberg <mika.westerberg@linux.intel.com> > > Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> > > Signed-off-by: Lukas Wunner <lukas@wunner.de> > > 9d26d3a8f1b0 appeared in v4.8-rc1, so I assume this fix should be merged > before v4.8, right? Yes, it's a bug in the new runtime PM code for PCIe ports, albeit a relatively harmless one: I think the worst that can happen is that a port which should suspend doesn't. But it's still a bug, so if this could still go into 4.8 it would be ideal. > I applied it to for-linus with Mika's reviewed-by and Rafael's acked-by on > that assumption. Perfect, thanks! Lukas -- 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
diff --git a/drivers/pci/remove.c b/drivers/pci/remove.c index d1ef7ac..f9357e0 100644 --- a/drivers/pci/remove.c +++ b/drivers/pci/remove.c @@ -40,6 +40,7 @@ static void pci_destroy_dev(struct pci_dev *dev) list_del(&dev->bus_list); up_write(&pci_bus_sem); + pci_bridge_d3_device_removed(dev); pci_free_resources(dev); put_device(&dev->dev); } @@ -96,8 +97,6 @@ static void pci_remove_bus_device(struct pci_dev *dev) dev->subordinate = NULL; } - pci_bridge_d3_device_removed(dev); - pci_destroy_dev(dev); }
Starting with v4.8, we allow a PCIe port to runtime suspend to D3hot if the port itself and its children satisfy a number of conditions. Once a child is removed, we recheck those conditions in case the removed device was blocking the port from suspending. The rechecking needs to happen *after* the device has been removed from the bus it resides on. Otherwise when walking the port's subordinate bus in pci_bridge_d3_update(), the device being removed would erroneously still be taken into account. However the device is removed from the bus_list in pci_destroy_dev() and we currently recheck *before* that. Fix it. Fixes: 9d26d3a8f1b0 ("PCI: Put PCIe ports into D3 during suspend") Cc: Mika Westerberg <mika.westerberg@linux.intel.com> Cc: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Signed-off-by: Lukas Wunner <lukas@wunner.de> --- drivers/pci/remove.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)