[PATCH v4 1/4] bus/cdx: introduce cdx bus
Thomas Monjalon
thomas at monjalon.net
Wed May 24 13:14:52 CEST 2023
Hello,
If I understand well, it is very specific to AMD devices.
So I suggest adding "AMD" in title and descriptions.
08/05/2023 13:18, Nipun Gupta:
> CDX bus supports multiple type of devices, which can be
> exposed to user-space via vfio-cdx.
>
> vfio-cdx provides the MMIO IO_MEMORY regions as well as the
> DMA interface for the device (IOMMU).
>
> This support aims to enable the DPDK to support the cdx
> devices in user-space using VFIO interface.
>
> Signed-off-by: Nipun Gupta <nipun.gupta at amd.com>
[...]
> +CDX bus driver
Can we name it "AMD CDX bus"?
> +M: Nipun Gupta <nipun.gupta at amd.com>
> +M: Nikhil Agarwal <nikhil.agarwal at amd.com>
> +F: drivers/bus/cdx/
[...]
> +* **Added CDX bus support.**
Here as well and in other places, would be more precise to say "AMD CDX".
> +
> + CDX bus driver has been added to support AMD CDX bus, which operates
> + on FPGA based CDX devices. The CDX devices are memory mapped on system
> + bus for embedded CPUs.
[...]
> +#ifndef _BUS_CDX_DRIVER_H_
> +#define _BUS_CDX_DRIVER_H_
In general I prefer not adding underscores,
as it is not required, and not really private.
[...]
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <limits.h>
> +#include <errno.h>
> +#include <stdint.h>
> +#include <inttypes.h>
I would bet some includes are not needed for this header file.
> +
> +#include <bus_driver.h>
> +#include <dev_driver.h>
> +#include <rte_debug.h>
> +#include <rte_interrupts.h>
> +#include <rte_dev.h>
> +#include <rte_bus.h>
[...]
More information about the dev
mailing list