| Message ID | 20220421085112.78858-9-miquel.raynal@bootlin.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Geert Uytterhoeven |
| Headers | show |
| Series | RZN1 DMA support | expand |
Hi Miquel, On Thu, Apr 21, 2022 at 10:51 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > Describe the two DMA controllers available on this SoC. > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> Still, a few comments below, valid for both instances... > --- a/arch/arm/boot/dts/r9a06g032.dtsi > +++ b/arch/arm/boot/dts/r9a06g032.dtsi > @@ -200,6 +200,36 @@ nand_controller: nand-controller@40102000 { > status = "disabled"; > }; > > + dma0: dma-controller@40104000 { > + compatible = "renesas,r9a06g032-dma", "renesas,rzn1-dma"; > + reg = <0x40104000 0x1000>; > + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; > + clock-names = "hclk"; > + clocks = <&sysctrl R9A06G032_HCLK_DMA0>; > + dma-channels = <8>; > + dma-requests = <16>; > + dma-masters = <1>; > + #dma-cells = <3>; > + block_size = <0xfff>; > + data_width = <3>; This property is deprecated, in favor of "dma-width". > + status = "disabled"; Why not keep it enabled? > + }; 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
Hi Geert, geert@linux-m68k.org wrote on Mon, 25 Apr 2022 18:29:58 +0200: > Hi Miquel, > > On Thu, Apr 21, 2022 at 10:51 AM Miquel Raynal > <miquel.raynal@bootlin.com> wrote: > > Describe the two DMA controllers available on this SoC. > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > Still, a few comments below, valid for both instances... > > > --- a/arch/arm/boot/dts/r9a06g032.dtsi > > +++ b/arch/arm/boot/dts/r9a06g032.dtsi > > @@ -200,6 +200,36 @@ nand_controller: nand-controller@40102000 { > > status = "disabled"; > > }; > > > > + dma0: dma-controller@40104000 { > > + compatible = "renesas,r9a06g032-dma", "renesas,rzn1-dma"; > > + reg = <0x40104000 0x1000>; > > + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; > > + clock-names = "hclk"; > > + clocks = <&sysctrl R9A06G032_HCLK_DMA0>; > > + dma-channels = <8>; > > + dma-requests = <16>; > > + dma-masters = <1>; > > + #dma-cells = <3>; > > + block_size = <0xfff>; > > + data_width = <3>; > > This property is deprecated, in favor of "dma-width". Indeed, data_width = <3>; is deprecated. However, dma-width does not seem to be described anywhere. Do you mean: data-width = <8>; instead? > > > + status = "disabled"; > > Why not keep it enabled? I'm used to always disable all the nodes from the SoC descriptions, but it's true that for a DMA controller it might make sense to keep it enabled. Would dropping the status property be enough or do you prefer a proper status = "okay"; instead? Thanks a lot, Miquèl
Hi Miquel, On Wed, Apr 27, 2022 at 11:14 AM Miquel Raynal <miquel.raynal@bootlin.com> wrote: > geert@linux-m68k.org wrote on Mon, 25 Apr 2022 18:29:58 +0200: > > On Thu, Apr 21, 2022 at 10:51 AM Miquel Raynal > > <miquel.raynal@bootlin.com> wrote: > > > Describe the two DMA controllers available on this SoC. > > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > Still, a few comments below, valid for both instances... > > > > > --- a/arch/arm/boot/dts/r9a06g032.dtsi > > > +++ b/arch/arm/boot/dts/r9a06g032.dtsi > > > @@ -200,6 +200,36 @@ nand_controller: nand-controller@40102000 { > > > status = "disabled"; > > > }; > > > > > > + dma0: dma-controller@40104000 { > > > + compatible = "renesas,r9a06g032-dma", "renesas,rzn1-dma"; > > > + reg = <0x40104000 0x1000>; > > > + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; > > > + clock-names = "hclk"; > > > + clocks = <&sysctrl R9A06G032_HCLK_DMA0>; > > > + dma-channels = <8>; > > > + dma-requests = <16>; > > > + dma-masters = <1>; > > > + #dma-cells = <3>; > > > + block_size = <0xfff>; > > > + data_width = <3>; > > > > This property is deprecated, in favor of "dma-width". > > Indeed, > data_width = <3>; > is deprecated. > > However, dma-width does not seem to be described anywhere. Do you mean: > data-width = <8>; > instead? Oops, I did mean "data-width". > > > + status = "disabled"; > > > > Why not keep it enabled? > > I'm used to always disable all the nodes from the SoC descriptions, > but it's true that for a DMA controller it might make sense to keep > it enabled. > > Would dropping the status property be enough or do you prefer a proper > status = "okay"; > instead? Please just drop the status property, like is done in other Renesas .dtsi files. Thanks! 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
Hi Geert, geert@linux-m68k.org wrote on Wed, 27 Apr 2022 11:36:23 +0200: > Hi Miquel, > > On Wed, Apr 27, 2022 at 11:14 AM Miquel Raynal > <miquel.raynal@bootlin.com> wrote: > > geert@linux-m68k.org wrote on Mon, 25 Apr 2022 18:29:58 +0200: > > > On Thu, Apr 21, 2022 at 10:51 AM Miquel Raynal > > > <miquel.raynal@bootlin.com> wrote: > > > > Describe the two DMA controllers available on this SoC. > > > > > > > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com> > > > > Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > > > > Still, a few comments below, valid for both instances... > > > > > > > --- a/arch/arm/boot/dts/r9a06g032.dtsi > > > > +++ b/arch/arm/boot/dts/r9a06g032.dtsi > > > > @@ -200,6 +200,36 @@ nand_controller: nand-controller@40102000 { > > > > status = "disabled"; > > > > }; > > > > > > > > + dma0: dma-controller@40104000 { > > > > + compatible = "renesas,r9a06g032-dma", "renesas,rzn1-dma"; > > > > + reg = <0x40104000 0x1000>; > > > > + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; > > > > + clock-names = "hclk"; > > > > + clocks = <&sysctrl R9A06G032_HCLK_DMA0>; > > > > + dma-channels = <8>; > > > > + dma-requests = <16>; > > > > + dma-masters = <1>; > > > > + #dma-cells = <3>; > > > > + block_size = <0xfff>; > > > > + data_width = <3>; > > > > > > This property is deprecated, in favor of "dma-width". > > > > Indeed, > > data_width = <3>; > > is deprecated. > > > > However, dma-width does not seem to be described anywhere. Do you mean: > > data-width = <8>; > > instead? > > Oops, I did mean "data-width". > > > > > + status = "disabled"; > > > > > > Why not keep it enabled? > > > > I'm used to always disable all the nodes from the SoC descriptions, > > but it's true that for a DMA controller it might make sense to keep > > it enabled. > > > > Would dropping the status property be enough or do you prefer a proper > > status = "okay"; > > instead? > > Please just drop the status property, like is done in other Renesas .dtsi > files. Sure. Thanks, Miquèl
diff --git a/arch/arm/boot/dts/r9a06g032.dtsi b/arch/arm/boot/dts/r9a06g032.dtsi index 636a6ab31c58..839580ec21ee 100644 --- a/arch/arm/boot/dts/r9a06g032.dtsi +++ b/arch/arm/boot/dts/r9a06g032.dtsi @@ -200,6 +200,36 @@ nand_controller: nand-controller@40102000 { status = "disabled"; }; + dma0: dma-controller@40104000 { + compatible = "renesas,r9a06g032-dma", "renesas,rzn1-dma"; + reg = <0x40104000 0x1000>; + interrupts = <GIC_SPI 56 IRQ_TYPE_LEVEL_HIGH>; + clock-names = "hclk"; + clocks = <&sysctrl R9A06G032_HCLK_DMA0>; + dma-channels = <8>; + dma-requests = <16>; + dma-masters = <1>; + #dma-cells = <3>; + block_size = <0xfff>; + data_width = <3>; + status = "disabled"; + }; + + dma1: dma-controller@40105000 { + compatible = "renesas,r9a06g032-dma", "renesas,rzn1-dma"; + reg = <0x40105000 0x1000>; + interrupts = <GIC_SPI 57 IRQ_TYPE_LEVEL_HIGH>; + clock-names = "hclk"; + clocks = <&sysctrl R9A06G032_HCLK_DMA1>; + dma-channels = <8>; + dma-requests = <16>; + dma-masters = <1>; + #dma-cells = <3>; + block_size = <0xfff>; + data_width = <3>; + status = "disabled"; + }; + gic: interrupt-controller@44101000 { compatible = "arm,gic-400", "arm,cortex-a7-gic"; interrupt-controller;