From mboxrd@z Thu Jan  1 00:00:00 1970
From: Bruce Richardson <bruce.richardson@intel.com>
Subject: Re: [PATCH v5 11/12] pci: implement hotplug bus operation
Date: Tue, 27 Jun 2017 14:49:15 +0100
Message-ID: <20170627134915.GG104744@bricha3-MOBL3.ger.corp.intel.com>
References: <cover.1498436062.git.gaetan.rivet@6wind.com>
 <98e3e67ce4b581325398aa6167e4a9a3ab20a6e8.1498436062.git.gaetan.rivet@6wind.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Cc: dev@dpdk.org
To: Gaetan Rivet <gaetan.rivet@6wind.com>
Return-path: <dev-bounces@dpdk.org>
Received: from mga14.intel.com (mga14.intel.com [192.55.52.115])
 by dpdk.org (Postfix) with ESMTP id 015732C6A
 for <dev@dpdk.org>; Tue, 27 Jun 2017 15:49:18 +0200 (CEST)
Content-Disposition: inline
In-Reply-To: <98e3e67ce4b581325398aa6167e4a9a3ab20a6e8.1498436062.git.gaetan.rivet@6wind.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

On Mon, Jun 26, 2017 at 02:22:09AM +0200, Gaetan Rivet wrote:
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>  lib/librte_eal/common/eal_common_pci.c | 44 ++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
> 
> diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
> index 00d48d9..286357d 100644
> --- a/lib/librte_eal/common/eal_common_pci.c
> +++ b/lib/librte_eal/common/eal_common_pci.c
> @@ -47,6 +47,7 @@
>  #include <rte_pci.h>
>  #include <rte_per_lcore.h>
>  #include <rte_memory.h>
> +#include <rte_memcpy.h>
>  #include <rte_memzone.h>
>  #include <rte_eal.h>
>  #include <rte_string_fns.h>
> @@ -500,11 +501,54 @@ pci_find_device(rte_dev_cmp_t cmp, const void *data)
>  	return NULL;
>  }
>  
> +static struct rte_device *
> +pci_plug(struct rte_devargs *da)
> +{
> +	struct rte_pci_device *pdev;
> +	struct rte_pci_addr *addr;
> +
> +	addr = &da->pci.addr;
> +	/*
> +	 * Update eventual pci device in global list.
> +	 * Insert it if none was found.
> +	 */
> +	if (pci_update_device(addr) < 0) {
> +		rte_errno = EIO;
> +		return NULL;
> +	}
> +	/* Find the current device holding this address in the bus. */
> +	FOREACH_DEVICE_ON_PCIBUS(pdev) {
> +		if (rte_eal_compare_pci_addr(&pdev->addr, addr) == 0) {
> +			if (rte_pci_probe_one(addr)) {

Please put the != 0, or == -1 in the condition, to make it clear it's an
error leg.

> +				rte_errno = ENODEV;
> +				return NULL;
> +			}
> +			break;
> +		}
> +	}
> +	return pdev ? &pdev->device : NULL;
> +}

Please put in explicit != NULL, as per coding standards here.

> +
> +static int
> +pci_unplug(struct rte_device *dev)
> +{
> +	struct rte_pci_device *pdev;
> +
> +	pdev = RTE_DEV_TO_PCI(dev);
> +	if (rte_pci_detach(&pdev->addr)) {

As above, please check for == or != some value.

> +		rte_errno = ENODEV;
> +		return -1;
> +	}
> +	return 0;
> +}
> +
>  struct rte_pci_bus rte_pci_bus = {
>  	.bus = {
>  		.scan = rte_pci_scan,
>  		.probe = rte_pci_probe,
>  		.find_device = pci_find_device,
> +		.plug = pci_plug,
> +		.unplug = pci_unplug,
>  	},
>  	.device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list),
>  	.driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
> -- 
> 2.1.4
> 
With above fixes,

Acked-by: Bruce Richardson <bruce.richardson@intel.com>