Am Montag, 11. August 2014, 10:01:08 schrieb Doug Anderson:
> Heiko,
>
> On Tue, Jul 29, 2014 at 12:12 PM, Heiko Stuebner <heiko@xxxxxxxxx> wrote:
> > Add both the bus and peripheral pl330 dma controllers present in rk3288
> > socs. The first dma controller can change between secure and non-secure
> > mode and is left by the bootloader in secure mode, which gets added here.
> >
> > Signed-off-by: Heiko Stuebner <heiko@xxxxxxxxx>
> > ---
> >
> > arch/arm/boot/dts/rk3288.dtsi | 27 +++++++++++++++++++++++++++
> > 1 file changed, 27 insertions(+)
>
> This is outside of my area of expertise, but comparing this to
> bindings, TRM, and other platforms it looks reasonable to me.
>
> > diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
> > index 3ef8951..3f39d26 100644
> > --- a/arch/arm/boot/dts/rk3288.dtsi
> > +++ b/arch/arm/boot/dts/rk3288.dtsi
> > @@ -62,6 +62,33 @@
> >
> > };
> >
> > };
> >
> > + amba {
> > + compatible = "arm,amba-bus";
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > + ranges;
> > +
> > + dmac1: dma@ffb20000 {
>
> I'm curious: why did you choose "dmac1" for the alias here? I would
> have called it "dmac_peri", "pdma", or something similar as per the
> address map in the TRM. I found a single reference in the TRM to
> "dmac2" but it wasn't immediately clear to me which DMA that was
> referring to. In a commit in a temporary tree I see that someone at
> Rockchip called the DMA at ff250000 "pdma1".
I think I just kept the alias similar to the clock-name. The naming of the
dma-controllers seems to be inconsistent ... the clocks are named dmac1 and
dmac2, while the interrupt table in the rk3188-TRM calls them dmac0 and dmac1.
So I guess I just duplicated the naming for the rk3288.
I guess for the rk3288, the one at 0xffb20000 should be named something like
dmac_bus and the other one dmac_peri, reflecting what they're called in the
TRM, but I'm of course open for suggestions :-) .
Also for an upcoming v2, I've also changed the structure a bit, as the first
dma-controller has both a secure and non-secure version of it.
So currently the rk3288.dtsi looks like [0]:
amba {
compatible = "arm,amba-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges;
/* dma1 in secure state */
dma-controller@ffb20000 {
compatible = "arm,pl330", "arm,primecell";
reg = <0xffb20000 0x4000>;
interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
#dma-cells = <1>;
clocks = <&cru ACLK_DMAC1>;
clock-names = "apb_pclk";
status = "disabled";
};
/* dma1 in non-secure state */
dma-controller@ffb60000 {
compatible = "arm,pl330", "arm,primecell";
reg = <0xffb60000 0x4000>;
interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
#dma-cells = <1>;
clocks = <&cru ACLK_DMAC1>;
clock-names = "apb_pclk";
status = "disabled";
};
dmac2: dma-controller@ff250000 {
compatible = "arm,pl330", "arm,primecell";
reg = <0xff250000 0x4000>;
interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
#dma-cells = <1>;
clocks = <&cru ACLK_DMAC2>;
clock-names = "apb_pclk";
};
};
and the board is responsible for enabling the correct variant [1], as most
likely the bootloader decides in which mode to start the dma controller:
amba {
/* dma1 in secure state */
dmac1: dma-controller@ffb20000 {
status = "okay";
};
};
This is based on some mailinglist discussion, I found at some point, about
this but for the life of me am not able to find anymore. So of course feedback
would appreciated there too.
[0] https://github.com/mmind/linux-rockchip/blob/devel/workbench/arch/arm/boot/dts/rk3288.dtsi
[1] https://github.com/mmind/linux-rockchip/blob/devel/workbench/arch/arm/boot/dts/rk3288-evb.dtsi
>
> > + compatible = "arm,pl330", "arm,primecell";
> > + reg = <0xffb20000 0x4000>;
> > + interrupts = <GIC_SPI 0 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> > + #dma-cells = <1>;
> > + clocks = <&cru ACLK_DMAC1>;
> > + clock-names = "apb_pclk";
> > + };
> > +
> > + dmac2: dma@ff250000 {
>
> nit: when we have easy addresses to compare to, we should probably
> sort by address?
correct :-)
>
> > + compatible = "arm,pl330", "arm,primecell";
> > + reg = <0xff250000 0x4000>;
> > + interrupts = <GIC_SPI 2 IRQ_TYPE_LEVEL_HIGH>,
> > + <GIC_SPI 3 IRQ_TYPE_LEVEL_HIGH>;
> > + #dma-cells = <1>;
> > + clocks = <&cru ACLK_DMAC2>;
> > + clock-names = "apb_pclk";
> > + };
> > + };
> > +
> >
> > xin24m: oscillator {
> >
> > compatible = "fixed-clock";
> > clock-frequency = <24000000>;
> >
> > --
> > 2.0.1
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
