Re: [PATCh v3 2/2] dma: sh: rz-dmac: Add DMA ACK signal routing support

[Frank Li <Frank.li@nxp.com>]

On Mon, May 11, 2026 at 05:04:46PM +0000, John Madieu wrote:
> Hi Frank,
>
> thanks for your review.
>
> > -----Original Message-----
> > From: Frank Li <Frank.li@nxp.com>
> > Sent: Donnerstag, 7. Mai 2026 20:49
> > To: John Madieu <john.madieu.xa@bp.renesas.com>
> > Subject: Re: [PATCh v3 2/2] dma: sh: rz-dmac: Add DMA ACK signal routing
> > support
> >
> >
> > On Thu, Apr 02, 2026 at 06:22:12PM +0200, John Madieu wrote:
> > > Some peripherals on RZ/G3E SoCs (SSIU, SPDIF, SCU/SRC, DVC, PFC)
> > > require explicit ACK signal routing through the ICU for level-based
> > > DMA handshaking.
> > >
> > > Rather than extending the DT binding with an optional second
> > > #dma-cells (which would require all DMA consumers to supply two cells
> > > even when ACK routing is not needed), derive the ACK signal number
> > > directly from the MID/RID request number using the linear mapping
> > > defined in RZ/G3E hardware manual Table 4.6-28:
> > >
> > >   PFC external DMA pins (DREQ0..DREQ4):
> > >     req_no 0x000-0x004 -> ACK No. 84-88
> > >
> > >   SSIU BUSIFs (ssip00..ssip93):
> > >     req_no 0x161-0x198 -> ACK No. 28-83
> > >
> > >   SPDIF (CH0..CH2) + SCU SRC (sr0..sr9) + DVC (cmd0..cmd1):
> > >     req_no 0x199-0x1b4 -> ACK No. 0-27
> > >
> > > ACK routing is programmed when a channel is prepared for transfer and
> > > cleared when the channel is released or the transfer times out,
> > > following the same pattern as MID/RID request routing.
> > >
> > > Signed-off-by: John Madieu <john.madieu.xa@bp.renesas.com>
> > > ---
> > >
> > > Changes:
> > >
> > > v3: No changes
> > >
> > > v2:
> > >  - Drop DMA ACK second cell from DT specifier
> > >  - Derive ACK signal number in-driver from MID/RID using arithmetic
> > formulas
> > >    per ICU Table 4.6-28 (3 linear peripheral groups)
> > >
> > >  drivers/dma/sh/rz-dmac.c | 72
> > > ++++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 72 insertions(+)
> > >
> > >  static void rz_dmac_prepare_desc_for_memcpy(struct rz_dmac_chan
> > > *channel)  {
> > >       struct dma_chan *chan = &channel->vc.chan; @@ -431,6 +489,7 @@
> > > static void rz_dmac_prepare_descs_for_slave_sg(struct rz_dmac_chan
> > *channel)
> > >       channel->lmdesc.tail = lmdesc;
> > >
> > >       rz_dmac_set_dma_req_no(dmac, channel->index, channel->mid_rid);
> > > +     rz_dmac_set_dma_ack_no(dmac, channel->index, channel->dmac_ack);
> >
> > I am not familar with your hardware, why ACK folllow req immediately?
> > suppose ACK happen after transfer done.
>
> rz_dmac_set_dma_ack_no() does not fire an ACK pulse, it programs a static
> routing mux in the ICU (ICU_DMACKSELk) that selects which DMAC channel is
> the source of the ACK line for a given peripheral. It is the symmetric
> counterpart of ICU_DMAREQSELk programmed by rz_dmac_set_dma_req_no().
>
> Both registers must be configured before any transfer can happen on
> the channel: the REQ mux routes the peripheral's request line into
> the DMAC, the ACK mux routes the DMAC's acknowledge line back to the
> peripheral. Once the routing is in place, the level-based REQ/ACK
> handshake itself runs entirely in hardware on every burst, with no
> driver involvement per transfer.

Supposed these register should belong to dma-engine, but it was located
into irq, you open a private API to irq chip drivers. Ideally these
register ownership should move to here.

these routing mux should be in allocate channel callback, not in
rz_dmac_prepare_descs_for_slave_sg() or in xlate function.

Frank

>
> Maybe should I reword the commit message to make this distinction
> explicit (routing config vs per-transfer signal).
>
> >
> > If ACK need after req, why not add ack handle in rz_dmac_set_dma_req_no()
> > directly.
>
> I would prefer to keep rz_dmac_set_dma_ack_no() as its own helper, to
> mirror the existing rz_dmac_set_dma_req_no() path. The surrounding
> infrastructure is already structured around per-routing helpers, and
> the ACK additions in this patch deliberately follow that pattern:
> .icu_register_dma_ack/.default_dma_ack_no.
>
> The two ICU registers also index their fields differently. ICU_DMAkSELy
> fields are indexed by DMAC channel and carry the peripheral req_no as
> the value, while ICU_DMACKSELk fields are indexed by the peripheral
> ack_no and carry the DMAC source channel as the value. Folding ACK
> into rz_dmac_set_dma_req_no() would mix those two layouts in one helper
> for no behavioral gain.
>
> Regards,
> John
>
>
> >
> > Frank
> >
> > > --
> > > 2.25.1
> > >