Re: [PATCH v9 3/9] net/zxdh: add zxdh device pci init implementation

[Ferruh Yigit <ferruh.yigit@amd.com>]

On 11/1/2024 6:21 AM, Junlong Wang wrote:
> Add device pci init implementation,
> to obtain PCI capability and read configuration, etc.
> 
> Signed-off-by: Junlong Wang <wang.junlong1@zte.com.cn>
> ---
>  drivers/net/zxdh/meson.build   |   1 +
>  drivers/net/zxdh/zxdh_ethdev.c |  43 +++++
>  drivers/net/zxdh/zxdh_ethdev.h |  18 ++-
>  drivers/net/zxdh/zxdh_pci.c    | 278 +++++++++++++++++++++++++++++++++
>  drivers/net/zxdh/zxdh_pci.h    | 138 ++++++++++++++++
>  drivers/net/zxdh/zxdh_queue.h  | 109 +++++++++++++
>  drivers/net/zxdh/zxdh_rxtx.h   |  55 +++++++
>

zxdh_queue.h & zxdh_rxtx.h seems not used in this patch, and they are
not indeed related to the PCI init, or PCI at all.
Can you add them in the patch that they are used?

<...>

> +#define ZXDH_PMD_DEFAULT_GUEST_FEATURES   \
> +    (1ULL << ZXDH_NET_F_MRG_RXBUF | \
> +     1ULL << ZXDH_NET_F_STATUS    | \
> +     1ULL << ZXDH_NET_F_MQ        | \
> +     1ULL << ZXDH_F_ANY_LAYOUT    | \
> +     1ULL << ZXDH_F_VERSION_1     | \
> +     1ULL << ZXDH_F_RING_PACKED   | \
> +     1ULL << ZXDH_F_IN_ORDER      | \
> +     1ULL << ZXDH_F_NOTIFICATION_DATA | \
> +     1ULL << ZXDH_NET_F_MAC)
> +
> +static void zxdh_read_dev_config(struct zxdh_hw *hw,
> +                                   size_t offset,
> +                                   void *dst,
> +                                   int32_t length)
>

Syntax issue, with tab stop = 8, above lines looks wrong can you please
check?

<...>

> +struct zxdh_virtqueue {
> +    struct zxdh_hw  *hw; /**< zxdh_hw structure pointer. */
> +    struct {
> +        /**< vring keeping descs and events */
> +        struct zxdh_vring_packed ring;
> +        uint8_t used_wrap_counter;
> +        uint8_t rsv;
> +        uint16_t cached_flags; /**< cached flags for descs */
> +        uint16_t event_flags_shadow;
> +        uint16_t rsv1;
> +    } __rte_packed vq_packed;
> +    uint16_t vq_used_cons_idx; /**< last consumed descriptor */
> +    uint16_t vq_nentries;  /**< vring desc numbers */
> +    uint16_t vq_free_cnt;  /**< num of desc available */
> +    uint16_t vq_avail_idx; /**< sync until needed */
> +    uint16_t vq_free_thresh; /**< free threshold */
> +    uint16_t rsv2;
> +
> +    void *vq_ring_virt_mem;  /**< linear address of vring*/
> +    uint32_t vq_ring_size;
> +
> +    union {
> +        struct zxdh_virtnet_rx rxq;
> +        struct zxdh_virtnet_tx txq;
> +    };
> +
> +    /** < physical address of vring,
> +     * or virtual address
> +     **/
>

'/**' is doxygen syntax, I don't know if you are using doxgen.
But '/**<' is doxygen sytax to say comment is for the code before it,
not after. So it is used wrong above.
If you don't have any specific reason, why not using regular comments
(non doxygen syntax), and leave first line empty, and terminate with
'*/', so above becomes:
/*
 * physical address of vring, or virtual address
 */

This comment is for all comment code in all the series. It looks like
there is a mixture of the usage.