Re: [PATCH v4 3/3] can: rockchip_canfd: serialize TX state and command writes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Hardware FIFO and software state desynchronization when can_put_echo_skb fails.
--

--- Patch [3]: [PATCH v4 3/3] can: rockchip_canfd: serialize TX state and command writes ---
commit ab677d838393ae26a8c9bbe2590b03c3dd72671f
Author: Cunhao Lu <1579567540@qq.com>

can: rockchip_canfd: serialize TX state and command writes

This commit adds a tx_lock to serialize TX state and command writes in the 
Rockchip CAN FD driver. It addresses races between the transmit and completion 
paths, as well as an erratum 12 workaround bypass.

> diff --git a/drivers/net/can/rockchip/rockchip_canfd-tx.c b/drivers/net/can/rockchip/rockchip_canfd-tx.c
> index 2b5cd6aab31b6..c311b6990bc9e 100644
> --- a/drivers/net/can/rockchip/rockchip_canfd-tx.c
> +++ b/drivers/net/can/rockchip/rockchip_canfd-tx.c

[ ... ]

> @@ -124,8 +138,11 @@ netdev_tx_t rkcanfd_start_xmit(struct sk_buff *skb, struct net_device *ndev)
>  			      *(u32 *)(cfd->data + i));
>  
>  	frame_len = can_skb_get_frame_len(skb);
> +	spin_lock_irqsave(&priv->tx_lock, flags);
>  	err = can_put_echo_skb(skb, ndev, tx_head, frame_len);
>  	if (err) {
> +		spin_unlock_irqrestore(&priv->tx_lock, flags);
> +
>  		ndev->stats.tx_dropped++;
>  		return NETDEV_TX_OK;
>  	}

[Severity: High]
Does this error path leave the software and hardware out of sync?

If can_put_echo_skb() fails, the driver returns early from rkcanfd_start_xmit().

However, the frame data has already been written to the memory-mapped FIFO
input port just before this code block. Because the error path skips
advancing tx_head and issuing the TX command, it consumes a hardware buffer
slot and causes the software and hardware pointers to become permanently
out of sync.

On the next transmission, a new frame is pushed to the hardware, but the
un-advanced tx_head command is issued, which transmits the previously dropped
frame instead.

Should the driver only write to the hardware registers after
can_put_echo_skb() succeeds?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-master-v4-0-07f7952f761d@qq.com?part=3