RE: [PATCH 04/22] dt-bindings: dma: renesas,rz-dmac: Document optional DMA ACK cell

[John Madieu]

Hi Geert,

Thanks for your review.

> -----Original Message-----
> From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> Sent: Thursday, March 26, 2026 4:28 PM
> To: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> Subject: Re: [PATCH 04/22] dt-bindings: dma: renesas,rz-dmac: Document
> optional DMA ACK cell
> 
> Hi John,
> 
> On Thu, 19 Mar 2026 at 16:55, John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> wrote:
> > Some peripherals on RZ/V2H, RZ/V2N, and RZ/G3E SoCs require explicit
> > ACK signal routing through the ICU. Document the optional second cell
> > in the DMA specifier for specifying the ACK signal number.
> >
> > The first cell remains unchanged and specifies the encoded MID/RID and
> > channel configuration. The optional second cell specifies the DMA ACK
> > signal number for peripherals requiring level-based handshaking.
> >
> > Signed-off-by: John Madieu <john.madieu.xa@xxxxxxxxxxxxxx>
> 
> Thanks for your patch!
> 
> Just a quick head-up, as I haven't read the actual secion in the
> documentation yet.
> 
> > --- a/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> > +++ b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> > @@ -63,17 +63,27 @@ properties:
> >        - const: register
> >
> >    '#dma-cells':
> > -    const: 1
> > -    description:
> > +    description: |
> >        The cell specifies the encoded MID/RID or the REQ No values of
> >        the DMAC port connected to the DMA client and the slave channel
> >        configuration parameters.
> > +      Use 1 cell for basic DMA configuration.
> > +      Use 2 cells when DMA ACK signal routing through ICU is required
> > +      (RZ/V2H, RZ/V2N, RZ/G3E audio peripherals such as SSIU, SPDIF,
> SRC, DVC).
> > +
> > +      First cell:
> >        bits[0:9] - Specifies the MID/RID or the REQ No value
> >        bit[10] - Specifies DMA request high enable (HIEN)
> >        bit[11] - Specifies DMA request detection type (LVL)
> >        bits[12:14] - Specifies DMAACK output mode (AM)
> >        bit[15] - Specifies Transfer Mode (TM)
> >
> > +      Second cell (optional, when #dma-cells = <2>):
> > +      bits[6:0] - DMA acknowledge signal number (from ICU ACK table),
> > +                  where 0 is a valid signal number.
> > +                  Required for peripherals using level-based DMA
> > +                  handshaking (SSIU, SPDIF, RSPI, SCU, ADC, PDM).
> 
> How do you expect this to work? #dma-cells applies to all DMA consumers of
> this provider, and these SoCs already have DMA users relying on #dma-cells
> being one.

Indeed.

> In addition, you cannot have optional cells: if #dma-cells is two, then
> all consumers must supply two cells (of course we could switch all of them
> to two cells at once).  However, as zero is a valid signal number, we
> cannot use that as a dummy when no DMA acknowledge signal number is needed
> (we could use e.g. 0xffffffff instead).
> 
> Is there any other way to provide this information?
> E.g. could we have a table in the driver that contains this info for the
> (presumably few) MID/RID values that need it?
> 

There are actually 89 entries, and I could identify 3 peripheral
group with linear ACK assignments. Thus instead of static array
we would get a simple function handling 3 req_no ranges.

Something like:

/*
 * Map MID/RID request number (bits[0:9] of DMA specifier) to the ICU
 * DMA ACK signal number, per RZ/G3E hardware manual Table 4.6-28.
 *
 * Three peripheral groups with linear ACK assignment:
 *
 *   PFC external DMA pins (DREQ0..DREQ4):
 *     req_no 0x000-0x004 -> ACK No. 84-88  (ack = req_no + 84)
 *
 *   SSIU BUSIFs (ssip00..ssip93):
 *     req_no 0x161-0x198 -> ACK No. 28-83  (ack = req_no - 0x145)
 *
 *   SPDIF (CH0..CH2) + SCU SRC (sr0..sr9) + DVC (cmd0..cmd1):
 *     req_no 0x199-0x1b4 -> ACK No. 0-27   (ack = req_no - 0x199)
 */
static int rz_dmac_get_ack_no(const struct rz_dmac_info *info, u16 req_no)
{
	if (!info->icu_register_dma_ack)
		return -EINVAL;

	/* PFC external DMA pins: ACK No. 84-88 */
	if (req_no <= 0x004)
		return req_no + 84;

	/* SSIU BUSIFs: ACK No. 28-83 */
	if (req_no >= 0x161 && req_no <= 0x198)
		return req_no - 0x145;

	/* SPDIF + SCU SRC + DVC: ACK No. 0-27 */
	if (req_no >= 0x199 && req_no <= 0x1b4)
		return req_no - 0x199;

	return -EINVAL;
}

I'll then drop the current patch and update the req_no-related patch
with something like the above.

What do you think ?

Regards,
John.

> > +
> >    dma-channels:
> >      const: 16
> >
> > @@ -212,6 +222,20 @@ allOf:
> >          - renesas,icu
> >          - resets
> >
> > +  - if:
> > +      properties:
> > +        compatible:
> > +          contains:
> > +            const: renesas,r9a09g057-dmac
> > +    then:
> > +      properties:
> > +        '#dma-cells':
> > +          enum: [1, 2]
> > +    else:
> > +      properties:
> > +        '#dma-cells':
> > +          const: 1
> > +
> >    - if:
> >        properties:
> >          compatible:
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-
> m68k.org
> 
> In personal conversations with technical people, I call myself a hacker.
> But when I'm talking to journalists I just say "programmer" or something
> like that.
>                                 -- Linus Torvalds