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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The direct register fallback in `k3_ringacc_ring_reconfig_qmode_sci` uses zero-initialized fields to destructively clear the ring's base address and size.
--

--- Patch [12]: [PATCH v8 12/19] drivers: soc: ti: k3-ringacc: handle absence of tisci ---
commit 19dd5943c68e7ce05e29d608e96d6376a6ce8fed
Author: Sai Sree Kartheek Adivi <[email protected]>

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

This commit adds direct register writes to handle platforms like AM62L that 
do not have tisci firmware. It modifies the reset, free, configuration, 
and reconfiguration paths to write directly to ring configuration registers 
when tisci is absent.

> diff --git a/drivers/soc/ti/k3-ringacc.c b/drivers/soc/ti/k3-ringacc.c
> --- a/drivers/soc/ti/k3-ringacc.c
> +++ b/drivers/soc/ti/k3-ringacc.c
[ ... ]
> @@ -501,10 +568,25 @@ static void k3_ringacc_ring_reconfig_qmode_sci(struct k3_ring *ring,
>  	ring_cfg.valid_params = TI_SCI_MSG_VALUE_RM_RING_MODE_VALID;
>  	ring_cfg.mode = mode;
>  
> -	ret = ringacc->tisci_ring_ops->set_cfg(ringacc->tisci, &ring_cfg);
> -	if (ret)
> -		dev_err(ringacc->dev, "TISCI reconf qmode fail (%d) ring_idx %d\n",
> -			ret, ring->ring_id);
> +	if (!ringacc->tisci) {
> +		u32 reg;
> +
> +		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);
> +
> +		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);

[Severity: Medium]
Does this code accidentally clear the ring's base address and size instead of
just reconfiguring the mode?

Because ring_cfg is zero-initialized at the start of the function:

struct ti_sci_msg_rm_ring_cfg ring_cfg = { 0 };

The addr_lo, addr_hi, asel, and count fields are never populated in
k3_ringacc_ring_reconfig_qmode_sci() before being used here. Will this write
zeros to ba_lo, ba_hi, and size, corrupting the live memory mapping?

Even if this path is currently unreachable because platforms like AM62L do not
enable the dma_ring_reset_quirk, this might still cause hardware misconfiguration
if the quirk is used on future platforms without TISCI.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=12