Re: [PATCH 5/7] dmaengine: sh: rz-dmac: Add suspend to RAM support

[Claudiu Beznea]

Hi, Biju,

On 2/5/26 19:41, Biju Das wrote:
> Hi Claudiu,
>
> > -----Original Message-----
> > From: Claudiu Beznea <claudiu.beznea@xxxxxxxxx>
> > Sent: 05 February 2026 17:21
> > Subject: Re: [PATCH 5/7] dmaengine: sh: rz-dmac: Add suspend to RAM support
> >
> > Hi, Biju,
> >
> > On 2/5/26 16:06, Biju Das wrote:
> > > Hi Geert,
> > >
> > > > -----Original Message-----
> > > > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > > > Sent: 05 February 2026 13:34
> > > > Subject: Re: [PATCH 5/7] dmaengine: sh: rz-dmac: Add suspend to RAM
> > > > support
> > > >
> > > > Hi Biju,
> > > >
> > > > On Thu, 5 Feb 2026 at 14:30, Biju Das <biju.das.jz@xxxxxxxxxxxxxx> wrote:
> > > > > > From: Claudiu Beznea <claudiu.beznea@xxxxxxxxx> On 1/26/26 17:28,
> > > > > > Biju Das wrote:
> > > > > > > > For s2idle issue on RZ/G3L is DMA device is in asserted state,
> > > > > > > > not forwarding any IRQ to cpu for wakeup.
> > > > > > > >
> > > > > > > > For S2RAM issue on RZ/G3L is during suspend hardware turns
> > > > > > > > DMAACLK off/ Asserted state. Clock framwork is not turning On DMAACLK as it critical clk.
> > > > > > > >
> > > > > > > > Can you please check your TF-A for the second case? First case,
> > > > > > > > RZ/G3S may ok for reset assert state, it can forward IRQs to CPU.
> > > > > > >
> > > > > > > Just to summarize, currently there are 2 differences identified between RZ/G3S and RZ/G3L:
> > > > > > >
> > > > > > > SoC differences for s2idle:
> > > > > > >
> > > > > > > RZ/G3S: Can wake the system if the DMA device is in the assert
> > > > > > > state
> > > > > > >
> > > > > > > RZ/G3L: Cannot wake the system if the DMA device is in the assert state.
> > > > > > >
> > > > > > >
> > > > > > > TF-A differences for s2ram:
> > > > > > >
> > > > > > > RZ/G3S: TF_A turns on DMA_ACLK during boot/resume.
> > > > > > >
> > > > > > > RZ/G3L: TF_A does not handle DMA_ACLK during boot/resume.
> > > > > >
> > > > > > I'm seeing at [1] you are addressing these differences in the
> > > > > > clock/reset drivers. With that, are you still considering this patch is breaking your system?
> > > > >
> > > > > Still, thinking whether to add critical reset or go with SoC quirk in DMA driver.
> > > > > Some SoCs need DMA should be deasserted like critical clock that can
> > > > > be handled either
> > > > >
> > > > > 1) Add a simple SoC quirk in DMA driver
> > > > >
> > > > > Or
> > > > >
> > > > > 2) Implement critical reset in SoC specific clock driver and check for all resets.
> > > > >
> > > > > Is simple SoC quirk in DMA driver, something can be done for RZ/G2L family SoCs?
> > > >
> > > > What if the DMA driver is not enabled?
> > >
> > > The below use cases will work (patch[1] - removing the code for
> > > deassert in cpg_resume) as there is no DMA driver to assert the reset.
> > >
> > > 1) system will boot without DMA driver
> > > 2) s2idle will work as there is no DMA driver to assert the reset.
> > > 3) s2ram will work without DMA driver.
> > >
> > > If DMA driver is enabled, then there is an issue with  s2idle as DMA
> > > driver assert the reset and we cannot use serial console as wakeup
> > > source
> >
> > I think we're taking here about both DMA clocks and resets.
> >
> > What if the DMA clocks are declared critical in Linux and clocks and resets are not handled by
> > bootloader in probe or resume? Who will restore critical clocks?
>
> Patch [1] will restore critical clocks.
> > > One solution is SoC quirk will prevent assert/deassert  of the DMA
> > > reset during
> > > suspend/resume() for affected SoCs.
> >
> > This can't work w/o taking care of the DMA clocks in the clock driver resume function (in case DMA
> > clocks are critical). If so, why handling DMA clocks and resets differently?
>
>
> What will you prefer
>
> a single check in suspend/resume of DMA driver?
>
> Or
>
> Around 100 checks in suspend/resume in clock driver for checking critical resets for skipping DMA reset?

I see no conditions in your code. Just raw writes for DMA clocks and resets. I 
suspect the intention for v2 is to loop over all the resets in the resume path 
to find the critical one.

While reviewing it I asked to avoid asserting the DMA resets on reset assert 
API. That could be handled either by adding the concept of critical assert in 
the reset driver (or framework) of by just checking directly the reset ID to 
match the DMA reset ID (as this is the only critical reset identified at the 
moment).

To answer you, my personal taste would be:
- to handle the setup of the critical clocks and resets in a single driver, for
  probe and suspend/resume as well
- to handle it in a SoC specific code as this is micro-architecture specific
  issue; this problem is only for some of the SoCs, if I'm not wrong; the
  manuals for some of the SoCs using this DMA driver states the following
  (RZ/G3S HW manual, Rev.1.20., chapter 8.8.1):

In addition, need following register settings *even if DMA controller is not used*.

● Set CPG_CLKON_DMAC_REG register to supply a clock for DMA Controller.

Refer to Section 7.2.4, Clock Control Register DMAC_REG for register detail.

● Set CPG_RST_DMAC register to release a reset for DMA Controller.

Refer to Section 7.2.4, Reset Control Register DMAC for register detail.

Geert, Vinod: could you please let us know how would you like us to handle this?

Thank you,
Claudiu