From patchwork Tue Apr 24 14:38:41 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Dmitry Osipenko X-Patchwork-Id: 10360165 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0CF8A60225 for ; Tue, 24 Apr 2018 14:42:15 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E906D28DF3 for ; Tue, 24 Apr 2018 14:42:14 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E3D4B28DFA; Tue, 24 Apr 2018 14:42:14 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, DKIM_VALID, FREEMAIL_FROM, MAILING_LIST_MULTI autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 47B932900A for ; Tue, 24 Apr 2018 14:41:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:Date: Message-ID:From:References:To:Subject:Reply-To:Content-ID:Content-Description :Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=jLfooDZF4kItQdbxqnQQh7MrLkgCyp3Y9N1JwQG5blQ=; b=ROnWNxFVDCoMTb iEiy3QKKj+q1lG+Kl28bKMiGCzff6EkTosmNYOkpPpza3fd6amx2llmblahwdADD0bd3+wNqabnT9 Ib9EjF1s5h4m7gIpgHBT3MA7jwZup/l45L7PUIXBYfLM2TfGxDEa2ZkmhsoK9DnGhMJCxbcvrak2S UvfEJu/7CYDwAGq3gXKX7hZ+wNTSWaNGqkT1CzgT60pxyFu9NxPFQBCw4C7fNN3X5xsZfWDnNb5wR 4s8gFwxsnWh0dCBAEoWUMbFID2Nm3zFNg+iOXmHrG/ZaRTGGhAW1kX+sLWuXpouJgiNQOFEHv+3bX ZoqOgygoAnK1zLr1JbHw==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1fAz88-0001iy-8c; Tue, 24 Apr 2018 14:41:16 +0000 Received: from mail-lf0-x22d.google.com ([2a00:1450:4010:c07::22d]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1fAz5t-0007oz-SL for linux-arm-kernel@lists.infradead.org; Tue, 24 Apr 2018 14:39:19 +0000 Received: by mail-lf0-x22d.google.com with SMTP id p142-v6so20693933lfd.6 for ; Tue, 24 Apr 2018 07:38:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=subject:to:cc:references:from:openpgp:autocrypt:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=HaSRY2a0c3OLGC/4APThWiFFXgpd6gMrGSqdy26Xa/o=; b=QDIp2sAlzXDobb/Y/Rwr2XeGdwk4ZsLgJj8oJhdZUxeLXNO9tNx6GzsqPbrP/7ui30 AkeaqvtJpbQCeh53jLHsT9JwEkYwJ4Fq+/oYRpn7LfZzEZX8EyUA6KUNZ7Kni8Up0UxN w0Xn09fgjov+/S66TMsnQi2ILins/vbsvON5Zczkz+Ez3cDhkkq7OzisQcWK9mS5RUL4 ZDHZkBTg23SbiQOtEgsAPkFPhco95BzSgbcvFhHvCMXa20NOQANYjUQ2QN/ZH+joBnQo POv1lB6fxnlIlpdPqXwqZByFUUawE31aplRg7KaL1z/fu3rsxRN2wJqxgRfi3M3nFD8J 9Xow== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:cc:references:from:openpgp:autocrypt :message-id:date:user-agent:mime-version:in-reply-to :content-language:content-transfer-encoding; bh=HaSRY2a0c3OLGC/4APThWiFFXgpd6gMrGSqdy26Xa/o=; b=uDm//kPJSYxoW6ZHsNmf+/If6LV7FGZCNUCpL2MSKWyhJgw1FKmsIi4n60TLPAcFrE JD7CoFOkYn2UXtfzr186Bx2EzZr13qkZIV0HbXuce1te7rxx+1MVbsT2v06/JLHwr7k1 MW/CB9VSCTuEbp/VIkXt03hzh63I68A23bI56YwFmuqbT+gc2N5xUW923XahJkOk01BW opLAJhKowMe3sLu6RWKqLZiN+GOpldDu5cJzm9VImEiYzOUFo48cj33YjCB+0WDk2Qf1 WOKUVzLN+emvX7uCGrLsHhSIWU1a74lqfuN2d+t6AVXrUTaBxfEkxFwbYqSgvMGiaO2a V3vA== X-Gm-Message-State: ALQs6tBfgzAjnAYQRIMsQZKgIIKtYXIhKDLMjuFpZP6w+u+G1AhzdVqx MIQ8kjxhOJJ+qlPsO56I/IE= X-Google-Smtp-Source: AB8JxZqFelefAxGyiY7Ihhw6WB0IDAgGqOOm1k1NY2X08TnVzTJJln5hLXjjdyNchTDtaGNXqVf6QA== X-Received: by 2002:a19:d015:: with SMTP id h21-v6mr11362255lfg.124.1524580724689; Tue, 24 Apr 2018 07:38:44 -0700 (PDT) Received: from [192.168.1.145] (ppp109-252-91-130.pppoe.spdop.ru. [109.252.91.130]) by smtp.googlemail.com with ESMTPSA id o82sm559361lja.67.2018.04.24.07.38.43 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 24 Apr 2018 07:38:43 -0700 (PDT) Subject: Re: [PATCH] ARM: tegra: fix ulpi regression on tegra20 To: Marcel Ziswiler , "marvin24@gmx.de" , Peter De Schrijver , Thierry Reding , Jon Hunter References: <20180219151252.29289-1-marcel@ziswiler.com> <6600596.BijQW1iq1K@ax5200p> <4f2ac009-8618-4b4d-e137-a5fd4907a56f@gmail.com> <1524521136.4493.70.camel@toradex.com> From: Dmitry Osipenko Openpgp: preference=signencrypt Autocrypt: addr=digetx@gmail.com; prefer-encrypt=mutual; keydata= xsBNBFpX5TwBCADQhg+lBnTunWSPbP5I+rM9q6EKPm5fu2RbqyVAh/W3fRvLyghdb58Yrmjm KpDYUhBIZvAQoFLEL1IPAgJBtmPvemO1XUGPxfYNh/3BlcDFBAgERrI3BfA/6pk7SAFn8u84 p+J1TW4rrPYcusfs44abJrn8CH0GZKt2AZIsGbGQ79O2HHXKHr9V95ZEPWH5AR0UtL6wxg6o O56UNG3rIzSL5getRDQW3yCtjcqM44mz6GPhSE2sxNgqureAbnzvr4/93ndOHtQUXPzzTrYB z/WqLGhPdx5Ouzn0Q0kSVCQiqeExlcQ7i7aKRRrELz/5/IXbCo2O+53twlX8xOps9iMfABEB AAHNIkRtaXRyeSBPc2lwZW5rbyA8ZGlnZXR4QGdtYWlsLmNvbT7CwJQEEwEIAD4WIQSczHcO 3uc4K1eb3yvTNNaPsNRzvAUCWlflPAIbAwUJA8JnAAULCQgHAgYVCgkICwIEFgIDAQIeAQIX gAAKCRDTNNaPsNRzvFjTCACqAh1M9/YPq73/ai5h2ExDquTgJnjegL8KL2yHL3G+XINwzN5E nPI7esoYm+zVWDJbv3UuRqylpookLNSRA01yyvkaMcipB/B128UnqmUiGRqezj9QE20yIauo uHRuwHPE2q+UkfUhRX9iuOaEyQtZDiCa0myMjmRkJ+Z8ZetclEPG8dYZu47w04phuMlu1QAt a0gkZOaMKvXgj21ushALS6nYnvm7HiIPQXfnEXThartatRvFdmbG4PCn0IoICkQBizwJtXrL HEjELIFap0M8krVJlUoZTFaZnaZkGpUDWikeFtAuie2KuIxmVBYPM4X7pM3eP3AVvIPGS7EE UUFuzsBNBFpX5TwBCADFNDou220thijaLLGaQsebWjzc/gPRxMixIpk856MRyRaQin+IbGD6 YskMb5ZSD3nS88LIKNfY4MMH0LwfYztI++ICG2vdFLkbBt78E+LqEa+kZ9072l4W5KO3mWQo +jMfxXbpgGlc7iuEReDgl8iyZ27r51kSW665CYvvu2YJhLqgdj6QM1lN2D1UnhEhkkU+pRAj 1rJVOxdfJaQNQS4+204p3TrURovzNGkN/brqakpNIcqGOAGQqb8F0tuwwuP7ERq/BzDNkbdr qJOrVC/wkHRq1jfabQczWKf8MwYOvivR3HY8d3CpSQxmUXDtdOWfg0XGm1dxYnVfqPjuJaZt ABEBAAHCwHwEGAEIACYWIQSczHcO3uc4K1eb3yvTNNaPsNRzvAUCWlflPAIbDAUJA8JnAAAK CRDTNNaPsNRzvJzuB/9d+sxcwHbO8ZDcgaLX9N+bXFqN9fIRVmBUyWa+qqTSREA4uVAtYcRT lfPE2OQ7aMFxaYPwo+/z5SLpu8HcEhN/FG9uIkfYwK0mdCO0vgvlfvBJm4VHe7C6vyAeEPJQ DKbBvdgeqFqO+PsLkk2sawF/9sontMJ5iFfjNDj4UeAo4VsdlduTBZv5hHFvIbv/p7jKH6OT 90FsgUSVbShh7SH5OzAcgqSy4kxuS1AHizWo6P3f9vei987LZWTyhuEuhJsOfivDsjKIq7qQ c5eR+JJtyLEA0Jt4cQGhpzHtWB0yB3XxXzHVa4QUp00BNVWyiJ/t9JHT4S5mdyLfcKm7ddc9 Message-ID: Date: Tue, 24 Apr 2018 17:38:41 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0 MIME-Version: 1.0 In-Reply-To: <1524521136.4493.70.camel@toradex.com> Content-Language: en-US X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20180424_073858_055142_8E5986D0 X-CRM114-Status: GOOD ( 42.65 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: "mark.rutland@arm.com" , "devicetree@vger.kernel.org" , "linux@armlinux.org.uk" , "linux-kernel@vger.kernel.org" , "robh+dt@kernel.org" , "thierry.reding@gmail.com" , "linux-tegra@vger.kernel.org" , "jonathanh@nvidia.com" , "linux-arm-kernel@lists.infradead.org" Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP Hi Marcel, On 24.04.2018 01:05, Marcel Ziswiler wrote: > Hi Dmitry > > On Fri, 2018-04-20 at 13:50 +0300, Dmitry Osipenko wrote: >> ... >> I managed to find CDEV clocks in TRM this time. > > And where exactly in which TRM? In all my attempts at finding anything > CDEV2 is just a pad group which includes the DAP_MCLK2 pad in question. That's the DEV2 clock you're talking about below. >> Seems assigning CDEV2 clock to >> "ulpi-link" was correct > > Sorry, but I do really not see how you can get to any such conclusion. > > Whatever that CDEV2 clock exactly is. Its offset 93 points towards the > CLK_RST_CONTROLLER_CLK_OUT_ENB_U_0 register which has CLK_ENB_DEV2_OUT > at bit position 29 reading "Enable clock to DEV2 pad". While the TRM > misses further documenting what exactly that DEV2 pad should be I guess > it may point towards our suspected DAP_MCLK2. So for some reason > besides PLL_P_OUT4 which is what that pad is actually muxed to also > some additional DEV2 pad clock needs enabling. In addition there would > be a CLK_RST_CONTROLLER_MISC_CLK_ENB_0 register where one could also > specify a DEV2_OSC_DIV_SEL but I would assume that one only applies if > the pad in question being muxed to OSC which is not the case at least > in all device trees I have looked at. > >> and both CDEV2 and PLL_P_OUT4 should be enabled, > > Agreed, basically the DAP_MCLK2 pad seems gated by an additional enable > called CLK_ENB_DEV2_OUT. > >> CDEV2 >> should gate the PLL_P_OUT4 that feeds USB HW and PLL_P_OUT4 should be >> always-enabled because it is enabled by init_table, but apparently it >> is getting >> disabled erroneously. > > At least that was the case back when I had trouble getting ULPI to work > on T20. Strangely latest -next right now does no longer seem to suffer > from that same issue even if my patch is reverted but as mentioned > before nobody stops the required PLL_P_OUT4 which is what is actually > driving DAP_MCLK2 to not be changed behind the scenes breaking the > whole thing again. > >> Marcel, could you please revert your patch, add >> "trace_event=clk_enable,clk_disable,clk_set_parent tp_printk" to >> kernels cmdline >> and post the log? > > Yes, the only difference in those traces is whether or not that non- > existent CDEV2 clock is enabled or not: > > [ 1.897521] clk_enable: cdev2_fixed > [ 1.901008] clk_enable: cdev2 > > I also do have an explanation why it kept working in my case. Probably > the boot ROM or U-Boot is already setting that bit. > >> It looks like there is some clk framework bug, > > My conclusion is that there should be a DAP_MCLK2 clock which is gated > by that CLK_ENB_DEV2_OUT but really outputs a T20 internal clock > according to its pin muxing which if set to OSC may be further divided > down by DEV2_OSC_DIV_SEL. Could be that DEV2_OSC_DIV_SEL is only relevant when DAP_MCLK2 is mux'ed to OSC. Maybe Peter could clarify everything. >> but just in case please also try >> to apply this patch: >> >> diff --git a/drivers/clk/tegra/clk-tegra-periph.c >> b/drivers/clk/tegra/clk-tegra-periph.c >> index 2acba2986bc6..407bd0c0ac2f 100644 >> --- a/drivers/clk/tegra/clk-tegra-periph.c >> +++ b/drivers/clk/tegra/clk-tegra-periph.c >> @@ -1024,7 +1024,7 @@ static void __init init_pllp(void __iomem >> *clk_base, void >> __iomem *pmc_base, >> if (dt_clk) { >> clk = >> tegra_clk_register_pll_out("pll_p_out4", >> "pll_p_out4_div", clk_base + >> PLLP_OUTB, >> - 17, 16, CLK_IGNORE_UNUSED | >> + 17, 16, CLK_IS_CRITICAL | >> CLK_SET_RATE_PARENT, 0, >> &PLLP_OUTB_lock); >> *dt_clk = clk; > > I did not have to do any such but maybe that would at least prevent any > further issues on PLL_P_OUT4. However I still believe this is kind of > wrong as PLL_P_OUT4 is what drives DAP_MCLK2 in its current muxing > which is connected to the ULPI transceivers REFCLK pin. > > BTW: That pin is usually to be driven at 24 MHz and not 26 MHz which > CDEV2 claims to be at. Pin is driven by the PLL_P_OUT4, which is set to 24 MHz via the init_table. The clock tree is invalid in that regards because Tegra's clock driver defines non-existent "cdev2_fixed" 26 MHz clock source and sets it as a parent for the "cdev2" clock, while "cdev2" should be a gate (and maybe divider?) for the real parent clock (PLL_P_OUT4 in our case). > What do you think? It looks to me that a clock signal coming from the mux'ed CDEV2 pin is routed to the DEV2 of CLK controller (which can gate it) and then it goes out to DAP_MCLK2. PLL_P_OUT4 ---> PINMUX_CDEV2 ---> CLK_DEV2 ---> DAP_MCLK2 But it also could be that CLK_ENB_DEV2_OUT is indeed some internal pinmux-related clock-gate. I think Peter should have a clue. Anyway the implementation details do not really matter for us, what matters is that PLL_P_OUT4 clk and DEV2 clk-gate must be enabled. Seems indeed ideally would be to have DAP_MCLK2 for the USB controller's "ulpi-link" clock and then DEV2 will be enable bit for the DAP_MCLK2. But also information about parent clock of the DAP_MCLK2 needs to be conducted to the clk driver via DT, that probably would require backward-incompatible change of the binding which is undesirable. One tolerable solution could be to remove fake "cdev2_fixed" clock in the driver and hardcode PLL_P_OUT4 for the parent of "cdev2". (Note that cdev1 also has a fake parent) index 0ee56dd04cec..4708653dedeb 100644 Now the real problem is that PLL_P_OUT4 was getting disabled. We had CDEV2 clock in the DT for "ulpi-link" and PLL_P_OUT4 was permanently enabled (supposedly) by the Tegra's clock driver using init_table. Apparently PLL_P_OUT4 was disabled on SCLK re-parenting, as you mentioned in the commit description. This should be considered as a bug because one of two purposes (permanently enable and setup default clock rate) of the init_table is defeated. Could you please try to bisect to the point when erroneous PLL_P_OUT4 disable issue is fixed->broken? It is quite important to know what commit changed the behaviour. If it was some common clk issue that got fixed - that should be a good case for us, if it was some unrelated change that obscured the issue - that's not good and we should go back and fix the real bug. --- a/drivers/clk/tegra/clk-tegra20.c +++ b/drivers/clk/tegra/clk-tegra20.c @@ -838,8 +838,7 @@ static void __init tegra20_periph_clk_init(void) clks[TEGRA20_CLK_CDEV1] = clk; /* cdev2 */ - clk = clk_register_fixed_rate(NULL, "cdev2_fixed", NULL, 0, 26000000); - clk = tegra_clk_register_periph_gate("cdev2", "cdev2_fixed", 0, + clk = tegra_clk_register_periph_gate("cdev2", "pll_p_out4", 0, clk_base, 0, 93, periph_clk_enb_refcnt); clks[TEGRA20_CLK_CDEV2] = clk;