Re: [PATCH -v2 1/2] PCI: decrease pci_dev->enable_cnt when no pcie capability found

[Yijing Wang <wangyijing@huawei.com>]

On 2013/4/16 16:22, huang ying wrote:
> On Mon, Apr 15, 2013 at 11:27 PM, Bjorn Helgaas <bhelgaas@google.com> wrote:
>> [+cc Rafael, Huang]
>>
>> On Mon, Apr 15, 2013 at 12:58 AM, Yijing Wang <wangyijing@huawei.com> wrote:
>>> We should decrease dev->enable_cnt when no pcie port capability
>>> found for balance.
>>>
>>> Signed-off-by: Yijing Wang <wangyijing@huawei.com>
>>> Cc: Jiang Liu <jiang.liu@huawei.com>
>>> Cc: Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>
>>> Cc: Shengzhou Liu <Shengzhou.Liu@freescale.com>
>>> ---
>>>  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.

Hi Huang Ying,
   Thanks for your comments! I will drop this patch, because as you said pcie port
device need pcie port driver regardless any pcie capabilities found.

But there is still a problem about two pci_disable_device() called in pcie_portdrv_remove().
In this case, If we unbind pcie port driver, the pcie port device will be disabled. The child devices
under it cannot use anymore. I send this patch in another reply.

Thanks!
Yijing

> 
> Best Regards,
> Huang Ying
> 
> .
> 


-- 
Thanks!
Yijing