[dpdk-dev] [PATCH v1 03/42] net/txgbe: add device init and uninit
Ferruh Yigit
ferruh.yigit at intel.com
Wed Sep 9 19:52:20 CEST 2020
On 9/1/2020 12:50 PM, Jiawen Wu wrote:
> Add basic init and uninit function, registers and some macro definitions prepare for hardware infrastructure.
>
> Signed-off-by: Jiawen Wu <jiawenwu at trustnetic.com>
<...>
> +static const struct eth_dev_ops txgbe_eth_dev_ops = {
> + .dev_start = txgbe_dev_start,
> + .dev_stop = txgbe_dev_stop,
> + .dev_close = txgbe_dev_close,
> + .stats_get = txgbe_dev_stats_get,
> + .stats_reset = txgbe_dev_stats_reset,
What do you think adding '.stats_get' & '.stats_reset' when
'txgbe_dev_stats_get()' & 'txgbe_dev_stats_reset()' implemented (patch 27/42).
Same is also for '.dev_start' & '.dev_stop'. (It will work if you drop
'txgbe_dev_stop()' from 'txgbe_dev_close()', and since the device did not start
at first place, it should be OK, it can be added where device start/stop support
added.)
I see you are using empty functions to constract the driver, I think better to
reduce the number of the empty functions as much as possible although sometimes
you may have to add them, you know it better.
<...>
> +++ b/drivers/net/txgbe/txgbe_pf.c
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: BSD-3-Clause
> + * Copyright(c) 2015-2020
> + */
> +
> +#include <stdio.h>
> +#include <errno.h>
> +#include <stdint.h>
> +#include <stdlib.h>
> +#include <unistd.h>
> +#include <stdarg.h>
> +#include <inttypes.h>
> +
> +#include <rte_interrupts.h>
> +#include <rte_log.h>
> +#include <rte_debug.h>
> +#include <rte_eal.h>
> +#include <rte_ether.h>
> +#include <rte_ethdev_driver.h>
> +#include <rte_memcpy.h>
> +#include <rte_malloc.h>
> +#include <rte_random.h>
Similar comment for all new files, if the include list really required or they
are from copy/paste. Can you only keep the includes needed, and add them as they
needed.
<...>
More information about the dev
mailing list