From patchwork Sat Feb 13 11:24:43 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Martin Sperl X-Patchwork-Id: 8299841 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by patchwork2.web.kernel.org (Postfix) with ESMTP id B8177C02AA for ; Sat, 13 Feb 2016 11:38:18 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id A14E720434 for ; Sat, 13 Feb 2016 11:38:17 +0000 (UTC) Received: from bombadil.infradead.org (unknown [198.137.202.9]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 3C8F12042C for ; Sat, 13 Feb 2016 11:38:16 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1aUYKN-00070G-Ma; Sat, 13 Feb 2016 11:25:27 +0000 Received: from 212-186-180-163.dynamic.surfer.at ([212.186.180.163] helo=cgate.sperl.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1aUYJv-0006es-0D; Sat, 13 Feb 2016 11:25:00 +0000 Received: from msmac.intern.sperl.org (account martin@sperl.org [10.10.10.11] verified) by sperl.org (CommuniGate Pro SMTP 6.1.2) with ESMTPSA id 6393443; Sat, 13 Feb 2016 11:24:35 +0000 Subject: Re: serial: clk: bcm2835: Strange effects when using aux-uart in console Mime-Version: 1.0 (Mac OS X Mail 8.2 \(2104\)) From: Martin Sperl X-Priority: 3 In-Reply-To: <470852136.382408.76a8faee-9235-4ee0-9fd1-e25a1e9f0bb3.open-xchange@email.1und1.de> Date: Sat, 13 Feb 2016 12:24:43 +0100 Message-Id: References: <56BC895E.2010108@sperl.org> <1617009200.323700.219000a8-c162-4594-aacc-a7e9894518b2.open-xchange@email.1und1.de> <662D2FB2-B088-4CFC-8416-A4F35BBAD7DD@sperl.org> <6CD3FC59-BA17-43D9-897C-A2D122D26EF8@martin.sperl.org> <470852136.382408.76a8faee-9235-4ee0-9fd1-e25a1e9f0bb3.open-xchange@email.1und1.de> To: Stefan Wahren X-Mailer: Apple Mail (2.2104) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20160213_032459_460116_F213D5C9 X-CRM114-Status: GOOD ( 22.87 ) X-Spam-Score: -0.9 (/) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: linux-arm-kernel@lists.infradead.org, Eric Anholt , linux-clk , linux-serial@vger.kernel.org, linux-rpi-kernel Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-4.2 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP > On 13.02.2016, at 11:01, Stefan Wahren wrote: > > Hi Martin, > >> Martin Sperl hat am 12. Februar 2016 um 20:44 >> geschrieben: >> >> So the issue is triggered as soon as the plld_per >> pll divider gets disabled/reenabled. >> >> This happens because the clk_hw.core.prepare_count >> drops down to 0 and then unprepare is called. >> >> So we need to increase the ref-count for the pll >> and pll_dividers to at least 1 so that these never >> get disabled - at least for now until we can come >> up with a better contract with the firmware. >> >> Obviously this may impact other drivers as well >> where a pll is used for the first time - if nothing >> else uses it and the clock gets released, then >> the clock would trigger a unprepare of the whole >> branch of the clock tree. >> >> The question is: how can we solve it in an acceptable >> manner? >> >> Do we need a driver that just holds a reference to >> those clocks? Or should we just prepare the clock >> after registering it in clk-bcm2835.c? >> >> As for why does this not show up when compiled in? >> It seems that in that case the amba_pl011 driver >> never gets removed and then probed again. >> >> This is possibly related to the optional use of DMA, >> with the amba-pl011 driver that retries the install, >> which is not supported on the bcm2835 - at least that >> is what the datasheet says. And DMA is (probably) not >> enabled during the early boot stages, so it does not >> fail once when it tries to register DMA. >> >> Thanks, >> Martin >> > > i think i must correct myself. The fixed apb should stay since there is no > dynamic replacement and a proper disabling of plld shouldn't cause this freeze. > > I have the suspicion the freeze is caused by a clock glitch or lock-up. > > Could you please revert your changes and apply the attached patch? Maybe we can > see more. I will give it a try. But to me it seems as if the disabling of PLLD produces a hickup in the firmware (which relies on PLLD-DIV and thus PLL). This would also explain the 3-4 second bar/blanking pattern seen on HDMI, which I would guess is related to a reset loop... I got a simple fix for now, that just prepares a few clocks/pll/pll_divs during probing, so that they will never get unprepared/fully stopped): And that resolves the issue and also allows loading/unloading of the amba-pl011 module as well without any hickups. I guess it would be better if we did make this configurable in the DT. (See patch 1 of my RFC patchset, which could get extended to handle this as well - at least a portion thereof). But in more general terms: we need to come up with some agreement on ownership of the clocks between linux and the firmware and how to change them without impacting the other. One example is the “overclocking” that currently happens for downstream kernels in the firmware, which impacts some of the plls and PLLdivs as well as the sram control registers. The reason for this seems to be that for overclocking the SRAM parameters also need to get changed and that can only get done reliably when running the code from L1/L2 caches - and that requirement of not touching SRAM while changing the SRAM parameters is probably hard to achieve cleanly on ARM alone. Ciao, Martin P.s: unloading amba_pl011 gives the following error: root@raspcm:~# rmmod amba_pl011 [ 142.318263] Trying to free nonexistent resource <0000000020201000-0000000020201fff> but that is an issue not related to clock... diff --git a/drivers/clk/bcm/clk-bcm2835.c b/drivers/clk/bcm/clk-bcm2835.c index 015e687..fe0c401 100644 --- a/drivers/clk/bcm/clk-bcm2835.c +++ b/drivers/clk/bcm/clk-bcm2835.c @@ -37,6 +37,7 @@ * generator). */ +#include #include #include #include @@ -1504,6 +1505,7 @@ static int bcm2835_clk_probe(struct platform_device *pdev) struct clk **clks; struct bcm2835_cprman *cprman; struct resource *res; + int ret; cprman = devm_kzalloc(dev, sizeof(*cprman), GFP_KERNEL); if (!cprman) @@ -1597,6 +1599,37 @@ static int bcm2835_clk_probe(struct platform_device *pdev clks[BCM2835_CLOCK_PWM] = bcm2835_register_clock(cprman, &bcm2835_clock_pwm_data); + /* + * Several PLLs, PLL-dividers and clocks need to get prepared + * so that they are never unprepared and released. + * We run this separately as there may be dependencies that would + * need to be fullfilled first... + * Note: Especially unpreparing PLLD_PER will kill the system + * (even if it is prepared again almost immediately + * the HDMI display will fail) + */ +#define CLK_PREPARE_INITIAL(clkid) \ + if (clks[clkid] && (ret = clk_prepare(clks[clkid]))) \ + dev_err(dev, "could not prepare clock %pC - %d\n", \ + clks[clkid], ret); + CLK_PREPARE_INITIAL(BCM2835_PLLA); + CLK_PREPARE_INITIAL(BCM2835_PLLB); + CLK_PREPARE_INITIAL(BCM2835_PLLC); + CLK_PREPARE_INITIAL(BCM2835_PLLD); + CLK_PREPARE_INITIAL(BCM2835_PLLH); + CLK_PREPARE_INITIAL(BCM2835_PLLA_CORE); + CLK_PREPARE_INITIAL(BCM2835_PLLA_PER); + CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE0); + CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE1); + CLK_PREPARE_INITIAL(BCM2835_PLLC_CORE2); + CLK_PREPARE_INITIAL(BCM2835_PLLC_PER); + CLK_PREPARE_INITIAL(BCM2835_PLLD_CORE); + CLK_PREPARE_INITIAL(BCM2835_PLLD_PER); + CLK_PREPARE_INITIAL(BCM2835_PLLH_RCAL); + CLK_PREPARE_INITIAL(BCM2835_PLLH_AUX); + CLK_PREPARE_INITIAL(BCM2835_PLLH_PIX); + CLK_PREPARE_INITIAL(BCM2835_CLOCK_VPU); + return of_clk_add_provider(dev->of_node, of_clk_src_onecell_get, &cprman->onecell); }