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

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_ec=
ho_skb fails.
--

--- Patch [3]: [PATCH v4 3/3] can: rockchip_canfd: serialize TX state and c=
ommand writes ---
commit ab677d838393ae26a8c9bbe2590b03c3dd72671f
Author: Cunhao Lu <[email protected]>

can: rockchip_canfd: serialize TX state and command writes

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

> diff --git a/drivers/net/can/rockchip/rockchip_canfd-tx.c b/drivers/net/c=
an/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));
> =20
>  	frame_len =3D can_skb_get_frame_len(skb);
> +	spin_lock_irqsave(&priv->tx_lock, flags);
>  	err =3D 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_xm=
it().

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 dropp=
ed
frame instead.

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

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260730-master-v4-=
[email protected]?part=3D3