On Mon, Apr 15, 2013 at 11:27 PM, Bjorn Helgaas <bhelgaas@xxxxxxxxxx> wrote: > [+cc Rafael, Huang] > > On Mon, Apr 15, 2013 at 12:58 AM, Yijing Wang <wangyijing@xxxxxxxxxx> wrote: >> We should decrease dev->enable_cnt when no pcie port capability >> found for balance. >> >> Signed-off-by: Yijing Wang <wangyijing@xxxxxxxxxx> >> Cc: Jiang Liu <jiang.liu@xxxxxxxxxx> >> Cc: Kenji Kaneshige <kaneshige.kenji@xxxxxxxxxxxxxx> >> Cc: Shengzhou Liu <Shengzhou.Liu@xxxxxxxxxxxxx> >> --- >> drivers/pci/pcie/portdrv_core.c | 4 ++-- >> 1 files changed, 2 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/pci/pcie/portdrv_core.c b/drivers/pci/pcie/portdrv_core.c >> index 31063ac..aef3fac 100644 >> --- a/drivers/pci/pcie/portdrv_core.c >> +++ b/drivers/pci/pcie/portdrv_core.c >> @@ -369,8 +369,8 @@ int pcie_port_device_register(struct pci_dev *dev) >> >> /* Get and check PCI Express port services */ >> capabilities = get_port_device_capability(dev); >> - if (!capabilities) >> - return 0; >> + if (!capabilities) >> + goto error_disable; >> >> pci_set_master(dev); >> /* > > Does this fix a problem you observed? If so, please refer to it in > your changelog. > > I think this patch is incorrect because pcie_portdrv_probe() will > return 0 (success) with the device disabled. When we call > pcie_portdrv_remove(), we will attempt to disable the device again, > even though it's already disabled. > > I don't know whether it is desirable for pcie_portdrv_probe() to > succeed when no capabilities are available or not. Maybe somebody > else has an opinion. I think this patchset is incorrect too. Even if there is no PCIe services for the PCIe port. We still need the PCIe drivers, at least for runtime power management. Although runtime power management is disabled for the PCIe port now, I think we will enable it again after we fixed the corresponding issues. Best Regards, Huang Ying -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
