Re: [PATCH 2/3] can: rockchip_canfd: retry the outstanding TX buffer

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but there is a data race on hardware registers RKCANFD_REG_MODE and RKCANFD_REG_CMD in rkcanfd_start_xmit_write_cmd() between softirq and hard IRQ contexts.
--

--- Patch [2]: [PATCH 2/3] can: rockchip_canfd: retry the outstanding TX buffer ---
commit 008079a4d397736133d0b75cae9351b6c5be751d
Author: Cunhao Lu <1579567540@qq.com>

can: rockchip_canfd: retry the outstanding TX buffer

After the FIFO depth was increased to two, tx_head selects the next
free buffer instead of the outstanding one, breaking the erratum 6
workaround. This patch fixes the issue by using tx_tail to select
the outstanding buffer for retransmission.

> diff --git a/drivers/net/can/rockchip/rockchip_canfd-tx.c b/drivers/net/can/rockchip/rockchip_canfd-tx.c
> index d2cbe25c46d6c..c4ecdc9411cc6 100644
> --- a/drivers/net/can/rockchip/rockchip_canfd-tx.c
> +++ b/drivers/net/can/rockchip/rockchip_canfd-tx.c
> @@ -57,8 +57,8 @@ static void rkcanfd_start_xmit_write_cmd(const struct rkcanfd_priv *priv,
>  
>  void rkcanfd_xmit_retry(struct rkcanfd_priv *priv)
>  {
> -	const unsigned int tx_head = rkcanfd_get_tx_head(priv);
> -	const u32 reg_cmd = RKCANFD_REG_CMD_TX_REQ(tx_head);
> +	const unsigned int tx_tail = rkcanfd_get_tx_tail(priv);
> +	const u32 reg_cmd = RKCANFD_REG_CMD_TX_REQ(tx_tail);
>  
>  	rkcanfd_start_xmit_write_cmd(priv, reg_cmd);

[Severity: High]
This isn't a bug introduced by this patch, but does this call expose a race
condition on the hardware registers between softirq and hardirq contexts?

rkcanfd_start_xmit() runs in softirq context and calls
rkcanfd_start_xmit_write_cmd(), which performs non-atomic register writes
to RKCANFD_REG_MODE. If a hardware RX interrupt preempts execution between
these writes, rkcanfd_irq() (hardirq) will call rkcanfd_xmit_retry()
which also calls rkcanfd_start_xmit_write_cmd().

The hardirq will restore priv->reg_mode_default, so when the softirq
resumes, RKCANFD_REG_CMD is written without the required SPACE_RX_MODE
bit set, bypassing the RK3568 Erratum 12 workaround. Could this lead to
hardware misbehavior under heavy load?

>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-master-v1-0-6945fdd4f0ff@qq.com?part=2