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

From: Bjorn Helgaas <bhelgaas@google.com>
To: Yijing Wang <wangyijing@huawei.com>
Cc: "linux-pci@vger.kernel.org" <linux-pci@vger.kernel.org>,
	Tony Luck <tony.luck@intel.com>,
	Hanjun Guo <guohanjun@huawei.com>,
	Jiang Liu <jiang.liu@huawei.com>,
	Kenji Kaneshige <kaneshige.kenji@jp.fujitsu.com>,
	Shengzhou Liu <Shengzhou.Liu@freescale.com>,
	"Rafael J. Wysocki" <rjw@sisk.pl>,
	Huang Ying <ying.huang@intel.com>
Subject: Re: [PATCH -v2 1/2] PCI: decrease pci_dev->enable_cnt when no pcie capability found
Date: Mon, 15 Apr 2013 09:27:21 -0600	[thread overview]
Message-ID: <CAErSpo7dgFR8a7Wee4CE7Rj+nZg_-+8uNOFq__d8EzY+gHQHGQ@mail.gmail.com> (raw)
In-Reply-To: <1366009135-19088-1-git-send-email-wangyijing@huawei.com>

[+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.

Bjorn