Re: [dpdk-dev] [PATCH v7 12/19] net/ngbe: add Rx queue setup and release

[Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>]

On 7/6/21 12:55 PM, Jiawen Wu wrote:
> Setup device Rx queue and release Rx queue.
> 
> Signed-off-by: Jiawen Wu <jiawenwu@trustnetic.com>

I thikn format string should be on the next its own line
to address below warnings:

### net/ngbe: add Rx queue setup and release

WARNING:LONG_LINE: line length of 86 exceeds 80 columns
#433: FILE: drivers/net/ngbe/ngbe_rxtx.c:280:
+       PMD_INIT_LOG(DEBUG, "sw_ring=%p sw_sc_ring=%p hw_ring=%p
dma_addr=0x%" PRIx64,

CHECK:CAMELCASE: Avoid CamelCase: <PRIx64>
#433: FILE: drivers/net/ngbe/ngbe_rxtx.c:280:
+       PMD_INIT_LOG(DEBUG, "sw_ring=%p sw_sc_ring=%p hw_ring=%p
dma_addr=0x%" PRIx64,

total: 0 errors, 1 warnings, 1 checks, 502 lines checked

[snip]

> +/**
> + * Structure associated with each Rx queue.
> + */
> +struct ngbe_rx_queue {
> +	struct rte_mempool  *mb_pool; /**< mbuf pool to populate Rx ring. */
> +	volatile struct ngbe_rx_desc *rx_ring; /**< Rx ring virtual address. */
> +	uint64_t            rx_ring_phys_addr; /**< Rx ring DMA address. */
> +	volatile uint32_t   *rdt_reg_addr; /**< RDT register address. */
> +	volatile uint32_t   *rdh_reg_addr; /**< RDH register address. */
> +	struct ngbe_rx_entry *sw_ring; /**< address of Rx software ring. */
> +	/**< address of scattered Rx software ring. */

Should start from /**

> +	struct ngbe_scattered_rx_entry *sw_sc_ring;
> +	struct rte_mbuf *pkt_first_seg; /**< First segment of current packet. */
> +	struct rte_mbuf *pkt_last_seg; /**< Last segment of current packet. */
> +	uint16_t            nb_rx_desc; /**< number of Rx descriptors. */
> +	uint16_t            rx_tail;  /**< current value of RDT register. */
> +	uint16_t            nb_rx_hold; /**< number of held free Rx desc. */
> +	uint16_t rx_nb_avail; /**< nr of staged pkts ready to ret to app */
> +	uint16_t rx_next_avail; /**< idx of next staged pkt to ret to app */
> +	uint16_t rx_free_trigger; /**< triggers rx buffer allocation */
> +	uint16_t            rx_free_thresh; /**< max free Rx desc to hold. */
> +	uint16_t            queue_id; /**< RX queue index. */
> +	uint16_t            reg_idx;  /**< RX queue register index. */
> +	/**< Packet type mask for different NICs. */

I don't understand above comment.  Where is packet type mask?

> +	uint16_t            port_id;  /**< Device port identifier. */
> +	uint8_t             drop_en;  /**< If not 0, set SRRCTL.Drop_En. */
> +	uint8_t             rx_deferred_start; /**< not in global dev start. */
> +	/** need to alloc dummy mbuf, for wraparound when scanning hw ring */
> +	struct rte_mbuf fake_mbuf;
> +	/** hold packets to return to application */
> +	struct rte_mbuf *rx_stage[RTE_PMD_NGBE_RX_MAX_BURST * 2];
> +};

Above is very hard to read. Please, align more consistently.
If alignment different in different lines, please, add empty
line to make it readable.