Hi Danny,
I wanted to apply this patchset which was reviewed. But when having a quick
overview, I've seen some strange additions.
2015-01-29 17:28, Danny Zhou:
> 1) Unify procedure to retrieve BAR resource mapping information.
> 2) Setup bus master bit in NIC's PCIe configuration space for uio_pci_generic.
>
> Signed-off-by: Danny Zhou <danny.zhou at intel.com>
> Tested-by: Qun Wan <qun.wan at intel.com>
[...]
> --- a/lib/librte_eal/common/include/rte_pci.h
> +++ b/lib/librte_eal/common/include/rte_pci.h
> @@ -148,6 +148,7 @@ struct rte_pci_device {
> struct rte_pci_id id; /**< PCI ID. */
> struct rte_pci_resource mem_resource[PCI_MAX_RESOURCE]; /**< PCI
> Memory Resource */
> struct rte_intr_handle intr_handle; /**< Interrupt handle */
> + char driver_name[BUFSIZ]; /**< driver name */
Why not embedding this field in driver struct?
The name and comment should be more precise.
There is also driver->name and hotplug patchset is adding a kernel driver name.
Please bring the light in all these driver names :)
> const struct rte_pci_driver *driver; /**< Associated driver */
[...]
> --- a/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> +++ b/lib/librte_eal/linuxapp/eal/eal_pci_uio.c
> +#define IORESOURCE_MEM 0x00000200
Please comment this value.
> --- a/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> +++ b/lib/librte_eal/linuxapp/eal/include/exec-env/rte_interrupts.h
> @@ -50,8 +50,14 @@ enum rte_intr_handle_type {
>
> /** Handle for interrupts. */
> struct rte_intr_handle {
> - int vfio_dev_fd; /**< VFIO device file descriptor */
> - int fd; /**< file descriptor */
> + union {
> + int vfio_dev_fd; /**< VFIO device file descriptor */
> + };
> + union {
> + int uio_cfg_fd; /**< UIO config file descriptor
> + for uio_pci_generic */
> + };
Apart the indent, it seems there is a mistake here.
Why 2 unions with 1 field each?
> + int fd; /**< interrupt event file descriptor */
> enum rte_intr_handle_type type; /**< handle type */
> };
