On 05/20/2015 07:35 PM, Krzysztof Kozlowski wrote: > On 21.05.2015 09:16, Krzysztof Kozlowski wrote: >> On 21.05.2015 05:30, Dinh Nguyen wrote: >>> Hi Krzysztof, >>> >>> On 05/05/2015 02:22 PM, Dinh Nguyen wrote: >>>> On 05/05/2015 09:56 AM, Dinh Nguyen wrote: >>>>> On 05/04/2015 10:55 PM, Krzysztof Kozlowski wrote: >>>>>> 2015-05-05 4:52 GMT+09:00 Dinh Nguyen <dinguyen@xxxxxxxxxxxxxxxxxxxxx>: >>>>>>> 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. >>>>>>> >>>>>> >>>>>> You mean runtime PM suspend and resume or system sleep? During boot >>>>>> only the first one should happen. >>>>> >>>>> It's runtime PM suspend/resume. >>>>> >>>>>> >>>>>> Could you test the DMA with dmatest? Disable the DMA in UART and >>>>>> compile with CONFIG_DMATEST. Syntax for testing is here: >>>>>> Documentation/dmaengine/dmatest.txt >>>>>> >>>>> >>>>> # echo Y > /sys/module/dmatest/parameters/run >>>>> [ 93.143775] dmatest: Started 1 threads using dma0chan0 >>>>> [ 93.149227] pm_generic_runtime_resume >>>>> [ 93.153334] dmatest: Started 1 threads using dma0chan1 >>>>> [ 93.159380] dmatest: Started 1 threads using dma0chan2 >>>>> [ 93.165041] dmatest: Started 1 threads using dma0chan3 >>>>> [ 93.170280] dmatest: Started 1 threads using dma0chan4 >>>>> [ 93.175996] dmatest: Started 1 threads using dma0chan5 >>>>> [ 93.181642] dmatest: Started 1 threads using dma0chan6 >>>>> [ 93.188754] dmatest: dma0chan1-copy0: summary 10 tests, 0 failures >>>>> 282 iops 2008 KB/s (0) >>>>> [ 93.197091] dmatest: Started 1 threads using dma0chan7 >>>>> [ 93.199353] dmatest: dma0chan3-copy0: summary 10 tests, 0 failures >>>>> 297 iops 2260 KB/s (0) >>>>> [ 93.205407] dmatest: dma0chan0-copy0: summary 10 tests, 0 failures >>>>> 177 iops 1364 KB/s (0) >>>>> [ 93.215599] dmatest: dma0chan2-copy0: summary 10 tests, 0 failures >>>>> 196 iops 1450 KB/s (0) >>>>> [ 93.219994] dmatest: dma0chan4-copy0: summary 10 tests, 0 failures >>>>> 225 iops 1554 KB/s (0) >>>>> [ 93.224322] dmatest: dma0chan5-copy0: summary 10 tests, 0 failures >>>>> 231 iops 1948 KB/s (0) >>>>> [ 93.230065] dmatest: dma0chan6-copy0: summary 10 tests, 0 failures >>>>> 231 iops 1759 KB/s (0) >>>>> [ 93.231251] dmatest: dma0chan7-copy0: summary 10 tests, 0 failures >>>>> 298 iops 2331 KB/s (0) >>>>> [ 93.243523] pm_generic_runtime_suspend >>>>> root@socfpga_cyclone5:~# >>>>> >>>> >>>> If I run dmatest the 2nd time it fails. It does not look like >>>> amba_pm_runtime_resume() is getting called to turn on the clocks on the >>>> subsequent tries. >>>> >>> >>> >>> I managed to track this down the call dmaengine_terminate_all(), which >>> then calls into pl330_terminate_all(). So in pl330_terminate_all(), it >>> call _stop, which hits a infinite loop, UNTIL. But since the >>> amba_pm_runtime_resume() has not been called yet, the clock is turned >>> off. Thus, we're stuck in an infinite loop. >>> >>> I'm not sure what would be right approach to fix this? >> >> Good catch. I confirmed that device is not runtime resumed. I wonder why >> it works in my case (pl330 on Exynos4412)... >> >> Anyway I have an idea to fix it. I'll send a patch. > > I sent a patch. Could you test on your board and confirm that this fixes > the issue? > Thanks alot for the patch! It does indeed fixes the issue for me. 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
