Re: [PATCH net-next v9 13/14] xsc: Add eth reception data path

[Simon Horman <horms@kernel.org>]

On Tue, Mar 18, 2025 at 11:15:21PM +0800, Xin Tian wrote:

...

> diff --git a/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth_common.h b/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth_common.h

...

> +#define XSC_SET_PFLAG(params, pflag, enable)			\
> +	do {							\
> +		if (enable)					\
> +			(params)->pflags |= BIT(pflag);		\
> +		else						\
> +			(params)->pflags &= ~(BIT(pflag));	\
> +	} while (0)

Hi Xin Tian,

XSC_SET_PFLAG() seems to be unused. Perhaps it is best to drop it and
add it when needed.

And, FWIIW, I would have implemented both XSC_SET_PFLAG() and
XSC_GET_PFLAG() as functions as there doesn't seem to be a reason that they
need to be macros.

> +
> +#define XSC_GET_PFLAG(params, pflag) (!!((params)->pflags & (BIT(pflag))))

...

> diff --git a/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth_rx.c b/drivers/net/ethernet/yunsilicon/xsc/net/xsc_eth_rx.c

...

> +bool xsc_eth_post_rx_wqes(struct xsc_rq *rq)
> +{
> +	struct xsc_wq_cyc *wq = &rq->wqe.wq;
> +	u8 wqe_bulk, wqe_bulk_min;
> +	int alloc;
> +	u16 head;
> +	int err;
> +
> +	wqe_bulk = rq->wqe.info.wqe_bulk;
> +	wqe_bulk_min = rq->wqe.info.wqe_bulk_min;
> +	if (xsc_wq_cyc_missing(wq) < wqe_bulk)
> +		return false;
> +
> +	do {
> +		head = xsc_wq_cyc_get_head(wq);
> +
> +		alloc = min_t(int, wqe_bulk, xsc_wq_cyc_missing(wq));
> +		if (alloc < wqe_bulk && alloc >= wqe_bulk_min)
> +			alloc = alloc & 0xfffffffe;
> +
> +		if (alloc > 0) {
> +			err = xsc_alloc_rx_wqes(rq, head, alloc);
> +			if (unlikely(err))
> +				break;
> +
> +			xsc_wq_cyc_push_n(wq, alloc);
> +		}
> +	} while (xsc_wq_cyc_missing(wq) >= wqe_bulk_min);
> +
> +	dma_wmb();
> +
> +	/* ensure wqes are visible to device before updating doorbell record */
> +	xsc_rq_notify_hw(rq);
> +
> +	return !!err;

Perhaps it can't occur in practice, but err will be used uninitialised here
if the alloc condition in the do loop above is never met.

> +}

...