On Sat, Sep 20, 2014 at 5:59 PM, Maxime Ripard <maxime.ripard@xxxxxxxxxxxxxxxxxx> wrote: > On Wed, Sep 17, 2014 at 12:01:46AM +0800, Chen-Yu Tsai wrote: >> On Tue, Sep 16, 2014 at 11:48 PM, Maxime Ripard >> <maxime.ripard@xxxxxxxxxxxxxxxxxx> wrote: >> > On Fri, Sep 12, 2014 at 10:10:25AM +0800, Chen-Yu Tsai wrote: >> >> On Fri, Sep 12, 2014 at 5:15 AM, Maxime Ripard >> >> <maxime.ripard@xxxxxxxxxxxxxxxxxx> wrote: >> >> > On Sat, Sep 06, 2014 at 06:47:27PM +0800, Chen-Yu Tsai wrote: >> >> >> The DMA controller requires AHB1 bus clock to be clocked from PLL6. >> >> >> >> >> >> Signed-off-by: Chen-Yu Tsai <wens@xxxxxxxx> >> >> >> --- >> >> >> arch/arm/boot/dts/sun6i-a31.dtsi | 5 +++++ >> >> >> 1 file changed, 5 insertions(+) >> >> >> >> >> >> diff --git a/arch/arm/boot/dts/sun6i-a31.dtsi b/arch/arm/boot/dts/sun6i-a31.dtsi >> >> >> index 8eb2c6d..1117989 100644 >> >> >> --- a/arch/arm/boot/dts/sun6i-a31.dtsi >> >> >> +++ b/arch/arm/boot/dts/sun6i-a31.dtsi >> >> >> @@ -317,6 +317,11 @@ >> >> >> clocks = <&ahb1_gates 6>; >> >> >> resets = <&ahb1_rst 6>; >> >> >> #dma-cells = <1>; >> >> >> + >> >> >> + /* DMA controller requires AHB1 clocked from PLL6 */ >> >> >> + assigned-clocks = <&ahb1>; >> >> >> + assigned-clock-parents = <&pll6>; >> >> >> + assigned-clock-rates = <200000000>; >> >> > >> >> > Where did you get that from? >> >> > >> >> > The user manual says that it should be clocked at 600MHz, and I'm not >> >> > sure it should be enforced there either. >> >> >> >> The bindings mean that ahb1 should be clocked from pll6 and at 200 MHz, >> >> not "pll6 should be 200 MHz". I assume you were misled by them. >> >> >> >> Clocking ahb1 from pll6 and at 200 MHz with the /3 pre-divider is the >> >> vendor BSP default: >> >> >> >> On sun6i, the clock init code calls aw_ccu_switch_ahb_2_pll6(), which muxes >> >> ahb1 from pll6 with the highest dividers, then sets the rate for ahb1 to >> >> pll6, which sets pre-divider to /3 and divider to /1. >> >> >> >> Hope this clears it up. :) >> > >> > It does, thanks :) >> > >> > But still, I find it the wrong place to enforce such a limit. This >> > should go into the clock driver itself. The DMA controller requires >> > such a parenting, but it doesn't require any specific rate, this is >> > more something of the global system policy. >> >> But I am guessing the AHB bus clock cannot be clock at any arbitrary >> rate. Just muxing without setting a specific clock rate might result >> in a hang. We could address this with per clock limitations, like the >> patch you posted earlier. Not sure if there's anything in the kernel >> like that at the moment, or how the clock framework handles clock rates >> after re-parenting. > > There's the Tomeu patch series that allows to set clock rate > requirements from the user side, I guess that could be easily extended > to integrate some clock side boundaries. That's why I stopped working > on the sun5i MMC clock issue until that patch is merged. I could take a look, but AFAIK none of our drivers need to propagate clock rate changes. >> Or we could just specify in the clock driver, or in the DT via clock >> supplier defaults, that the AHB bus should be clocked from PLL6 @ 200 MHz. >> >> The latter seems easier to do. > > IIRC, there was some opposition to having the muxing assignment in the > clock driver. > >> Also, I tested the DMA controller using dmatest on my A31 Hummingbird >> without this re-parenting patch. It worked fine. How did your tests fail? > > It was just timeouting, and actually, it was only working to the SRAM, > and not the DRAM. > > Maybe the bootloader sets up the clock muxing already? I just reran my tests on the A31, and dmatest indeed passes. I'm using your A31 sdcard.img, replaced u-boot with one built from u-boot-sunxi, so it supports DT. I also checked both the clock framework and the raw registers. AHB1 is indeed clocked from AXI on my system. Full log: https://gist.github.com/wens/ae17aed6728b83c0a9c3 If you (or someone else with A31) can verify the results, maybe we can just drop this patch. Cheers ChenYu -- To unsubscribe from this list: send the line "unsubscribe dmaengine" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
