Re: [PATCH 10/22] ASoC: rsnd: Add DMA support infrastructure for RZ/G3E
From: Kuninori Morimoto
Date: Sun Mar 22 2026 - 21:08:17 EST
Hi John
Thank you for your patch
> RZ/G3E has different DMA register base addresses and offset calculations
> compared to R-Car platforms, and requires additional audmac-pp clock and
> reset lines for Audio DMAC operation.
>
> Add RZ/G3E-specific DMA address macros and audmac-pp clock/reset support
> using optional APIs to remain transparent to other platforms.
>
> Signed-off-by: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> ---
I think it include many features in 1 patch.
You should separate it into each features.
> diff --git a/sound/soc/renesas/rcar/dma.c b/sound/soc/renesas/rcar/dma.c
> index 68c859897e68..d3123ae3b402 100644
> --- a/sound/soc/renesas/rcar/dma.c
> +++ b/sound/soc/renesas/rcar/dma.c
> @@ -496,24 +496,71 @@ static struct rsnd_mod_ops rsnd_dmapp_ops = {
> * SSIU: 0xec541000 / 0xec100000 / 0xec100000 / 0xec400000 / 0xec400000
> * SCU : 0xec500000 / 0xec000000 / 0xec004000 / 0xec300000 / 0xec304000
> * CMD : 0xec500000 / / 0xec008000 0xec308000
> + *
> + * ex) G3E case
> + * mod / DMAC in / DMAC out / DMAC PP in / DMAC pp out
> + * SSI : 0x13C31000 / 0x13C40000 / 0x13C40000
> + * SSIU: 0x13C31000 / 0x13C40000 / 0x13C40000 / 0xEC400000 / 0xEC400000
> + * SCU : 0x13C00000 / 0x13C10000 / 0x13C14000 / 0xEC300000 / 0xEC304000
> + * CMD : 0x13C00000 / / 0x13C18000 0xEC308000
> */
> -#define RDMA_SSI_I_N(addr, i) (addr ##_reg - 0x00300000 + (0x40 * i) + 0x8)
> -#define RDMA_SSI_O_N(addr, i) (addr ##_reg - 0x00300000 + (0x40 * i) + 0xc)
>
> -#define RDMA_SSIU_I_N(addr, i, j) (addr ##_reg - 0x00441000 + (0x1000 * (i)) + (((j) / 4) * 0xA000) + (((j) % 4) * 0x400) - (0x4000 * ((i) / 9) * ((j) / 4)))
> -#define RDMA_SSIU_O_N(addr, i, j) RDMA_SSIU_I_N(addr, i, j)
> +/* RZ/G3E DMA address macros */
> +#define RDMA_SSI_I_N_G3E(addr, i) (addr ##_reg + 0x0000F000 + (0x1000 * i))
> +#define RDMA_SSI_O_N_G3E(addr, i) (addr ##_reg + 0x0000F000 + (0x1000 * i))
> +
> +#define RDMA_SSIU_I_N_G3E(addr, i, j) (addr ##_reg + 0x0000F000 + (0x1000 * (i)) + (((j) / 4) * 0xA000) + (((j) % 4) * 0x400) - (0x4000 * ((i) / 9) * ((j) / 4)))
> +#define RDMA_SSIU_O_N_G3E(addr, i, j) RDMA_SSIU_I_N_G3E(addr, i, j)
> +
> +#define RDMA_SSIU_I_P_G3E(addr, i, j) (addr ##_reg + 0xD87CF000 + (0x1000 * (i)) + (((j) / 4) * 0xA000) + (((j) % 4) * 0x400) - (0x4000 * ((i) / 9) * ((j) / 4)))
> +#define RDMA_SSIU_O_P_G3E(addr, i, j) RDMA_SSIU_I_P_G3E(addr, i, j)
> +
> +#define RDMA_SRC_I_N_G3E(addr, i) (addr ##_reg + 0x00010000 + (0x400 * i))
> +#define RDMA_SRC_O_N_G3E(addr, i) (addr ##_reg + 0x00014000 + (0x400 * i))
> +
> +#define RDMA_SRC_I_P_G3E(addr, i) (addr ##_reg + 0xD8700000 + (0x400 * i))
> +#define RDMA_SRC_O_P_G3E(addr, i) (addr ##_reg + 0xD8704000 + (0x400 * i))
> +
> +#define RDMA_CMD_O_N_G3E(addr, i) (addr ##_reg + 0x00018000 + (0x400 * i))
> +#define RDMA_CMD_O_P_G3E(addr, i) (addr ##_reg + 0xD8708000 + (0x400 * i))
> +
> +/* R-Car DMA address macros */
> +#define RDMA_SSI_I_N_RCAR(addr, i) (addr ##_reg - 0x00300000 + (0x40 * i) + 0x8)
> +#define RDMA_SSI_O_N_RCAR(addr, i) (addr ##_reg - 0x00300000 + (0x40 * i) + 0xc)
>
> -#define RDMA_SSIU_I_P(addr, i, j) (addr ##_reg - 0x00141000 + (0x1000 * (i)) + (((j) / 4) * 0xA000) + (((j) % 4) * 0x400) - (0x4000 * ((i) / 9) * ((j) / 4)))
> -#define RDMA_SSIU_O_P(addr, i, j) RDMA_SSIU_I_P(addr, i, j)
> +#define RDMA_SSIU_I_N_RCAR(addr, i, j) (addr ##_reg - 0x00441000 + (0x1000 * (i)) + (((j) / 4) * 0xA000) + (((j) % 4) * 0x400) - (0x4000 * ((i) / 9) * ((j) / 4)))
> +#define RDMA_SSIU_O_N_RCAR(addr, i, j) RDMA_SSIU_I_N_RCAR(addr, i, j)
>
> -#define RDMA_SRC_I_N(addr, i) (addr ##_reg - 0x00500000 + (0x400 * i))
> -#define RDMA_SRC_O_N(addr, i) (addr ##_reg - 0x004fc000 + (0x400 * i))
> +#define RDMA_SSIU_I_P_RCAR(addr, i, j) (addr ##_reg - 0x00141000 + (0x1000 * (i)) + (((j) / 4) * 0xA000) + (((j) % 4) * 0x400) - (0x4000 * ((i) / 9) * ((j) / 4)))
> +#define RDMA_SSIU_O_P_RCAR(addr, i, j) RDMA_SSIU_I_N_RCAR(addr, i, j)
>
> -#define RDMA_SRC_I_P(addr, i) (addr ##_reg - 0x00200000 + (0x400 * i))
> -#define RDMA_SRC_O_P(addr, i) (addr ##_reg - 0x001fc000 + (0x400 * i))
> +#define RDMA_SRC_I_N_RCAR(addr, i) (addr ##_reg - 0x00500000 + (0x400 * i))
> +#define RDMA_SRC_O_N_RCAR(addr, i) (addr ##_reg - 0x004fc000 + (0x400 * i))
>
> -#define RDMA_CMD_O_N(addr, i) (addr ##_reg - 0x004f8000 + (0x400 * i))
> -#define RDMA_CMD_O_P(addr, i) (addr ##_reg - 0x001f8000 + (0x400 * i))
> +#define RDMA_SRC_I_P_RCAR(addr, i) (addr ##_reg - 0x00200000 + (0x400 * i))
> +#define RDMA_SRC_O_P_RCAR(addr, i) (addr ##_reg - 0x001fc000 + (0x400 * i))
> +
> +#define RDMA_CMD_O_N_RCAR(addr, i) (addr ##_reg - 0x004f8000 + (0x400 * i))
> +#define RDMA_CMD_O_P_RCAR(addr, i) (addr ##_reg - 0x001f8000 + (0x400 * i))
> +
> +/* Platform-agnostic address macros */
> +#define RDMA_SSI_I_N(p, addr, i) rsnd_is_rzg3e(p) ? RDMA_SSI_I_N_G3E(addr, i) : RDMA_SSI_I_N_RCAR(addr, i)
> +#define RDMA_SSI_O_N(p, addr, i) rsnd_is_rzg3e(p) ? RDMA_SSI_O_N_G3E(addr, i) : RDMA_SSI_O_N_RCAR(addr, i)
> +
> +#define RDMA_SSIU_I_N(p, addr, i, j) rsnd_is_rzg3e(p) ? RDMA_SSIU_I_N_G3E(addr, i, j) : RDMA_SSIU_I_N_RCAR(addr, i, j)
> +#define RDMA_SSIU_O_N(p, addr, i, j) rsnd_is_rzg3e(p) ? RDMA_SSIU_O_N_G3E(addr, i, j) : RDMA_SSIU_O_N_RCAR(addr, i, j)
> +
> +#define RDMA_SSIU_I_P(p, addr, i, j) rsnd_is_rzg3e(p) ? RDMA_SSIU_I_P_G3E(addr, i, j) : RDMA_SSIU_I_P_RCAR(addr, i, j)
> +#define RDMA_SSIU_O_P(p, addr, i, j) rsnd_is_rzg3e(p) ? RDMA_SSIU_O_P_G3E(addr, i, j) : RDMA_SSIU_O_P_RCAR(addr, i, j)
> +
> +#define RDMA_SRC_I_N(p, addr, i) rsnd_is_rzg3e(p) ? RDMA_SRC_I_N_G3E(addr, i) : RDMA_SRC_I_N_RCAR(addr, i)
> +#define RDMA_SRC_O_N(p, addr, i) rsnd_is_rzg3e(p) ? RDMA_SRC_O_N_G3E(addr, i) : RDMA_SRC_O_N_RCAR(addr, i)
> +
> +#define RDMA_SRC_I_P(p, addr, i) rsnd_is_rzg3e(p) ? RDMA_SRC_I_P_G3E(addr, i) : RDMA_SRC_I_P_RCAR(addr, i)
> +#define RDMA_SRC_O_P(p, addr, i) rsnd_is_rzg3e(p) ? RDMA_SRC_O_P_G3E(addr, i) : RDMA_SRC_O_P_RCAR(addr, i)
> +
> +#define RDMA_CMD_O_N(p, addr, i) rsnd_is_rzg3e(p) ? RDMA_CMD_O_N_G3E(addr, i) : RDMA_CMD_O_N_RCAR(addr, i)
> +#define RDMA_CMD_O_P(p, addr, i) rsnd_is_rzg3e(p) ? RDMA_CMD_O_P_G3E(addr, i) : RDMA_CMD_O_P_RCAR(addr, i)
I think you want to create new rsnd_rzg3e_dma_addr() and call it,
instead of makes existing code complex.
+ static dma_addr_t rsnd_rzg3e_dma_addr(...)
+ {
+ ...
+ }
...
static dma_addr_t rsnd_dma_addr(...)
{
...
else if (rsnd_is_gen4(priv))
return rsnd_gen4_dma_addr(...);
+ else if (rsnd_is_rzg3e(priv))
+ return rsnd_rzg3e_dma_addr(...)
else
return rsnd_gen2_dma_addr(...);
}
> @@ -860,6 +917,56 @@ int rsnd_dma_probe(struct rsnd_priv *priv)
> return 0; /* it will be PIO mode */
> }
>
> + /*
> + * audmac_pp clock/reset management strategy:
> + *
> + * Unlike other modules (SSI, SRC, etc.) which have their own dedicated
> + * clocks, all DMA modules share the single audmac_pp clock/reset.
> + * Managing it per-stream or per-DMA-module causes
> + * reference count imbalances:
> + *
> + * - rsnd_mod_init() does clk_prepare_enable() then clk_disable(),
> + * leaving prepare_count=1 per module
> + * - With N DMA modules sharing the same clock handle, prepare_count=N
> + * - suspend does single clk_disable_unprepare() (-1)
> + * - resume does single clk_prepare_enable() (+1)
> + * - Result: prepare_count leaks on each suspend/resume cycle
> + *
> + * Per-stream management (iterating DMA modules in suspend/resume) is
> + * not worth the complexity:
> + *
> + * - No power benefit: audmac_pp is needed whenever ANY stream is
> + * active, and every stream uses DMA, so it's essentially always on
> + * - Architecture mismatch: DMA modules live in io->dma, not in a
> + * priv array -- no clean way to iterate like SSI/SRC/DVC
> + * - Shared handle problem: all DMA modules point to the same clock,
> + * so iterating would call clk_unprepare() N times on one clock
> + * - Would require manual refcounting ("enable on first stream,
> + * disable on last") -- reimplementing what clk framework does
> + *
> + * The correct approach is to treat audmac_pp as always-on infrastructure
> + * (same pattern as clk_adg), managed globally:
> + * - Probe: acquire + enable (via devm_clk_get_optional_enabled)
> + * - Suspend/Resume: toggle in core.c rsnd_suspend/rsnd_resume
> + * - Remove: devm cleanup
> + * - DMA modules: pass NULL clock/reset to rsnd_mod_init()
> + *
> + * Use devm variants that handle deassert/enable automatically.
> + * Order: reset deasserted first, then clock enabled.
> + */
> + priv->rstc_audmac_pp =
> + devm_reset_control_get_optional_exclusive_deasserted(dev, "audmac_pp");
> + if (IS_ERR(priv->rstc_audmac_pp)) {
> + return dev_err_probe(dev, PTR_ERR(priv->rstc_audmac_pp),
> + "failed to get audmac_pp reset\n");
> + }
> +
> + priv->clk_audmac_pp = devm_clk_get_optional_enabled(dev, "audmac_pp");
> + if (IS_ERR(priv->clk_audmac_pp)) {
> + return dev_err_probe(dev, PTR_ERR(priv->clk_audmac_pp),
> + "failed to get audmac_pp clock\n");
> + }
rsnd_dma_probe() is common fucntion.
Is above possible to keep compatible with other SoCs ?
And, we are already using "audmacpp".
I think it time to update rsnd_dma_probe() like below ?
int rsnd_dma_probe(...)
{
if (rsnd_is_gen1(..))
return ...
else if (rsnd_is_gen2(...) ||
rsnd_is_gen3(...))
return ...
else if (rsnd_is_gen4(...))
return ...
else if (rsnd_is_rzg3e(...))
return ...
...
}