| Message ID | 1401716531-29794-6-git-send-email-geert+renesas@glider.be (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Geert, Thank you for the patch. On Monday 02 June 2014 15:42:09 Geert Uytterhoeven wrote: > Add nodes for the SYS-DMA controllers, SYS-DMAC0 and SYS-DMAC1. These > both share the same device sources, so are wrapped in the shdma-mux > node to allow both to be used. > > Cfr. the r8a7790 version by Ben Dooks. > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > --- > v2: > - No changes > > arch/arm/boot/dts/r8a7791.dtsi | 69 +++++++++++++++++++++++++++++++++++++++ > 1 file changed, 69 insertions(+) > > diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi > index e5c35d784ec8..37685ef32ea0 100644 > --- a/arch/arm/boot/dts/r8a7791.dtsi > +++ b/arch/arm/boot/dts/r8a7791.dtsi > @@ -11,6 +11,7 @@ > */ > > #include <dt-bindings/clock/r8a7791-clock.h> > +#include <dt-bindings/dma/r8a7791-dma.h> > #include <dt-bindings/interrupt-controller/arm-gic.h> > #include <dt-bindings/interrupt-controller/irq.h> > > @@ -55,6 +56,74 @@ > }; > }; > > + dma0: dma-mux@0 { > + compatible = "renesas,shdma-mux"; > + #dma-cells = <2>; > + dma-channels = <30>; > + dma-requests = <256>; > + #address-cells = <2>; > + #size-cells = <2>; > + ranges; > + > + sysdma0: dma-controller@e6700020 { > + compatible = "renesas,shdma-r8a7791"; > + reg = <0 0xe6700020 0 0xffc0>; Even though the registered defined in the datasheet start at 0xe6700020, the documentation clearly states that "The base address of registers for the lower-numbered channels (0 to 14) is H'E670 0000". I'm also wondering where the size comes from, if you want to span the 0xe6700000 to 0xe670ffff range, the size should be 0xffe0, not 0xffc0. Interrupts and clocks look fine to me. > + clocks = <&mstp2_clks R8A7791_CLK_SYS_DMAC0>; > + dma-channels = <15>; > + interrupts = <0 197 IRQ_TYPE_LEVEL_HIGH>, /* error */ > + <0 200 IRQ_TYPE_LEVEL_HIGH>, /* ch0 */ > + <0 201 IRQ_TYPE_LEVEL_HIGH>, > + <0 202 IRQ_TYPE_LEVEL_HIGH>, > + <0 203 IRQ_TYPE_LEVEL_HIGH>, > + <0 204 IRQ_TYPE_LEVEL_HIGH>, > + <0 205 IRQ_TYPE_LEVEL_HIGH>, > + <0 206 IRQ_TYPE_LEVEL_HIGH>, > + <0 207 IRQ_TYPE_LEVEL_HIGH>, > + <0 208 IRQ_TYPE_LEVEL_HIGH>, > + <0 209 IRQ_TYPE_LEVEL_HIGH>, > + <0 210 IRQ_TYPE_LEVEL_HIGH>, > + <0 211 IRQ_TYPE_LEVEL_HIGH>, > + <0 212 IRQ_TYPE_LEVEL_HIGH>, > + <0 213 IRQ_TYPE_LEVEL_HIGH>, > + <0 214 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "error", > + "ch0", "ch1", "ch2", "ch3", > + "ch4", "ch5", "ch6", "ch7", > + "ch8", "ch9", "ch10", "ch11", > + "ch12", "ch13", "ch14"; > + status = "disabled"; > + }; > + > + sysdma1: dma-controller@e6720020 { > + compatible = "renesas,shdma-r8a7791"; > + reg = <0 0xe6720020 0 0xffc0>; > + clocks = <&mstp2_clks R8A7791_CLK_SYS_DMAC1>; > + dma-channels = <15>; > + interrupts = <0 220 IRQ_TYPE_LEVEL_HIGH>, > + <0 216 IRQ_TYPE_LEVEL_HIGH>, > + <0 217 IRQ_TYPE_LEVEL_HIGH>, > + <0 218 IRQ_TYPE_LEVEL_HIGH>, > + <0 219 IRQ_TYPE_LEVEL_HIGH>, > + <0 308 IRQ_TYPE_LEVEL_HIGH>, > + <0 309 IRQ_TYPE_LEVEL_HIGH>, > + <0 310 IRQ_TYPE_LEVEL_HIGH>, > + <0 311 IRQ_TYPE_LEVEL_HIGH>, > + <0 312 IRQ_TYPE_LEVEL_HIGH>, > + <0 313 IRQ_TYPE_LEVEL_HIGH>, > + <0 314 IRQ_TYPE_LEVEL_HIGH>, > + <0 315 IRQ_TYPE_LEVEL_HIGH>, > + <0 316 IRQ_TYPE_LEVEL_HIGH>, > + <0 317 IRQ_TYPE_LEVEL_HIGH>, > + <0 318 IRQ_TYPE_LEVEL_HIGH>; > + interrupt-names = "error", > + "ch0", "ch1", "ch2", "ch3", > + "ch4", "ch5", "ch6", "ch7", > + "ch8", "ch9", "ch10", "ch11", > + "ch12", "ch13", "ch14"; > + status = "disabled"; > + }; > + }; > + > gic: interrupt-controller@f1001000 { > compatible = "arm,cortex-a15-gic"; > #interrupt-cells = <3>;
Hi Laurent > > Add nodes for the SYS-DMA controllers, SYS-DMAC0 and SYS-DMAC1. These > > both share the same device sources, so are wrapped in the shdma-mux > > node to allow both to be used. > > > > Cfr. the r8a7790 version by Ben Dooks. > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > --- (snip) > > + sysdma0: dma-controller@e6700020 { > > + compatible = "renesas,shdma-r8a7791"; > > + reg = <0 0xe6700020 0 0xffc0>; > > Even though the registered defined in the datasheet start at 0xe6700020, the > documentation clearly states that "The base address of registers for the > lower-numbered channels (0 to 14) is H'E670 0000". I'm also wondering where > the size comes from, if you want to span the 0xe6700000 to 0xe670ffff range, > the size should be 0xffe0, not 0xffc0. Unfortunately, this mapping (form 0x20) is required from driver for historical reasons. So, driver setting needs to care about it. Please check DMA_CHANNEL macro for each setup-xxx.c # but, it works without care about it (?) # becase there are mirror register there (?), I'm not sure detail Best regards --- Kuninori Morimoto
Hi Morimoto-san, On Wednesday 04 June 2014 17:17:53 Kuninori Morimoto wrote: > Hi Laurent > > > > Add nodes for the SYS-DMA controllers, SYS-DMAC0 and SYS-DMAC1. These > > > both share the same device sources, so are wrapped in the shdma-mux > > > node to allow both to be used. > > > > > > Cfr. the r8a7790 version by Ben Dooks. > > > > > > Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> > > > --- > > (snip) > > > > + sysdma0: dma-controller@e6700020 { > > > + compatible = "renesas,shdma-r8a7791"; > > > + reg = <0 0xe6700020 0 0xffc0>; > > > > Even though the registered defined in the datasheet start at 0xe6700020, > > the documentation clearly states that "The base address of registers for > > the lower-numbered channels (0 to 14) is H'E670 0000". I'm also > > wondering where the size comes from, if you want to span the 0xe6700000 > > to 0xe670ffff range, the size should be 0xffe0, not 0xffc0. > > Unfortunately, this mapping (form 0x20) is required from driver > for historical reasons. > So, driver setting needs to care about it. > Please check DMA_CHANNEL macro for each setup-xxx.c Right, but that doesn't mean we shouldn't fix that while we still can :-) For the r8a73a4 we're probably stuck (I won't reiterate here my view of considering DT bindings as stable when it's clear they haven't gone through proper review), but before we add support for new SoCs I'd like to see to problems being fixed. > # but, it works without care about it (?) > # becase there are mirror register there (?), I'm not sure detail
On 04/06/14 13:25, Laurent Pinchart wrote: > Hi Geert, > > Thank you for the patch. > > On Monday 02 June 2014 15:42:09 Geert Uytterhoeven wrote: >> Add nodes for the SYS-DMA controllers, SYS-DMAC0 and SYS-DMAC1. These >> both share the same device sources, so are wrapped in the shdma-mux >> node to allow both to be used. >> >> Cfr. the r8a7790 version by Ben Dooks. >> >> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> >> --- >> v2: >> - No changes >> >> arch/arm/boot/dts/r8a7791.dtsi | 69 +++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 69 insertions(+) >> >> diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi >> index e5c35d784ec8..37685ef32ea0 100644 >> --- a/arch/arm/boot/dts/r8a7791.dtsi >> +++ b/arch/arm/boot/dts/r8a7791.dtsi >> @@ -11,6 +11,7 @@ >> */ >> >> #include <dt-bindings/clock/r8a7791-clock.h> >> +#include <dt-bindings/dma/r8a7791-dma.h> >> #include <dt-bindings/interrupt-controller/arm-gic.h> >> #include <dt-bindings/interrupt-controller/irq.h> >> >> @@ -55,6 +56,74 @@ >> }; >> }; >> >> + dma0: dma-mux@0 { >> + compatible = "renesas,shdma-mux"; >> + #dma-cells = <2>; >> + dma-channels = <30>; >> + dma-requests = <256>; >> + #address-cells = <2>; >> + #size-cells = <2>; >> + ranges; >> + >> + sysdma0: dma-controller@e6700020 { >> + compatible = "renesas,shdma-r8a7791"; >> + reg = <0 0xe6700020 0 0xffc0>; > > Even though the registered defined in the datasheet start at 0xe6700020, the > documentation clearly states that "The base address of registers for the > lower-numbered channels (0 to 14) is H'E670 0000". I'm also wondering where > the size comes from, if you want to span the 0xe6700000 to 0xe670ffff range, > the size should be 0xffe0, not 0xffc0. There seems to be no registers below the 0x20 point and therefore all current implementations assume that the first register is the point for the start. 0xffc0 is wrong, should be 0xffe0.
diff --git a/arch/arm/boot/dts/r8a7791.dtsi b/arch/arm/boot/dts/r8a7791.dtsi index e5c35d784ec8..37685ef32ea0 100644 --- a/arch/arm/boot/dts/r8a7791.dtsi +++ b/arch/arm/boot/dts/r8a7791.dtsi @@ -11,6 +11,7 @@ */ #include <dt-bindings/clock/r8a7791-clock.h> +#include <dt-bindings/dma/r8a7791-dma.h> #include <dt-bindings/interrupt-controller/arm-gic.h> #include <dt-bindings/interrupt-controller/irq.h> @@ -55,6 +56,74 @@ }; }; + dma0: dma-mux@0 { + compatible = "renesas,shdma-mux"; + #dma-cells = <2>; + dma-channels = <30>; + dma-requests = <256>; + #address-cells = <2>; + #size-cells = <2>; + ranges; + + sysdma0: dma-controller@e6700020 { + compatible = "renesas,shdma-r8a7791"; + reg = <0 0xe6700020 0 0xffc0>; + clocks = <&mstp2_clks R8A7791_CLK_SYS_DMAC0>; + dma-channels = <15>; + interrupts = <0 197 IRQ_TYPE_LEVEL_HIGH>, /* error */ + <0 200 IRQ_TYPE_LEVEL_HIGH>, /* ch0 */ + <0 201 IRQ_TYPE_LEVEL_HIGH>, + <0 202 IRQ_TYPE_LEVEL_HIGH>, + <0 203 IRQ_TYPE_LEVEL_HIGH>, + <0 204 IRQ_TYPE_LEVEL_HIGH>, + <0 205 IRQ_TYPE_LEVEL_HIGH>, + <0 206 IRQ_TYPE_LEVEL_HIGH>, + <0 207 IRQ_TYPE_LEVEL_HIGH>, + <0 208 IRQ_TYPE_LEVEL_HIGH>, + <0 209 IRQ_TYPE_LEVEL_HIGH>, + <0 210 IRQ_TYPE_LEVEL_HIGH>, + <0 211 IRQ_TYPE_LEVEL_HIGH>, + <0 212 IRQ_TYPE_LEVEL_HIGH>, + <0 213 IRQ_TYPE_LEVEL_HIGH>, + <0 214 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "error", + "ch0", "ch1", "ch2", "ch3", + "ch4", "ch5", "ch6", "ch7", + "ch8", "ch9", "ch10", "ch11", + "ch12", "ch13", "ch14"; + status = "disabled"; + }; + + sysdma1: dma-controller@e6720020 { + compatible = "renesas,shdma-r8a7791"; + reg = <0 0xe6720020 0 0xffc0>; + clocks = <&mstp2_clks R8A7791_CLK_SYS_DMAC1>; + dma-channels = <15>; + interrupts = <0 220 IRQ_TYPE_LEVEL_HIGH>, + <0 216 IRQ_TYPE_LEVEL_HIGH>, + <0 217 IRQ_TYPE_LEVEL_HIGH>, + <0 218 IRQ_TYPE_LEVEL_HIGH>, + <0 219 IRQ_TYPE_LEVEL_HIGH>, + <0 308 IRQ_TYPE_LEVEL_HIGH>, + <0 309 IRQ_TYPE_LEVEL_HIGH>, + <0 310 IRQ_TYPE_LEVEL_HIGH>, + <0 311 IRQ_TYPE_LEVEL_HIGH>, + <0 312 IRQ_TYPE_LEVEL_HIGH>, + <0 313 IRQ_TYPE_LEVEL_HIGH>, + <0 314 IRQ_TYPE_LEVEL_HIGH>, + <0 315 IRQ_TYPE_LEVEL_HIGH>, + <0 316 IRQ_TYPE_LEVEL_HIGH>, + <0 317 IRQ_TYPE_LEVEL_HIGH>, + <0 318 IRQ_TYPE_LEVEL_HIGH>; + interrupt-names = "error", + "ch0", "ch1", "ch2", "ch3", + "ch4", "ch5", "ch6", "ch7", + "ch8", "ch9", "ch10", "ch11", + "ch12", "ch13", "ch14"; + status = "disabled"; + }; + }; + gic: interrupt-controller@f1001000 { compatible = "arm,cortex-a15-gic"; #interrupt-cells = <3>;
Add nodes for the SYS-DMA controllers, SYS-DMAC0 and SYS-DMAC1. These both share the same device sources, so are wrapped in the shdma-mux node to allow both to be used. Cfr. the r8a7790 version by Ben Dooks. Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be> --- v2: - No changes arch/arm/boot/dts/r8a7791.dtsi | 69 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+)