RE: [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC support

From: Biju Das <biju.das.jz@bp.renesas.com>
To: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Rob Herring <robh+dt@kernel.org>,
	Magnus Damm <magnus.damm@gmail.com>,
	Linux-Renesas <linux-renesas-soc@vger.kernel.org>,
	"open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
	<devicetree@vger.kernel.org>,
	Chris Paterson <Chris.Paterson2@renesas.com>,
	Chris Brandt <Chris.Brandt@renesas.com>,
	Prabhakar Mahadev Lad <prabhakar.mahadev-lad.rj@bp.renesas.com>
Subject: RE: [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC support
Date: Mon, 14 Jun 2021 13:02:42 +0000	[thread overview]
Message-ID: <OS0PR01MB59227529257477835867FEFA86319@OS0PR01MB5922.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CAMuHMdUthmAbwNrBKeEBzqBUN6HYR=Fuz5ALpK+iGY_kUQpV1A@mail.gmail.com>

Hi Geert,

Thanks for the feedback.

> Subject: Re: [PATCH 4/5] arm64: dts: renesas: r9a07g044: Add DMAC support
> 
> Hi Biju,
> 
> On Fri, Jun 11, 2021 at 1:36 PM Biju Das <biju.das.jz@bp.renesas.com>
> wrote:
> > Add DMAC support to RZ/G2L SoC DT.
> >
> > Signed-off-by: Biju Das <biju.das.jz@bp.renesas.com>
> > Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
> 
> Thanks for your patch!
> 
> > --- a/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > +++ b/arch/arm64/boot/dts/renesas/r9a07g044.dtsi
> > @@ -8,6 +8,10 @@
> >  #include <dt-bindings/interrupt-controller/arm-gic.h>
> >  #include <dt-bindings/clock/r9a07g044-cpg.h>
> >
> > +#define CH_CFG(reqd, loen, hien, lvl, am, sds, dds, tm) \
> > +       ((((tm) << 22) | ((dds) << 16) | ((sds) << 12) | ((am) << 8) | \
> > +       ((lvl) << 6) | ((hien) << 5) | ((loen) << 4) | ((reqd) << 3))
> > +& 0x004FF778)
> > +
> 
> I assume the above will be removed?

Basically the macro simplifies the channel configuration values in Table 16.4 page 569 of the hardware manual.

Client driver will use MID+RID, and pass (Src address or dest address along with the channel configuration values
For configuring DMA channel.

For eg:-

                ssi0: ssi@10049c00 {
                        compatible = "renesas,r9a07g044-ssi",
                                     "renesas,rz-ssi";
                        reg = <0 0x10049c00 0 0x400>;
                        interrupts = <GIC_SPI 326 IRQ_TYPE_LEVEL_HIGH>,
                                     <GIC_SPI 327 IRQ_TYPE_EDGE_RISING>,
                                     <GIC_SPI 328 IRQ_TYPE_EDGE_RISING>;
                        interrupt-names = "int", "rx", "tx";
                        clocks = <&cpg CPG_MOD R9A07G044_CLK_SSI0>,
                                 <&audio_clk1>,
                                 <&audio_clk2>;
                        clock-names = "ssi", "audio_clk1", "audio_clk2";
                        resets = <&cpg R9A07G044_CLK_SSI0>;
                        dmas = <&dmac 0x255 0x10049c18 CH_CFG(0x1,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>,
                               <&dmac 0x256 0x10049c1c CH_CFG(0x0,0x0,0x1,0x0,0x2,0x1,0x1,0x0)>;
                        dma-names = "tx", "rx";
                        power-domains = <&cpg>;
                        #sound-dai-cells = <0>;
                        status = "disabled";
                };

Please let me know your thoughts on this.

Regards,
Biju

> >  / {
> >         compatible = "renesas,r9a07g044";
> >         #address-cells = <2>;
> > @@ -111,6 +115,40 @@
> >                         status = "disabled";
> >                 };
> >
> > +               dmac: dma-controller@11820000 {
> > +                       compatible = "renesas,dmac-r9a07g044",
> > +                                    "renesas,rz-dmac";
> > +                       reg = <0 0x11820000 0 0x10000>,
> > +                             <0 0x11830000 0 0x10000>;
> > +                       interrupts = <GIC_SPI 125 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 126 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 127 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 128 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 129 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 130 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 131 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 132 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 133 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 134 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 135 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 136 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 137 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 138 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 139 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 140 IRQ_TYPE_EDGE_RISING>,
> > +                                    <GIC_SPI 141 IRQ_TYPE_EDGE_RISING>;
> > +                       interrupt-names = "ch0", "ch1", "ch2", "ch3",
> > +                                         "ch4", "ch5", "ch6", "ch7",
> > +                                         "ch8", "ch9", "ch10", "ch11",
> > +                                         "ch12", "ch13", "ch14",
> "ch15",
> > +                                         "error";
> > +                       clocks = <&cpg CPG_MOD R9A07G044_CLK_DMAC>;
> > +                       power-domains = <&cpg>;
> > +                       resets = <&cpg R9A07G044_CLK_DMAC>;
> > +                       #dma-cells = <1>;
> > +                       dma-channels = <16>;
> > +               };
> > +
> >                 gic: interrupt-controller@11900000 {
> >                         compatible = "arm,gic-v3";
> >                         #interrupt-cells = <3>;
> 
> The rest looks good to me, so
> Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>
> 
> 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