Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754284AbbEUAgF (ORCPT ); Wed, 20 May 2015 20:36:05 -0400 Received: from mailout1.w1.samsung.com ([210.118.77.11]:44734 "EHLO mailout1.w1.samsung.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753328AbbEUAgB (ORCPT ); Wed, 20 May 2015 20:36:01 -0400 X-AuditID: cbfec7f5-f794b6d000001495-72-555d286efc1e Message-id: <555D286C.2000901@samsung.com> Date: Thu, 21 May 2015 09:35:56 +0900 From: Krzysztof Kozlowski User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.7.0 MIME-version: 1.0 To: Dinh Nguyen Cc: dan.j.williams@intel.com, Dinh Nguyen , vinod.koul@intel.com, dmaengine@vger.kernel.org, linux-kernel@vger.kernel.org, olof Johansson Subject: Re: [PATCH] dmaengine: pl300: enable the clock to PL330 dma References: <1430713734-12175-1-git-send-email-dinguyen@opensource.altera.com> <55477413.2020908@opensource.altera.com> <55477CEC.1050600@opensource.altera.com> <5547CDEF.5050003@opensource.altera.com> <5548DA1B.2080604@opensource.altera.com> <5549187E.7050802@opensource.altera.com> <555CEEDA.4050309@opensource.altera.com> <555D23E5.7060200@samsung.com> In-reply-to: <555D23E5.7060200@samsung.com> Content-type: text/plain; charset=windows-1252 Content-transfer-encoding: 7bit X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrMLMWRmVeSWpSXmKPExsVy+t/xa7p5GrGhBtNP8llMn3qB0WLVpzXM FitPalusnvqX1eL1C0OLy7vmsFmcuv6ZzeJl334WBw6PnbPusnss3vOSyePKiSZWj/5rk9g9 +rasYvT4vEkugC2KyyYlNSezLLVI3y6BK2Px2yVsBYstK/re7mdpYJym18XIwSEhYCLxdrF7 FyMnkCkmceHeerYuRi4OIYGljBJ/+5czQzhPGSUWzP7MClLFK6AlMe/SAjYQm0VAVeJfywxG EJtNwFhi8/IlYHFRgQiJt5dPMkHUC0r8mHyPBcQWETCQ+PT4NAvIUGaBLYwSG/+vBBsqLOAi sfFcKxPEtiMsEof3d4N1cwpoS1zbPo0F5FRmAT2J+xe1QMLMAvISm9e8ZZ7AKDALyY5ZCFWz kFQtYGRexSiaWppcUJyUnmukV5yYW1yal66XnJ+7iRES9F93MC49ZnWIUYCDUYmHd4NKbKgQ a2JZcWXuIUYJDmYlEd5z62NChXhTEiurUovy44tKc1KLDzFKc7AoifPO3PU+REggPbEkNTs1 tSC1CCbLxMEp1cB4Pe3AHnGRVmbfr1LJao/F58S326/mkGplkOT5Yci77Ed6mKKZZgjbDIX5 KY+53afHxr3ctlr+bU/55Y1uJoKB3NeUIj4s3L3+4qrQmL6apMU586Tc3eZEs11/9vMfwwp3 2/R9Nr8qw5ap5yw95S72RVDVXuDZyvBNK1QuOt9qWeF+cGsZU6sSS3FGoqEWc1FxIgDK4umz dgIAAA== Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 7406 Lines: 175 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 : >>>>>> 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 : >>>>>>>>> Hi Krzystof, >>>>>>>>> >>>>>>>>> On 5/4/15 12:30 AM, Krzysztof Kozlowski wrote: >>>>>>>>>> 2015-05-04 13:28 GMT+09:00 : >>>>>>>>>>> From: Dinh Nguyen >>>>>>>>>>> >>>>>>>>>>> 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? Best regards, Krzysztof -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/