On 18/02/2025 20:03, Simon Horman wrote:
> On Mon, Feb 17, 2025 at 09:31:49AM +0200, Roger Quadros wrote:
>> This allows us to re-use am65_cpsw_run_xdp() for zero copy
>> case. Add AM65_CPSW_XDP_TX case for successful XDP_TX so we don't
>> free the page while in flight.
>>
>> Signed-off-by: Roger Quadros <rogerq@xxxxxxxxxx>
>
> Reviewed-by: Simon Horman <horms@xxxxxxxxxx>
>
>> ---
>> drivers/net/ethernet/ti/am65-cpsw-nuss.c | 13 ++++++++-----
>> 1 file changed, 8 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
>
> ...
>
>> @@ -1230,9 +1230,6 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_rx_flow *flow,
>> ndev->stats.rx_dropped++;
>> }
>>
>> - page = virt_to_head_page(xdp->data);
>> - am65_cpsw_put_page(flow, page, true);
>> -
>> return ret;
>
> It seems that before and after this patch ret is always initialised to
> AM65_CPSW_XDP_CONSUMED and never changed. So it can be removed.
>
> Given that with this patch the function only returns after the switch
> statement, I think this would be a nice follow-up.
>
> diff --git a/drivers/net/ethernet/ti/am65-cpsw-nuss.c b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> index 20a4fc3e579f..4052c9153632 100644
> --- a/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> +++ b/drivers/net/ethernet/ti/am65-cpsw-nuss.c
> @@ -1172,7 +1172,6 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_rx_flow *flow,
> {
> struct am65_cpsw_common *common = flow->common;
> struct net_device *ndev = port->ndev;
> - int ret = AM65_CPSW_XDP_CONSUMED;
> struct am65_cpsw_tx_chn *tx_chn;
> struct netdev_queue *netif_txq;
> int cpu = smp_processor_id();
> @@ -1228,9 +1227,8 @@ static int am65_cpsw_run_xdp(struct am65_cpsw_rx_flow *flow,
> fallthrough;
> case XDP_DROP:
> ndev->stats.rx_dropped++;
> + return AM65_CPSW_XDP_CONSUMED;
> }
> -
> - return ret;
> }
>
> /* RX psdata[2] word format - checksum information */
>
> ...
Thank you for this suggestion. I will add this cleanup in my next set of patches.
--
cheers,
-roger
