Re: [PATCH v9 12/19] drivers: soc: ti: k3-ringacc: handle absence of tisci

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] soc: ti: k3-ringacc: base address zeroed before size in ring_free
- [Medium] soc: ti: k3-ringacc: missing interrupt disable during ring free
--

commit a7de7c5cbd863cd6f1879ceaa2a3c115f7f1b58b
Author: Sai Sree Kartheek Adivi <s-adivi@ti.com>

drivers: soc: ti: k3-ringacc: handle absence of tisci

This commit adds support for platforms without tisci firmware, such as AM62L.
It does this by handling the absence of tisci using direct register writes for
ring configuration and reset.

> diff --git a/drivers/soc/ti/k3-ringacc.c b/drivers/soc/ti/k3-ringacc.c
> index 5966db4327b13..0ef07207e1dfa 100644
> --- a/drivers/soc/ti/k3-ringacc.c
> +++ b/drivers/soc/ti/k3-ringacc.c

[ ... ]

> @@ -576,10 +653,25 @@ static void k3_ringacc_ring_free_sci(struct k3_ring *ring)
>  	ring_cfg.index = ring->ring_id;
>  	ring_cfg.valid_params = TI_SCI_MSG_VALUE_RM_ALL_NO_ORDER;
>  
> -	ret = ringacc->tisci_ring_ops->set_cfg(ringacc->tisci, &ring_cfg);
> -	if (ret)
> -		dev_err(ringacc->dev, "TISCI ring free fail (%d) ring_idx %d\n",
> -			ret, ring->ring_id);
> +	if (!ringacc->tisci) {
> +		u32 reg;
> +

[Severity: Medium]
When k3_ringacc_ring_cfg_sci() configures a ring in the !tisci path, it
explicitly enables interrupts by writing to ring->intr->enable_set:

	writel(K3_RINGACC_RT_INT_ENABLE_SET_COMPLETE | 
	       K3_RINGACC_RT_INT_ENABLE_SET_TR,
	       &ring->intr->enable_set);

Should k3_ringacc_ring_free_sci() symmetrically disable these interrupts
during teardown? Failing to disable interrupts while setting the size to 0
might leave the hardware in an inconsistent state or cause spurious IRQs if
the ring is reallocated.

> +		writel(ring_cfg.addr_lo, &ring->cfg->ba_lo);
> +		writel((ring_cfg.addr_hi & K3_DMARING_CFG_ADDR_HI_MASK) +
> +				(ring_cfg.asel << K3_DMARING_CFG_ASEL_SHIFT),
> +				&ring->cfg->ba_hi);

[Severity: High]
Does writing the base addresses before clearing the size register introduce a
race condition?

Since the size is not yet updated to 0 here, the DMA engine remains technically
active. By clearing ba_lo and ba_hi first, any background transaction executing
in this narrow window could initiate a memory access at physical address 0x0,
potentially causing memory corruption or an IOMMU fault.

Would it be safer to clear the size register before updating the base address
registers?

> +
> +		reg = readl(&ring->cfg->size);
> +		reg &= ~K3_DMARING_CFG_SIZE_MASK;
> +		reg |= ring_cfg.count & K3_DMARING_CFG_SIZE_MASK;
> +
> +		writel(reg, &ring->cfg->size);
> +	} else {
> +		ret = ringacc->tisci_ring_ops->set_cfg(ringacc->tisci, &ring_cfg);
> +		if (ret)
> +			dev_err(ringacc->dev, "TISCI ring free fail (%d) ring_idx %d\n",
> +				ret, ring->ring_id);
> +	}
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260922064902.2719979-1-s-adivi@ti.com?part=12