On 9/29/20 4:17 PM, Chautru, Nicolas wrote:
> Hi Tom,
>
>> -----Original Message-----
>> From: Tom Rix <t...@redhat.com>
>> Sent: Tuesday, September 29, 2020 12:54 PM
>> To: Chautru, Nicolas <nicolas.chau...@intel.com>; dev@dpdk.org;
>> akhil.go...@nxp.com
>> Cc: Richardson, Bruce <bruce.richard...@intel.com>; Xu, Rosen
>> <rosen...@intel.com>; dave.bur...@accelercomm.com;
>> aidan.godd...@accelercomm.com; Yigit, Ferruh <ferruh.yi...@intel.com>;
>> Liu, Tianjiao <tianjiao....@intel.com>
>> Subject: Re: [dpdk-dev] [PATCH v9 01/10] drivers/baseband: add PMD for
>> ACC100
>>
>>
>> On 9/28/20 5:29 PM, Nicolas Chautru wrote:
>>> Add stubs for the ACC100 PMD
>>>
>>> Signed-off-by: Nicolas Chautru <nicolas.chau...@intel.com>
>>> Acked-by: Liu Tianjiao <tianjiao....@intel.com>
>>> ---
>>> doc/guides/bbdevs/acc100.rst | 233
>>> +++++++++++++++++++++
>>> doc/guides/bbdevs/features/acc100.ini | 14 ++
>>> doc/guides/bbdevs/index.rst | 1 +
>>> drivers/baseband/acc100/meson.build | 6 +
>>> drivers/baseband/acc100/rte_acc100_pmd.c | 175
>> ++++++++++++++++
>>> drivers/baseband/acc100/rte_acc100_pmd.h | 37 ++++
>>> .../acc100/rte_pmd_bbdev_acc100_version.map | 3 +
>>> drivers/baseband/meson.build | 2 +-
>>> 8 files changed, 470 insertions(+), 1 deletion(-) create mode 100644
>>> doc/guides/bbdevs/acc100.rst create mode 100644
>>> doc/guides/bbdevs/features/acc100.ini
>>> create mode 100644 drivers/baseband/acc100/meson.build
>>> create mode 100644 drivers/baseband/acc100/rte_acc100_pmd.c
>>> create mode 100644 drivers/baseband/acc100/rte_acc100_pmd.h
>>> create mode 100644
>>> drivers/baseband/acc100/rte_pmd_bbdev_acc100_version.map
>>>
>>> diff --git a/doc/guides/bbdevs/acc100.rst
>>> b/doc/guides/bbdevs/acc100.rst new file mode 100644 index
>>> 0000000..f87ee09
>>> --- /dev/null
>>> +++ b/doc/guides/bbdevs/acc100.rst
>>> @@ -0,0 +1,233 @@
>>> +.. SPDX-License-Identifier: BSD-3-Clause
>>> + Copyright(c) 2020 Intel Corporation
>>> +
>>> +Intel(R) ACC100 5G/4G FEC Poll Mode Driver
>>> +==========================================
>>> +
>>> +The BBDEV ACC100 5G/4G FEC poll mode driver (PMD) supports an
>>> +implementation of a VRAN FEC wireless acceleration function.
>>> +This device is also known as Mount Bryce.
>> If this is code name or general chip name it should be removed.
> We have used general chip name for other PMDs (ie. Vista Creek), I can remove
> but
> why should this be removed for my benefit? This tends to be the most user
> friendly
> name so arguablygood to name drop in documentation .
VistaCreek is the code name, the chip would be aria10.
Since mt bryce is the chip name, after more than 1 eASIC this becomes confusing.
Generally public product names should be used because only the early developers
will know the development code names.
>
>
>>> +
>>> +Features
>>> +--------
>>> +
>>> +ACC100 5G/4G FEC PMD supports the following features:
>>> +
>>> +- LDPC Encode in the DL (5GNR)
>>> +- LDPC Decode in the UL (5GNR)
>>> +- Turbo Encode in the DL (4G)
>>> +- Turbo Decode in the UL (4G)
>>> +- 16 VFs per PF (physical device)
>>> +- Maximum of 128 queues per VF
>>> +- PCIe Gen-3 x16 Interface
>>> +- MSI
>>> +- SR-IOV
>>> +
>>> +ACC100 5G/4G FEC PMD supports the following BBDEV capabilities:
>>> +
>>> +* For the LDPC encode operation:
>>> + - ``RTE_BBDEV_LDPC_CRC_24B_ATTACH`` : set to attach CRC24B to
>> CB(s)
>>> + - ``RTE_BBDEV_LDPC_RATE_MATCH`` : if set then do not do Rate Match
>> bypass
>>> + - ``RTE_BBDEV_LDPC_INTERLEAVER_BYPASS`` : if set then bypass
>>> +interleaver
>>> +
>>> +* For the LDPC decode operation:
>>> + - ``RTE_BBDEV_LDPC_CRC_TYPE_24B_CHECK`` : check CRC24B from
>> CB(s)
>>> + - ``RTE_BBDEV_LDPC_ITERATION_STOP_ENABLE`` : disable early
>> termination
>>> + - ``RTE_BBDEV_LDPC_CRC_TYPE_24B_DROP`` : drops CRC24B bits
>> appended while decoding
>>> + - ``RTE_BBDEV_LDPC_HQ_COMBINE_IN_ENABLE`` : provides an input
>> for HARQ combining
>>> + - ``RTE_BBDEV_LDPC_HQ_COMBINE_OUT_ENABLE`` : provides an input
>> for HARQ combining
>>> + - ``RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_IN_ENABLE`` : HARQ
>> memory input is internal
>>> + - ``RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_OUT_ENABLE`` :
>> HARQ memory output is internal
>>> + - ``RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_LOOPBACK`` :
>> loopback data to/from HARQ memory
>>> + - ``RTE_BBDEV_LDPC_INTERNAL_HARQ_MEMORY_FILLERS`` : HARQ
>> memory includes the fillers bits
>>> + - ``RTE_BBDEV_LDPC_DEC_SCATTER_GATHER`` : supports scatter-
>> gather for input/output data
>>> + - ``RTE_BBDEV_LDPC_HARQ_6BIT_COMPRESSION`` : supports
>> compression of the HARQ input/output
>>> + - ``RTE_BBDEV_LDPC_LLR_COMPRESSION`` : supports LLR input
>>> +compression
>>> +
>>> +* For the turbo encode operation:
>>> + - ``RTE_BBDEV_TURBO_CRC_24B_ATTACH`` : set to attach CRC24B to
>> CB(s)
>>> + - ``RTE_BBDEV_TURBO_RATE_MATCH`` : if set then do not do Rate
>> Match bypass
>>> + - ``RTE_BBDEV_TURBO_ENC_INTERRUPTS`` : set for encoder dequeue
>> interrupts
>>> + - ``RTE_BBDEV_TURBO_RV_INDEX_BYPASS`` : set to bypass RV index
>>> + - ``RTE_BBDEV_TURBO_ENC_SCATTER_GATHER`` : supports
>>> +scatter-gather for input/output data
>>> +
>>> +* For the turbo decode operation:
>>> + - ``RTE_BBDEV_TURBO_CRC_TYPE_24B`` : check CRC24B from CB(s)
>>> + - ``RTE_BBDEV_TURBO_SUBBLOCK_DEINTERLEAVE`` : perform subblock
>> de-interleave
>>> + - ``RTE_BBDEV_TURBO_DEC_INTERRUPTS`` : set for decoder dequeue
>> interrupts
>>> + - ``RTE_BBDEV_TURBO_NEG_LLR_1_BIT_IN`` : set if negative LLR
>> encoder i/p is supported
>>> + - ``RTE_BBDEV_TURBO_POS_LLR_1_BIT_IN`` : set if positive LLR encoder
>> i/p is supported
>>> + - ``RTE_BBDEV_TURBO_DEC_TB_CRC_24B_KEEP`` : keep CRC24B bits
>> appended while decoding
>>> + - ``RTE_BBDEV_TURBO_EARLY_TERMINATION`` : set early early
>> termination feature
>>> + - ``RTE_BBDEV_TURBO_DEC_SCATTER_GATHER`` : supports scatter-
>> gather for input/output data
>>> + - ``RTE_BBDEV_TURBO_HALF_ITERATION_EVEN`` : set half iteration
>>> +granularity
>>> +
>>> +Installation
>>> +------------
>>> +
>>> +Section 3 of the DPDK manual provides instuctions on installing and
>>> +compiling DPDK. The default set of bbdev compile flags may be found
>>> +in config/common_base, where for example the flag to build the ACC100
>>> +5G/4G FEC device, ``CONFIG_RTE_LIBRTE_PMD_BBDEV_ACC100``,
>>> +is already set.
>>> +
>>> +DPDK requires hugepages to be configured as detailed in section 2 of the
>> DPDK manual.
>>> +The bbdev test application has been tested with a configuration 40 x
>>> +1GB hugepages. The hugepage configuration of a server may be examined
>> using:
>>> +
>>> +.. code-block:: console
>>> +
>>> + grep Huge* /proc/meminfo
>>> +
>>> +
>>> +Initialization
>>> +--------------
>>> +
>>> +When the device first powers up, its PCI Physical Functions (PF) can be
>> listed through this command:
>>> +
>>> +.. code-block:: console
>>> +
>>> + sudo lspci -vd8086:0d5c
>>> +
>>> +The physical and virtual functions are compatible with Linux UIO drivers:
>>> +``vfio`` and ``igb_uio``. However, in order to work the ACC100 5G/4G
>>> +FEC device firstly needs to be bound to one of these linux drivers through
>> DPDK.
>> FEC device first
> ok
>
>>> +
>>> +
>>> +Bind PF UIO driver(s)
>>> +~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +Install the DPDK igb_uio driver, bind it with the PF PCI device ID
>>> +and use ``lspci`` to confirm the PF device is under use by ``igb_uio`` DPDK
>> UIO driver.
>>> +
>>> +The igb_uio driver may be bound to the PF PCI device using one of three
>> methods:
>>> +
>>> +
>>> +1. PCI functions (physical or virtual, depending on the use case) can
>>> +be bound to the UIO driver by repeating this command for every function.
>>> +
>>> +.. code-block:: console
>>> +
>>> + cd <dpdk-top-level-directory>
>>> + insmod ./build/kmod/igb_uio.ko
>>> + echo "8086 0d5c" > /sys/bus/pci/drivers/igb_uio/new_id
>>> + lspci -vd8086:0d5c
>>> +
>>> +
>>> +2. Another way to bind PF with DPDK UIO driver is by using the
>>> +``dpdk-devbind.py`` tool
>>> +
>>> +.. code-block:: console
>>> +
>>> + cd <dpdk-top-level-directory>
>>> + ./usertools/dpdk-devbind.py -b igb_uio 0000:06:00.0
>>> +
>>> +where the PCI device ID (example: 0000:06:00.0) is obtained using
>>> +lspci -vd8086:0d5c
>>> +
>>> +
>>> +3. A third way to bind is to use ``dpdk-setup.sh`` tool
>>> +
>>> +.. code-block:: console
>>> +
>>> + cd <dpdk-top-level-directory>
>>> + ./usertools/dpdk-setup.sh
>>> +
>>> + select 'Bind Ethernet/Crypto/Baseband device to IGB UIO module'
>>> + or
>>> + select 'Bind Ethernet/Crypto/Baseband device to VFIO module'
>>> + depending on driver required
>> This is the igb_uio section, should defer vfio select to its section.
> Ok
>
>>> + enter PCI device ID
>>> + select 'Display current Ethernet/Crypto/Baseband device settings'
>>> + to confirm binding
>>> +
>>> +
>>> +In the same way the ACC100 5G/4G FEC PF can be bound with vfio, but
>>> +vfio driver does not support SR-IOV configuration right out of the box, so
>> it will need to be patched.
>> Other documentation says works with 5.7
> Yes this is a bit historical now. I can remove this bit which is not very
> informative and non specific to that PMD.
>
>>> +
>>> +
>>> +Enable Virtual Functions
>>> +~~~~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +Now, it should be visible in the printouts that PCI PF is under
>>> +igb_uio control "``Kernel driver in use: igb_uio``"
>>> +
>>> +To show the number of available VFs on the device, read ``sriov_totalvfs``
>> file..
>>> +
>>> +.. code-block:: console
>>> +
>>> + cat /sys/bus/pci/devices/0000\:<b>\:<d>.<f>/sriov_totalvfs
>>> +
>>> + where 0000\:<b>\:<d>.<f> is the PCI device ID
>>> +
>>> +
>>> +To enable VFs via igb_uio, echo the number of virtual functions
>>> +intended to enable to ``max_vfs`` file..
>>> +
>>> +.. code-block:: console
>>> +
>>> + echo <num-of-vfs> > /sys/bus/pci/devices/0000\:<b>\:<d>.<f>/max_vfs
>>> +
>>> +
>>> +Afterwards, all VFs must be bound to appropriate UIO drivers as
>>> +required, same way it was done with the physical function previously.
>>> +
>>> +Enabling SR-IOV via vfio driver is pretty much the same, except that
>>> +the file name is different:
>>> +
>>> +.. code-block:: console
>>> +
>>> + echo <num-of-vfs> >
>>> + /sys/bus/pci/devices/0000\:<b>\:<d>.<f>/sriov_numvfs
>>> +
>>> +
>>> +Configure the VFs through PF
>>> +~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>> +
>>> +The PCI virtual functions must be configured before working or
>>> +getting assigned to VMs/Containers. The configuration involves
>>> +allocating the number of hardware queues, priorities, load balance,
>>> +bandwidth and other settings necessary for the device to perform FEC
>> functions.
>>> +
>>> +This configuration needs to be executed at least once after reboot or
>>> +PCI FLR and can be achieved by using the function
>>> +``acc100_configure()``, which sets up the parameters defined in
>> ``acc100_conf`` structure.
>>> +
>>> +Test Application
>>> +----------------
>>> +
>>> +BBDEV provides a test application, ``test-bbdev.py`` and range of
>>> +test data for testing the functionality of ACC100 5G/4G FEC encode
>>> +and decode, depending on the device's capabilities. The test
>>> +application is located under app->test-bbdev folder and has the following
>> options:
>>> +
>>> +.. code-block:: console
>>> +
>>> + "-p", "--testapp-path": specifies path to the bbdev test app.
>>> + "-e", "--eal-params" : EAL arguments which are passed to the test
>> app.
>>> + "-t", "--timeout" : Timeout in seconds (default=300).
>>> + "-c", "--test-cases" : Defines test cases to run. Run all if not
>>> specified.
>>> + "-v", "--test-vector" : Test vector path (default=dpdk_path+/app/test-
>> bbdev/test_vectors/bbdev_null.data).
>>> + "-n", "--num-ops" : Number of operations to process on device
>> (default=32).
>>> + "-b", "--burst-size" : Operations enqueue/dequeue burst size
>> (default=32).
>>> + "-s", "--snr" : SNR in dB used when generating LLRs for bler
>>> tests.
>>> + "-s", "--iter_max" : Number of iterations for LDPC decoder.
>>> + "-l", "--num-lcores" : Number of lcores to run (default=16).
>>> + "-i", "--init-device" : Initialise PF device with default values.
>>> +
>>> +
>>> +To execute the test application tool using simple decode or encode
>>> +data, type one of the following:
>>> +
>>> +.. code-block:: console
>>> +
>>> + ./test-bbdev.py -c validation -n 64 -b 1 -v ./ldpc_dec_default.data
>>> + ./test-bbdev.py -c validation -n 64 -b 1 -v ./ldpc_enc_default.data
>>> +
>>> +
>>> +The test application ``test-bbdev.py``, supports the ability to
>>> +configure the PF device with a default set of values, if the "-i" or
>>> +"- -init-device" option is included. The default values are defined in
>> test_bbdev_perf.c.
>>> +
>>> +
>>> +Test Vectors
>>> +~~~~~~~~~~~~
>>> +
>>> +In addition to the simple LDPC decoder and LDPC encoder tests, bbdev
>>> +also provides a range of additional tests under the test_vectors
>>> +folder, which may be useful. The results of these tests will depend
>>> +on the ACC100 5G/4G FEC capabilities which may cause some testcases to
>> be skipped, but no failure should be reported.
>>
>> Just
>>
>> to be skipped.
>>
>> should be able to assume skipped test do not get reported as failures.
> Not necessaraly that obvious from feedback. It doesn't hurt to be explicit and
> this statement is common to all PMDs.
>
ok
>>> diff --git a/doc/guides/bbdevs/features/acc100.ini
>>> b/doc/guides/bbdevs/features/acc100.ini
>>> new file mode 100644
>>> index 0000000..c89a4d7
>>> --- /dev/null
>>> +++ b/doc/guides/bbdevs/features/acc100.ini
>>> @@ -0,0 +1,14 @@
>>> +;
>>> +; Supported features of the 'acc100' bbdev driver.
>>> +;
>>> +; Refer to default.ini for the full list of available PMD features.
>>> +;
>>> +[Features]
>>> +Turbo Decoder (4G) = N
>>> +Turbo Encoder (4G) = N
>>> +LDPC Decoder (5G) = N
>>> +LDPC Encoder (5G) = N
>>> +LLR/HARQ Compression = N
>>> +External DDR Access = N
>>> +HW Accelerated = Y
>>> +BBDEV API = Y
>>> diff --git a/doc/guides/bbdevs/index.rst b/doc/guides/bbdevs/index.rst
>>> index a8092dd..4445cbd 100644
>>> --- a/doc/guides/bbdevs/index.rst
>>> +++ b/doc/guides/bbdevs/index.rst
>>> @@ -13,3 +13,4 @@ Baseband Device Drivers
>>> turbo_sw
>>> fpga_lte_fec
>>> fpga_5gnr_fec
>>> + acc100
>>> diff --git a/drivers/baseband/acc100/meson.build
>>> b/drivers/baseband/acc100/meson.build
>>> new file mode 100644
>>> index 0000000..8afafc2
>>> --- /dev/null
>>> +++ b/drivers/baseband/acc100/meson.build
>>> @@ -0,0 +1,6 @@
>>> +# SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2020 Intel
>>> +Corporation
>>> +
>>> +deps += ['bbdev', 'bus_vdev', 'ring', 'pci', 'bus_pci']
>>> +
>>> +sources = files('rte_acc100_pmd.c')
>>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.c
>>> b/drivers/baseband/acc100/rte_acc100_pmd.c
>>> new file mode 100644
>>> index 0000000..1b4cd13
>>> --- /dev/null
>>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.c
>>> @@ -0,0 +1,175 @@
>>> +/* SPDX-License-Identifier: BSD-3-Clause
>>> + * Copyright(c) 2020 Intel Corporation */
>>> +
>>> +#include <unistd.h>
>>> +
>>> +#include <rte_common.h>
>>> +#include <rte_log.h>
>>> +#include <rte_dev.h>
>>> +#include <rte_malloc.h>
>>> +#include <rte_mempool.h>
>>> +#include <rte_byteorder.h>
>>> +#include <rte_errno.h>
>>> +#include <rte_branch_prediction.h>
>>> +#include <rte_hexdump.h>
>>> +#include <rte_pci.h>
>>> +#include <rte_bus_pci.h>
>>> +
>>> +#include <rte_bbdev.h>
>>> +#include <rte_bbdev_pmd.h>
>> Should these #includes' be in alpha order ?
> Interesting comment. Is this a coding guide line for DPDK or others?
> I have never heard of this personnally, what is the rational?
Not sure if this is dpdk style, i know some other project do this.
This works for self consistent headers, no idea if dpdk are.
don't bother with this.
>>> +#include "rte_acc100_pmd.h"
>>> +
>>> +#ifdef RTE_LIBRTE_BBDEV_DEBUG
>>> +RTE_LOG_REGISTER(acc100_logtype, pmd.bb.acc100, DEBUG); #else
>>> +RTE_LOG_REGISTER(acc100_logtype, pmd.bb.acc100, NOTICE); #endif
>>> +
>>> +/* Free 64MB memory used for software rings */ static int
>>> +acc100_dev_close(struct rte_bbdev *dev __rte_unused) {
>>> + return 0;
>>> +}
>>> +
>>> +static const struct rte_bbdev_ops acc100_bbdev_ops = {
>>> + .close = acc100_dev_close,
>>> +};
>>> +
>>> +/* ACC100 PCI PF address map */
>>> +static struct rte_pci_id pci_id_acc100_pf_map[] = {
>>> + {
>>> + RTE_PCI_DEVICE(RTE_ACC100_VENDOR_ID,
>> RTE_ACC100_PF_DEVICE_ID)
>>> + },
>>> + {.device_id = 0},
>>> +};
>>> +
>>> +/* ACC100 PCI VF address map */
>>> +static struct rte_pci_id pci_id_acc100_vf_map[] = {
>>> + {
>>> + RTE_PCI_DEVICE(RTE_ACC100_VENDOR_ID,
>> RTE_ACC100_VF_DEVICE_ID)
>>> + },
>>> + {.device_id = 0},
>>> +};
>>> +
>>> +/* Initialization Function */
>>> +static void
>>> +acc100_bbdev_init(struct rte_bbdev *dev, struct rte_pci_driver *drv)
>>> +{
>>> + struct rte_pci_device *pci_dev = RTE_DEV_TO_PCI(dev->device);
>>> +
>>> + dev->dev_ops = &acc100_bbdev_ops;
>>> +
>>> + ((struct acc100_device *) dev->data->dev_private)->pf_device =
>>> + !strcmp(drv->driver.name,
>>> +
>> RTE_STR(ACC100PF_DRIVER_NAME));
>>> + ((struct acc100_device *) dev->data->dev_private)->mmio_base =
>>> + pci_dev->mem_resource[0].addr;
>>> +
>>> + rte_bbdev_log_debug("Init device %s [%s] @ vaddr %p paddr
>> %#"PRIx64"",
>>> + drv->driver.name, dev->data->name,
>>> + (void *)pci_dev->mem_resource[0].addr,
>>> + pci_dev->mem_resource[0].phys_addr);
>>> +}
>>> +
>>> +static int acc100_pci_probe(struct rte_pci_driver *pci_drv,
>>> + struct rte_pci_device *pci_dev)
>>> +{
>>> + struct rte_bbdev *bbdev = NULL;
>>> + char dev_name[RTE_BBDEV_NAME_MAX_LEN];
>>> +
>>> + if (pci_dev == NULL) {
>>> + rte_bbdev_log(ERR, "NULL PCI device");
>>> + return -EINVAL;
>>> + }
>>> +
>>> + rte_pci_device_name(&pci_dev->addr, dev_name,
>> sizeof(dev_name));
>>> +
>>> + /* Allocate memory to be used privately by drivers */
>>> + bbdev = rte_bbdev_allocate(pci_dev->device.name);
>>> + if (bbdev == NULL)
>>> + return -ENODEV;
>>> +
>>> + /* allocate device private memory */
>>> + bbdev->data->dev_private = rte_zmalloc_socket(dev_name,
>>> + sizeof(struct acc100_device), RTE_CACHE_LINE_SIZE,
>>> + pci_dev->device.numa_node);
>>> +
>>> + if (bbdev->data->dev_private == NULL) {
>>> + rte_bbdev_log(CRIT,
>>> + "Allocate of %zu bytes for device \"%s\"
>> failed",
>>> + sizeof(struct acc100_device), dev_name);
>>> + rte_bbdev_release(bbdev);
>>> + return -ENOMEM;
>>> + }
>>> +
>>> + /* Fill HW specific part of device structure */
>>> + bbdev->device = &pci_dev->device;
>>> + bbdev->intr_handle = &pci_dev->intr_handle;
>>> + bbdev->data->socket_id = pci_dev->device.numa_node;
>>> +
>>> + /* Invoke ACC100 device initialization function */
>>> + acc100_bbdev_init(bbdev, pci_drv);
>>> +
>>> + rte_bbdev_log_debug("Initialised bbdev %s (id = %u)",
>>> + dev_name, bbdev->data->dev_id);
>>> + return 0;
>>> +}
>>> +
>>> +static int acc100_pci_remove(struct rte_pci_device *pci_dev) {
>>> + struct rte_bbdev *bbdev;
>>> + int ret;
>>> + uint8_t dev_id;
>>> +
>>> + if (pci_dev == NULL)
>>> + return -EINVAL;
>>> +
>>> + /* Find device */
>>> + bbdev = rte_bbdev_get_named_dev(pci_dev->device.name);
>>> + if (bbdev == NULL) {
>>> + rte_bbdev_log(CRIT,
>>> + "Couldn't find HW dev \"%s\" to uninitialise
>> it",
>>> + pci_dev->device.name);
>>> + return -ENODEV;
>>> + }
>>> + dev_id = bbdev->data->dev_id;
>>> +
>>> + /* free device private memory before close */
>>> + rte_free(bbdev->data->dev_private);
>>> +
>>> + /* Close device */
>>> + ret = rte_bbdev_close(dev_id);
>> Do you want to reorder this close before the rte_free so you could recover
>> from the failure ?
> Given this is done the same way for other PMDs I would not change it as it
> would create a discrepency.
> It could be done in principle as another patch for multiple PMDs to support
> this, but really I don't see a usecase for try to fall back in case there was
> such a speculative aerror.
>
fair enough
Tom
>> Tom
>>
> Thanks
> Nic
>
>
>>> + if (ret < 0)
>>> + rte_bbdev_log(ERR,
>>> + "Device %i failed to close during uninit: %i",
>>> + dev_id, ret);
>>> +
>>> + /* release bbdev from library */
>>> + rte_bbdev_release(bbdev);
>>> +
>>> + rte_bbdev_log_debug("Destroyed bbdev = %u", dev_id);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static struct rte_pci_driver acc100_pci_pf_driver = {
>>> + .probe = acc100_pci_probe,
>>> + .remove = acc100_pci_remove,
>>> + .id_table = pci_id_acc100_pf_map,
>>> + .drv_flags = RTE_PCI_DRV_NEED_MAPPING };
>>> +
>>> +static struct rte_pci_driver acc100_pci_vf_driver = {
>>> + .probe = acc100_pci_probe,
>>> + .remove = acc100_pci_remove,
>>> + .id_table = pci_id_acc100_vf_map,
>>> + .drv_flags = RTE_PCI_DRV_NEED_MAPPING };
>>> +
>>> +RTE_PMD_REGISTER_PCI(ACC100PF_DRIVER_NAME,
>> acc100_pci_pf_driver);
>>> +RTE_PMD_REGISTER_PCI_TABLE(ACC100PF_DRIVER_NAME,
>>> +pci_id_acc100_pf_map);
>> RTE_PMD_REGISTER_PCI(ACC100VF_DRIVER_NAME,
>>> +acc100_pci_vf_driver);
>>> +RTE_PMD_REGISTER_PCI_TABLE(ACC100VF_DRIVER_NAME,
>>> +pci_id_acc100_vf_map);
>>> +
>>> diff --git a/drivers/baseband/acc100/rte_acc100_pmd.h
>>> b/drivers/baseband/acc100/rte_acc100_pmd.h
>>> new file mode 100644
>>> index 0000000..6f46df0
>>> --- /dev/null
>>> +++ b/drivers/baseband/acc100/rte_acc100_pmd.h
>>> @@ -0,0 +1,37 @@
>>> +/* SPDX-License-Identifier: BSD-3-Clause
>>> + * Copyright(c) 2020 Intel Corporation */
>>> +
>>> +#ifndef _RTE_ACC100_PMD_H_
>>> +#define _RTE_ACC100_PMD_H_
>>> +
>>> +/* Helper macro for logging */
>>> +#define rte_bbdev_log(level, fmt, ...) \
>>> + rte_log(RTE_LOG_ ## level, acc100_logtype, fmt "\n", \
>>> + ##__VA_ARGS__)
>>> +
>>> +#ifdef RTE_LIBRTE_BBDEV_DEBUG
>>> +#define rte_bbdev_log_debug(fmt, ...) \
>>> + rte_bbdev_log(DEBUG, "acc100_pmd: " fmt, \
>>> + ##__VA_ARGS__)
>>> +#else
>>> +#define rte_bbdev_log_debug(fmt, ...) #endif
>>> +
>>> +/* ACC100 PF and VF driver names */
>>> +#define ACC100PF_DRIVER_NAME intel_acc100_pf
>>> +#define ACC100VF_DRIVER_NAME intel_acc100_vf
>>> +
>>> +/* ACC100 PCI vendor & device IDs */
>>> +#define RTE_ACC100_VENDOR_ID (0x8086)
>>> +#define RTE_ACC100_PF_DEVICE_ID (0x0d5c)
>>> +#define RTE_ACC100_VF_DEVICE_ID (0x0d5d)
>>> +
>>> +/* Private data structure for each ACC100 device */ struct
>>> +acc100_device {
>>> + void *mmio_base; /**< Base address of MMIO registers (BAR0) */
>>> + bool pf_device; /**< True if this is a PF ACC100 device */
>>> + bool configured; /**< True if this ACC100 device is configured */ };
>>> +
>>> +#endif /* _RTE_ACC100_PMD_H_ */
>>> diff --git a/drivers/baseband/acc100/rte_pmd_bbdev_acc100_version.map
>>> b/drivers/baseband/acc100/rte_pmd_bbdev_acc100_version.map
>>> new file mode 100644
>>> index 0000000..4a76d1d
>>> --- /dev/null
>>> +++ b/drivers/baseband/acc100/rte_pmd_bbdev_acc100_version.map
>>> @@ -0,0 +1,3 @@
>>> +DPDK_21 {
>>> + local: *;
>>> +};
>>> diff --git a/drivers/baseband/meson.build
>>> b/drivers/baseband/meson.build index 415b672..72301ce 100644
>>> --- a/drivers/baseband/meson.build
>>> +++ b/drivers/baseband/meson.build
>>> @@ -5,7 +5,7 @@ if is_windows
>>> subdir_done()
>>> endif
>>>
>>> -drivers = ['null', 'turbo_sw', 'fpga_lte_fec', 'fpga_5gnr_fec']
>>> +drivers = ['null', 'turbo_sw', 'fpga_lte_fec', 'fpga_5gnr_fec',
>>> +'acc100']
>>>
>>> config_flag_fmt = 'RTE_LIBRTE_PMD_BBDEV_@0@'
>>> driver_name_fmt = 'rte_pmd_bbdev_@0@'
