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

[John Madieu]

Hi Frank,

> -----Original Message-----
> From: Frank Li <Frank.li@xxxxxxx>
> Sent: Montag, 11. Mai 2026 22:21
> To: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> Subject: Re: [PATCh v3 2/2] dma: sh: rz-dmac: Add DMA ACK signal routing
> support
> 
> [You don't often get email from frank.li@xxxxxxx. Learn why this is
> important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> 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@xxxxxxx>
> > > Sent: Donnerstag, 7. Mai 2026 20:49
> > > To: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> > > 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@xxxxxxxxxxxxxx>
> > > > ---
> > > >
> > > > 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 registers belong to the ICU's MMIO region, along with the
IRQ/TINT/NMI/ECC machinery owned by the irqchip driver. The established
pattern is for the irqchip to keep ownership of the whole region and
export a small entry point for the DMA engine to call into. That pattern
was introduced by Fabrizio when the irqchip driver landed and adopted
by Cosmin when chip-specific REQ routing was added to rz-dmac [1].

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

The prepare-path placement is also pre-existing. The new
rz_dmac_setset_dma_ack_no() call in this patch sit at the same
sites, by design, so the REQ and ACK routes share one lifecycle.

Moving the routing setup to device_alloc_chan_resources() is a
reasonable direction, but it would only make sense if REQ and ACK
move together, and it would be a refactor of existing and recently
landed [1] REQ code in the same change. Should we go this way ?

[1] https://lore.kernel.org/all/20260105114445.878262-1-cosmin-gabriel.tanislav.xa@xxxxxxxxxxx/

Regards,
John