Hi Geert,
> From: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>
> Sent: 06 March 2025 13:41
> Subject: RE: [PATCH v5 2/6] dt-bindings: dma: rz-dmac: Document RZ/V2H(P) family of SoCs
>
> Hi Geert,
>
> Thanks for your feedback!
>
> > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > Sent: 06 March 2025 13:27
> > Subject: Re: [PATCH v5 2/6] dt-bindings: dma: rz-dmac: Document RZ/V2H(P) family of SoCs
> >
> > Hi Fabrizio,
> >
> > On Wed, 5 Mar 2025 at 01:21, Fabrizio Castro
> > <fabrizio.castro.jz@xxxxxxxxxxx> wrote:
> > > Document the Renesas RZ/V2H(P) family of SoCs DMAC block.
> > > The Renesas RZ/V2H(P) DMAC is very similar to the one found on the
> > > Renesas RZ/G2L family of SoCs, but there are some differences:
> > > * It only uses one register area
> > > * It only uses one clock
> > > * It only uses one reset
> > > * Instead of using MID/IRD it uses REQ No
> > > * It is connected to the Interrupt Control Unit (ICU)
> > >
> > > Signed-off-by: Fabrizio Castro <fabrizio.castro.jz@xxxxxxxxxxx>
> > > Acked-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> > > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@xxxxxxxxxxxxxx>
> > > ---
> > > v4->v5:
> > > * Removed ACK No from the specification of the dma cell.
> > > * I have kept the tags received as this is a minor change and the
> > > structure remains the same as v4. Please let me know if this is
> > > not okay.
> >
> > Thanks for the update!
> >
> > > --- a/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> > > +++ b/Documentation/devicetree/bindings/dma/renesas,rz-dmac.yaml
> > > @@ -61,14 +66,21 @@ properties:
> > > '#dma-cells':
> > > const: 1
> > > description:
> > > - The cell specifies the encoded MID/RID values of the DMAC port
> >
> > Please just insert "or the REQ No" and be done with it?
>
> Will do.
>
> >
> > > - connected to the DMA client and the slave channel configuration
> > > - parameters.
> > > + For the RZ/A1H, RZ/Five, RZ/G2{L,LC,UL}, RZ/V2L, and RZ/G3S SoCs, the cell
> > > + specifies the encoded MID/RID values of the DMAC port connected to the
> > > + DMA client and the slave channel configuration parameters.
> > > bits[0:9] - Specifies MID/RID 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)
> > > + For the RZ/V2H(P) SoC the cell specifies the DMAC REQ No and the slave channel
> > > + configuration parameters.
> > > + bits[0:9] - Specifies the DMAC REQ No
> > > + 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)
> >
> > ... so the casual reader doesn't have to look for the (nonexisting)
> > differences in the other bits.
>
> Agreed.
>
> >
> > >
> > > dma-channels:
> > > const: 16
> > > @@ -80,12 +92,29 @@ properties:
> > > items:
> > > - description: Reset for DMA ARESETN reset terminal
> > > - description: Reset for DMA RST_ASYNC reset terminal
> > > + minItems: 1
> > >
> > > reset-names:
> > > items:
> > > - const: arst
> > > - const: rst_async
> > >
> > > + renesas,icu:
> > > + description:
> > > + On the RZ/V2H(P) SoC configures the ICU to which the DMAC is connected to.
I'll take out this line, as the remainder of the description is sufficient enough I think,
and it'll be more generic.
> >
> > Are other SoCs with ICU planned?
>
> Yes.
> Also the DMACs on RZ/G3E are connected to the ICU in a similar fashion.
>
> >
> > > + It must contain the phandle to the ICU, and the index of the DMAC as seen
> > > + from the ICU (e.g. parameter k from register ICU_DMkSELy).
> >
> > This is already described more formally below
> >
> > > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > > + items:
> > > + - items:
> > > + - description: phandle to the ICU node.
> > > + - description: The DMAC index.
> > > + 4 for DMAC0
> > > + 0 for DMAC1
> > > + 1 for DMAC2
> > > + 2 for DMAC3
> > > + 3 for DMAC4
> >
> > Other SoCs may have other mappings.
> > So perhaps leave out the translation table, but write:
> >
> > The number of the DMAC as seen from the ICU, i.e. parameter k from
> > register ICU_DMkSELy.
> > This may differ from the actual DMAC instance number!
>
> Good shout, I will adjust accordingly.
>
> I'll wait for your feedback on the ICU driver patch and on the DMAC driver patch
> before sending v6.
>
> Thanks!
>
> Cheers,
> Fab
>
>
> >
> > > +
> > > required:
> > > - compatible
> > > - reg
> > > @@ -98,13 +127,25 @@ allOf:
> > > - $ref: dma-controller.yaml#
> > >
> > > - if:
> > > - not:
> > > - properties:
> > > - compatible:
> > > - contains:
> > > - enum:
> > > - - renesas,r7s72100-dmac
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + enum:
> > > + - renesas,r9a07g043-dmac
> > > + - renesas,r9a07g044-dmac
> > > + - renesas,r9a07g054-dmac
> > > + - renesas,r9a08g045-dmac
> > > then:
> > > + properties:
> > > + reg:
> > > + minItems: 2
> > > + clocks:
> > > + minItems: 2
> > > + resets:
> > > + minItems: 2
> > > +
> > > + renesas,icu: false
> > > +
> > > required:
> > > - clocks
> > > - clock-names
> > > @@ -112,13 +153,42 @@ allOf:
> > > - resets
> > > - reset-names
> > >
> > > - else:
> > > + - if:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + const: renesas,r7s72100-dmac
> > > + then:
> > > properties:
> >
> > reg:
> > minItems: 2
Sorry I missed this. Good point.
I'll add it.
Thanks!
Cheers,
Fab
> >
> > > clocks: false
> > > clock-names: false
> > > power-domains: false
> > > resets: false
> > > reset-names: false
> > > + renesas,icu: false
> > > +
> > > + - if:
> > > + properties:
> > > + compatible:
> > > + contains:
> > > + const: renesas,r9a09g057-dmac
> > > + then:
> > > + properties:
> > > + reg:
> > > + maxItems: 1
> > > + clocks:
> > > + maxItems: 1
> > > + resets:
> > > + maxItems: 1
> > > +
> > > + clock-names: false
> > > + reset-names: false
> > > +
> > > + required:
> > > + - clocks
> > > + - power-domains
> > > + - renesas,icu
> > > + - resets
> > >
> > > additionalProperties: false
> >
> > Gr{oetje,eeting}s,
> >
> > Geert
> >
> > --
> > Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx
> >
> > 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
