On 05/04/2015 09:06 AM, Dinh Nguyen wrote: > +CC Olof > > On 5/4/15 8:50 AM, Krzysztof Kozlowski wrote: >> 2015-05-04 22:28 GMT+09:00 Dinh Nguyen <dinguyen@xxxxxxxxxxxxxxxxxxxxx>: >>> Hi Krzystof, >>> >>> On 5/4/15 12:30 AM, Krzysztof Kozlowski wrote: >>>> 2015-05-04 13:28 GMT+09:00 <dinguyen@xxxxxxxxxxxxxxxxxxxxx>: >>>>> From: Dinh Nguyen <dinguyen@xxxxxxxxxxxxxxxxxxxxx> >>>>> >>>>> Turn on the clock to the PL330 DMA if there is a clock node provided. >>>> >>>> Why? There is no explanation in the patch for this important question - why? >>>> >>>> Amba bus already does this and provide a wrapper function. >>>> Additionally that would mess up with runtime PM and clock >>>> enable/disable. >>> >>> I don't see the clock for the DMA getting turned on at all, which is why >>> after the kernel has booted, the filesystem tries to open up a serial >>> port using DMA and the system hangs. The failure is seen here: >>> >>> http://arm-soc.lixom.net/bootlogs/next/next-20150504/socfpga-arm-multi_v7_defconfig.html >> >> Thanks! >> >> The amba bus and pl330 should enable the clock and then disable it >> after probing: >> static int amba_probe(struct device *dev) >> { >> ... >> ret = amba_get_enable_pclk(pcdev); >> ... >> >> I wonder why do you think it is not enabled at all? > > I've checked it down to the register level that the gate for this clock > does not get set. > >> >>> >>> This only happens with the multi_v7_defconfig, because the PL330 DMA is >>> getting built into the kernel, while the socfpga_defconfig does not >>> enable the PL330. >> >> It makes sense. If pl330 driver is not enabled then necessary clocks >> are turned on by bootloader. Probing pl330 effectively disables the >> clock (if DMA is not used). >> >>> The DTS for the socfpga platform looks like this: >>> >>> pdma: pdma@ffe01000 { >>> compatible = "arm,pl330", "arm,primecell"; >>> reg = <0xffe01000 0x1000>; >>> interrupts = <0 104 4>, >>> <0 105 4>, >>> ... >>> #dma-cells = <1>; >>> #dma-channels = <8>; >>> #dma-requests = <32>; >>> clocks = <&l4_main_clk>; >>> clock-names = "apb_pclk"; >>> }; >>> >>> Perhaps I have the wrong designation for clock-names and the amba bus is >>> not able to pick up the correct clock? >> >> I have two ideas: >> 1. Is this really the clock for the DMA? If DMA is not used then >> disabling it should be OK. > > Yes, this is the clock for the DMA. Yeah, leaving this clock off is > fine, until the DMA gets used. Up until v4.0, SoCFPGA was not using the > DMA at all, but in v4.0, there was a patch to assign the UARTs to it's > DMA channel. > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/arch/arm/boot/dts/socfpga.dtsi?id=78c03c7af89721bd8a4428408a8cc7b53972e4b8 > >> 2. Disabling the clock may effectively disable its parent or >> grandparent if there are not more users. Maybe some other driver needs >> these parents to be enabled? This was the issue for at least one >> similar error (on Exynos boards). >> > > I'll check up on these issues. When I was debugging this issue, the > l4_main_clk is only used by the DMA, so it was not getting turned on by > an other drivers. > Ah, it looks like perhaps there's a problem with the serial driver and suspend/resume? If disable CONFIG_PM, then the DMA seems to be working fine with the debug uart. It appears the DMA is getting suspended and doesn't get resumed. Dinh -- 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