| Message ID | 1373535825-49972-3-git-send-email-wangyijing@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Thu, 2013-07-11 at 17:43 +0800, Yijing Wang wrote: > Currently, pciehp_resume() try to hot add device if the slot adapter > status return true. But if there are already some devices exist, > namely list_empty(bus->devices) return false. We should not add the device > again, because the device add action will fail. Also print some uncomfortable > messages like this: > pciehp 0000:00:1c.1:pcie04: Device 0000:03:00.0 already exists at 0000:03:00, cannot hot-add > pciehp 0000:00:1c.1:pcie04: Cannot add device at 0000:03:00 > > Signed-off-by: Yijing Wang <wangyijing@huawei.com> > Cc: Paul Bolle <pebolle@tiscali.nl> > Cc: "Rafael J. Wysocki" <rjw@sisk.pl> > Cc: Oliver Neukum <oneukum@suse.de> > Cc: Gu Zheng <guz.fnst@cn.fujitsu.com> > Cc: linux-pci@vger.kernel.org > --- > drivers/pci/hotplug/pciehp_core.c | 9 ++++++--- > 1 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c > index 7d72c5e..1542735 100644 > --- a/drivers/pci/hotplug/pciehp_core.c > +++ b/drivers/pci/hotplug/pciehp_core.c [...] > @@ -311,10 +312,12 @@ static int pciehp_resume (struct pcie_device *dev) > > /* Check if slot is occupied */ > pciehp_get_adapter_status(slot, &status); > - if (status) > - pciehp_enable_slot(slot); > - else > + if (status) { > + if (list_empty(&pbus->devices)) > + pciehp_enable_slot(slot); > + } else if (!list_empty(&pbus->devices)) > pciehp_disable_slot(slot); > + Coding style: braces for the "else if" branch too? Or change the first test to "if (status && list_empty([...]))" and drop the braces? > return 0; > } > #endif /* PM */ Paul Bolle -- 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
>> --- >> drivers/pci/hotplug/pciehp_core.c | 9 ++++++--- >> 1 files changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c >> index 7d72c5e..1542735 100644 >> --- a/drivers/pci/hotplug/pciehp_core.c >> +++ b/drivers/pci/hotplug/pciehp_core.c > [...] >> @@ -311,10 +312,12 @@ static int pciehp_resume (struct pcie_device *dev) >> >> /* Check if slot is occupied */ >> pciehp_get_adapter_status(slot, &status); >> - if (status) >> - pciehp_enable_slot(slot); >> - else >> + if (status) { >> + if (list_empty(&pbus->devices)) >> + pciehp_enable_slot(slot); >> + } else if (!list_empty(&pbus->devices)) >> pciehp_disable_slot(slot); >> + > > Coding style: braces for the "else if" branch too? Or change the first > test to "if (status && list_empty([...]))" and drop the braces? Hmmm, I will add the braces for "else if " Change the first test "if (status && list_empty([...]))" is not a good idea, because this change will modify the code logic, for example if a device was present before suspend and still there during resume, we should do nothing, but after the logic change, we may disable it. > >> return 0; >> } >> #endif /* PM */ > > > Paul Bolle > > > . >
diff --git a/drivers/pci/hotplug/pciehp_core.c b/drivers/pci/hotplug/pciehp_core.c index 7d72c5e..1542735 100644 --- a/drivers/pci/hotplug/pciehp_core.c +++ b/drivers/pci/hotplug/pciehp_core.c @@ -300,6 +300,7 @@ static int pciehp_resume (struct pcie_device *dev) { struct controller *ctrl; struct slot *slot; + struct pci_bus *pbus = dev->port->subordinate; u8 status; ctrl = get_service_data(dev); @@ -311,10 +312,12 @@ static int pciehp_resume (struct pcie_device *dev) /* Check if slot is occupied */ pciehp_get_adapter_status(slot, &status); - if (status) - pciehp_enable_slot(slot); - else + if (status) { + if (list_empty(&pbus->devices)) + pciehp_enable_slot(slot); + } else if (!list_empty(&pbus->devices)) pciehp_disable_slot(slot); + return 0; } #endif /* PM */
Currently, pciehp_resume() try to hot add device if the slot adapter status return true. But if there are already some devices exist, namely list_empty(bus->devices) return false. We should not add the device again, because the device add action will fail. Also print some uncomfortable messages like this: pciehp 0000:00:1c.1:pcie04: Device 0000:03:00.0 already exists at 0000:03:00, cannot hot-add pciehp 0000:00:1c.1:pcie04: Cannot add device at 0000:03:00 Signed-off-by: Yijing Wang <wangyijing@huawei.com> Cc: Paul Bolle <pebolle@tiscali.nl> Cc: "Rafael J. Wysocki" <rjw@sisk.pl> Cc: Oliver Neukum <oneukum@suse.de> Cc: Gu Zheng <guz.fnst@cn.fujitsu.com> Cc: linux-pci@vger.kernel.org --- drivers/pci/hotplug/pciehp_core.c | 9 ++++++--- 1 files changed, 6 insertions(+), 3 deletions(-)