[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<TY3PR01MB113463BE8A4B1A40DBB0860538693A@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Date: Mon, 26 Jan 2026 12:10:17 +0000
From: Biju Das <biju.das.jz@...renesas.com>
To: Claudiu.Beznea <claudiu.beznea@...on.dev>, "vkoul@...nel.org"
<vkoul@...nel.org>, Prabhakar Mahadev Lad
<prabhakar.mahadev-lad.rj@...renesas.com>, "lgirdwood@...il.com"
<lgirdwood@...il.com>, "broonie@...nel.org" <broonie@...nel.org>,
"perex@...ex.cz" <perex@...ex.cz>, "tiwai@...e.com" <tiwai@...e.com>,
"p.zabel@...gutronix.de" <p.zabel@...gutronix.de>, "geert+renesas@...der.be"
<geert+renesas@...der.be>, Fabrizio Castro <fabrizio.castro.jz@...esas.com>
CC: "dmaengine@...r.kernel.org" <dmaengine@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-sound@...r.kernel.org" <linux-sound@...r.kernel.org>,
"linux-renesas-soc@...r.kernel.org" <linux-renesas-soc@...r.kernel.org>,
Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
Subject: RE: [PATCH 5/7] dmaengine: sh: rz-dmac: Add suspend to RAM support
> -----Original Message-----
> From: Claudiu Beznea <claudiu.beznea@...on.dev>
> Sent: 26 January 2026 12:05
> To: Biju Das <biju.das.jz@...renesas.com>; vkoul@...nel.org; Prabhakar Mahadev Lad <prabhakar.mahadev-
> lad.rj@...renesas.com>; lgirdwood@...il.com; broonie@...nel.org; perex@...ex.cz; tiwai@...e.com;
> p.zabel@...gutronix.de; geert+renesas@...der.be; Fabrizio Castro <fabrizio.castro.jz@...esas.com>
> Cc: dmaengine@...r.kernel.org; linux-kernel@...r.kernel.org; linux-sound@...r.kernel.org; linux-
> renesas-soc@...r.kernel.org; Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> Subject: Re: [PATCH 5/7] dmaengine: sh: rz-dmac: Add suspend to RAM support
>
> Hi,
>
> On 1/26/26 13:03, Biju Das wrote:
> > Hi Claudiu,
> >
> > Thanks for the patch.
> >
> >> -----Original Message-----
> >> From: Claudiu <claudiu.beznea@...on.dev>
> >> Sent: 26 January 2026 10:32
> >> Subject: [PATCH 5/7] dmaengine: sh: rz-dmac: Add suspend to RAM
> >> support
> >>
> >> From: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> >>
> >> The Renesas RZ/G3S SoC supports a power saving mode in which power to
> >> most SoC components is turned off, including the DMA IP. Add suspend to RAM support to save and
> restore the DMA IP registers.
> >>
> >> Cyclic DMA channels require special handling. Since they can be
> >> paused and resumed during system suspend and resume, the driver
> >> restores additional registers for these channels during the resume
> >> phase. If a channel was not explicitly paused during suspend, the
> >> driver ensures that it is paused and resumed as part of the system suspend/resume flow. This might
> be the case of a serial device being used with no_console_suspend.
> >>
> >> For non-cyclic channels, the dev_pm_ops::prepare callback waits for
> >> all ongoing transfers to complete before allowing suspend-to-RAM to proceed.
> >>
> >> Signed-off-by: Claudiu Beznea <claudiu.beznea.uj@...renesas.com>
> >> ---
> >> drivers/dma/sh/rz-dmac.c | 183 +++++++++++++++++++++++++++++++++++++--
> >> 1 file changed, 175 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/dma/sh/rz-dmac.c b/drivers/dma/sh/rz-dmac.c
> >> index ab5f49a0b9f2..8f3e2719e639
> >> 100644
> >> --- a/drivers/dma/sh/rz-dmac.c
> >> +++ b/drivers/dma/sh/rz-dmac.c
> >> @@ -69,11 +69,15 @@ struct rz_dmac_desc {
> >> * enum rz_dmac_chan_status: RZ DMAC channel status
> >> * @RZ_DMAC_CHAN_STATUS_ENABLED: Channel is enabled
> >> * @RZ_DMAC_CHAN_STATUS_PAUSED: Channel is paused though DMA engine
> >> callbacks
> >> + * @RZ_DMAC_CHAN_STATUS_PAUSED_INTERNAL: Channel is paused through
> >> + driver internal logic
> >> + * @RZ_DMAC_CHAN_STATUS_SYS_SUSPENDED: Channel was prepared for
> >> + system suspend
> >> * @RZ_DMAC_CHAN_STATUS_CYCLIC: Channel is cyclic
> >> */
> >> enum rz_dmac_chan_status {
> >> RZ_DMAC_CHAN_STATUS_ENABLED,
> >> RZ_DMAC_CHAN_STATUS_PAUSED,
> >> + RZ_DMAC_CHAN_STATUS_PAUSED_INTERNAL,
> >> + RZ_DMAC_CHAN_STATUS_SYS_SUSPENDED,
> >> RZ_DMAC_CHAN_STATUS_CYCLIC,
> >> };
> >>
> >> @@ -94,6 +98,10 @@ struct rz_dmac_chan {
> >> u32 chctrl;
> >> int mid_rid;
> >>
> >> + struct {
> >> + u32 nxla;
> >> + } pm_state;
> >> +
> >> struct list_head ld_free;
> >> struct list_head ld_queue;
> >> struct list_head ld_active;
> >> @@ -1002,10 +1010,17 @@ static int rz_dmac_device_pause(struct dma_chan *chan)
> >> return rz_dmac_device_pause_set(channel,
> >> RZ_DMAC_CHAN_STATUS_PAUSED); }
> >>
> >> +static int rz_dmac_device_pause_internal(struct rz_dmac_chan
> >> +*channel) {
> >> + lockdep_assert_held(&channel->vc.lock);
> >> +
> >> + return rz_dmac_device_pause_set(channel,
> >> +RZ_DMAC_CHAN_STATUS_PAUSED_INTERNAL);
> >> +}
> >> +
> >> static int rz_dmac_device_resume_set(struct rz_dmac_chan *channel,
> >> enum rz_dmac_chan_status status) {
> >> - u32 val;
> >> + u32 val, chctrl;
> >> int ret;
> >>
> >> lockdep_assert_held(&channel->vc.lock);
> >> @@ -1013,14 +1028,33 @@ static int rz_dmac_device_resume_set(struct rz_dmac_chan *channel,
> >> if (!(channel->status & BIT(RZ_DMAC_CHAN_STATUS_PAUSED)))
> >> return 0;
> >>
> >> - rz_dmac_ch_writel(channel, CHCTRL_CLRSUS, CHCTRL, 1);
> >> - ret = read_poll_timeout_atomic(rz_dmac_ch_readl, val,
> >> - !(val & CHSTAT_SUS), 1, 1024, false,
> >> - channel, CHSTAT, 1);
> >> - if (ret)
> >> - return ret;
> >> + if (channel->status & BIT(RZ_DMAC_CHAN_STATUS_SYS_SUSPENDED)) {
> >> + /*
> >> + * We can be after a sleep state with power loss. If power was
> >> + * lost, the CHSTAT_SUS bit is zero. In this case, we need to
> >> + * enable the channel directly. Otherwise, just set the CLRSUS
> >> + * bit.
> >> + */
> >> + val = rz_dmac_ch_readl(channel, CHSTAT, 1);
> >> + if (val & CHSTAT_SUS)
> >> + chctrl = CHCTRL_CLRSUS;
> >> + else
> >> + chctrl = CHCTRL_SETEN;
> >> + } else {
> >> + chctrl = CHCTRL_CLRSUS;
> >> + }
> >> +
> >> + rz_dmac_ch_writel(channel, chctrl, CHCTRL, 1);
> >>
> >> - channel->status &= ~BIT(status);
> >> + if (chctrl & CHCTRL_CLRSUS) {
> >> + ret = read_poll_timeout_atomic(rz_dmac_ch_readl, val,
> >> + !(val & CHSTAT_SUS), 1, 1024, false,
> >> + channel, CHSTAT, 1);
> >> + if (ret)
> >> + return ret;
> >> + }
> >> +
> >> + channel->status &= ~(BIT(status) |
> >> +BIT(RZ_DMAC_CHAN_STATUS_SYS_SUSPENDED));
> >>
> >> return 0;
> >> }
> >> @@ -1034,6 +1068,13 @@ static int rz_dmac_device_resume(struct dma_chan *chan)
> >> return rz_dmac_device_resume_set(channel,
> >> RZ_DMAC_CHAN_STATUS_PAUSED); }
> >>
> >> +static int rz_dmac_device_resume_internal(struct rz_dmac_chan
> >> +*channel) {
> >> + lockdep_assert_held(&channel->vc.lock);
> >> +
> >> + return rz_dmac_device_resume_set(channel,
> >> +RZ_DMAC_CHAN_STATUS_PAUSED_INTERNAL);
> >> +}
> >> +
> >> /*
> >> * -----------------------------------------------------------------------------
> >> * IRQ handling
> >> @@ -1438,6 +1479,131 @@ static void rz_dmac_remove(struct platform_device *pdev)
> >> pm_runtime_disable(&pdev->dev);
> >> }
> >>
> >> +static int rz_dmac_suspend_prepare(struct device *dev) {
> >> + struct rz_dmac *dmac = dev_get_drvdata(dev);
> >> +
> >> + for (unsigned int i = 0; i < dmac->n_channels; i++) {
> >> + struct rz_dmac_chan *channel = &dmac->channels[i];
> >> +
> >> + guard(spinlock_irqsave)(&channel->vc.lock);
> >> +
> >> + /* Wait for transfer completion, except in cyclic case. */
> >> + if (channel->status & BIT(RZ_DMAC_CHAN_STATUS_ENABLED) &&
> >> + !(channel->status & BIT(RZ_DMAC_CHAN_STATUS_CYCLIC)))
> >> + return -EAGAIN;
> >> + }
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static void rz_dmac_suspend_recover(struct rz_dmac *dmac) {
> >> + for (unsigned int i = 0; i < dmac->n_channels; i++) {
> >> + struct rz_dmac_chan *channel = &dmac->channels[i];
> >> +
> >> + guard(spinlock_irqsave)(&channel->vc.lock);
> >> +
> >> + if (!(channel->status & BIT(RZ_DMAC_CHAN_STATUS_CYCLIC)))
> >> + continue;
> >> +
> >> + if (!(channel->status & BIT(RZ_DMAC_CHAN_STATUS_PAUSED_INTERNAL)))
> >> + continue;
> >> +
> >> + rz_dmac_device_resume_internal(channel);
> >> + }
> >> +}
> >> +
> >> +static int rz_dmac_suspend(struct device *dev) {
> >> + struct rz_dmac *dmac = dev_get_drvdata(dev);
> >> + int ret;
> >> +
> >> + for (unsigned int i = 0; i < dmac->n_channels; i++) {
> >> + struct rz_dmac_chan *channel = &dmac->channels[i];
> >> +
> >> + guard(spinlock_irqsave)(&channel->vc.lock);
> >> +
> >> + if (!(channel->status & BIT(RZ_DMAC_CHAN_STATUS_CYCLIC)))
> >> + continue;
> >> +
> >> + if (!(channel->status & BIT(RZ_DMAC_CHAN_STATUS_PAUSED))) {
> >> + ret = rz_dmac_device_pause_internal(channel);
> >> + if (ret) {
> >> + dev_err(dev, "Failed to suspend channel %s\n",
> >> + dma_chan_name(&channel->vc.chan));
> >> + continue;
> >> + }
> >> + }
> >> +
> >> + channel->pm_state.nxla = rz_dmac_ch_readl(channel, NXLA, 1);
> >> + channel->status |= BIT(RZ_DMAC_CHAN_STATUS_SYS_SUSPENDED);
> >> + }
> >> +
> >> + pm_runtime_put_sync(dmac->dev);
> >> +
> >> + ret = reset_control_assert(dmac->rstc);
> >> + if (ret) {
> >> + pm_runtime_resume_and_get(dmac->dev);
> >> + rz_dmac_suspend_recover(dmac);
> >> + }
> >> +
> >
> >
> > This patch breaks, s2idle in RZ/G3L as it turns off DMA ACLK and IRQ's
> > are not routed to CPU for wakeup.
>
> Is this particular patch the one that explicitly breaks it? Is there any mainline PM support available
> for RZ/G3L? Can it be fixed along with the RZ/G3L support, if any, as I don't have the board to test
> it?
Maybe your TF-A is enabling DMAACLK during resume. Can you check that mean time, I will check what you have mentioned
Here?
Cheers,
Biju
Powered by blists - more mailing lists