From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 49FF1C3DA4A for ; Mon, 29 Jul 2024 08:54:47 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: Content-Type:In-Reply-To:From:References:Cc:To:Subject:MIME-Version:Date: Message-ID:Reply-To:Content-ID:Content-Description:Resent-Date:Resent-From: Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=s0HhR+uV6rXGa7auJe5KTVeSyzHEcaR1Su3Fj7Gk8K0=; b=0xb6emC6ZSsczTXLDKJTLuY690 iKbfAYynKCe5eEljHJ8MQcixeXHv4KWLGEn0NVsHt5/PLapjHSNaES9dIx0kPOhrb0Qa2dywYzBvs bvVEFs402yf6M6M6TJDSDUXrvXigviWUhQM6jLKP/Dj5KDRKI/f5Mb6N9RAeZb1viGY/mN8w0/Ujk 3zOMjPBw2V6VALQdiRwrbbrpY3imoSGUTHJrepdMSIg0KwMIuROLQsrOAxAWRBuv/0tIzrIWMe+Wa DZEFG9lrfNwhBX4zhI6x6BdEQOZvnBHY8RNN27jMim4uFL/XQQsxARqit+qq2/d56rKclO2d0okY3 XpdXWFpw==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1sYM9T-0000000AcGG-3K1g; Mon, 29 Jul 2024 08:54:43 +0000 Received: from madrid.collaboradmins.com ([46.235.227.194]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1sYM92-0000000Ac9s-2dvB; Mon, 29 Jul 2024 08:54:18 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=collabora.com; s=mail; t=1722243253; bh=mtlAilVTPNs8lYp8w2hPGsvB8pLsy8XrVF7CDsDBCxI=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=u7HdGz6MSzyPAYP0xlb5hQAV2DXtnrDP4m+i1OiL5EtIynq0MzeVaXqPWeYc5L4Bf GUdO5IdhTj1FcJ8QM4d8eCgZbuHnCoJKUKVOcorYOACwS2Uzo8Qst48jMa3N1+b02J u8LMpAR8xbLjFdQzc/jiUddK7FQaV4+jPtkGfTEvVseYlitNicnlwNOhZrd+RQpIZb 6KzcrwIHlO4PE1h2jChsYnAEpV5oHEQCK0A5Ipow6aHUnTgyRl1xiQq6JfV8sSVFd6 PcQ6O8qgjZ0Nq/YKYpOUGGbNRTXkjnQz8XAoiEvL5IfJaABvh+qwj7T/7ha7htg/G6 DnN0cshChwIGA== Received: from [100.113.186.2] (cola.collaboradmins.com [195.201.22.229]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) (Authenticated sender: kholk11) by madrid.collaboradmins.com (Postfix) with ESMTPSA id ABCD13780BC9; Mon, 29 Jul 2024 08:54:12 +0000 (UTC) Message-ID: Date: Mon, 29 Jul 2024 10:54:11 +0200 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH] arm64: dts: mediatek: mt8195: Add missing clock for xhci1 controller To: Chen-Yu Tsai Cc: =?UTF-8?B?TsOtY29sYXMgRi4gUi4gQS4gUHJhZG8=?= , Matthias Brugger , Rob Herring , Krzysztof Kozlowski , Conor Dooley , kernel@collabora.com, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-mediatek@lists.infradead.org References: <20240722-usb-1129-probe-pci-clk-fix-v1-1-99ea804228b6@collabora.com> <51f0f4f3-11a5-4d74-981e-3f24f8475c7f@collabora.com> From: AngeloGioacchino Del Regno Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240729_015416_829614_A63FB81E X-CRM114-Status: GOOD ( 41.05 ) X-BeenThere: linux-mediatek@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "Linux-mediatek" Errors-To: linux-mediatek-bounces+linux-mediatek=archiver.kernel.org@lists.infradead.org Il 29/07/24 10:07, Chen-Yu Tsai ha scritto: > On Mon, Jul 29, 2024 at 3:59 PM AngeloGioacchino Del Regno > wrote: >> >> Il 26/07/24 17:11, Chen-Yu Tsai ha scritto: >>> On Fri, Jul 26, 2024 at 8:11 PM AngeloGioacchino Del Regno >>> wrote: >>>> >>>> Il 25/07/24 12:34, Chen-Yu Tsai ha scritto: >>>>> Hi, >>>>> >>>>> On Mon, Jul 22, 2024 at 11:27 PM Nícolas F. R. A. Prado >>>>> wrote: >>>>>> >>>>>> Currently if the xhci1 controller happens to probe before the pcie1 >>>>>> controller then it fails with the following errors: >>>>>> >>>>>> xhci-mtk 11290000.usb: clocks are not stable (0x1003d0f) >>>>>> xhci-mtk 11290000.usb: can't setup: -110 >>>>>> xhci-mtk: probe of 11290000.usb failed with error -110 >>>>>> >>>>>> The issue has been tracked down to the CLK_INFRA_AO_PCIE_P1_TL_96M >>>>>> clock, although exactly why this pcie clock is needed for the usb >>>>>> controller is still unknown. Add the clock to the xhci1 controller so it >>>>>> always probes successfully and use a placeholder clock name for it. >>>>>> >>>>>> Reported-by: Nícolas F. R. A. Prado #KernelCI >>>>>> Closes: https://lore.kernel.org/all/9fce9838-ef87-4d1b-b3df-63e1ddb0ec51@notapiano/ >>>>>> Signed-off-by: Nícolas F. R. A. Prado >>>>> >>>>> So I asked MediaTek about this, and it seems the correct thing to do is >>>>> disable USB 3 on this host controller using the following snippet. The >>>>> snippet is copy-pasted from our issue tracker and won't apply directly. >>>>> >>>>> This is also seen in mt8395-kontron-3-5-sbc-i1200.dts, on which xhci1 >>>>> is used only for USB 2.0 on an M.2 slot. >>>>> >>>> >>>> Uhm, okay, but why should USB3 be disabled on a controller that supports USB3? >>>> >>>> I agree about disabling it on specific boards that use only the USB 2.0 lines of >>>> this controller, but doing that globally looks wrong... and looks like being a >>>> workaround for an error that gets solved with adding a clock as well. >>>> >>>> In short, the question is: why would that be the correct thing to do? >>> >>> We can disable it in mt8195-cherry.dtsi then? >> >> That device does not use this controller, so yes we can disable it, but that still >> doesn't resolve the issue pointed out by Nicolas...! > > No. I mean disable USB3 on this port. Also see the next paragraph. > Yes, sorry I was meaning the same - but I effectively wrote "disable controller" instead, my bad. >> Please note that the issue that he sees doesn't happen only on Tomato, but also on >> other MediaTek MT8195/MT8395 boards - and applying this commit makes disabling the >> controller, or disabling the USB 3 lines on the controller, kinda redundant, as >> this will effectively fix probing it... but again, fixing the actual issue with >> this controller is something that must be done. > > If those other boards use the XHCI1 USB3 lines for ... USB3, then the USB3 > PHY should also be tied to XHCI1, right now due to the Cherry Chromebook > design, only the USB2 PHY is tied to it. > Yes, I am aware of that. >> Disabling the controller on Tomato is a different topic - here we are discussing >> about fixing the issue, and that will happen, again, on any board that has this >> controller enabled with USB3 lines. :-) >> >> So, unless there is any specific reason for which applying this commit is a bad >> idea, or any alternative fix to this that is better than the proposed one, and >> not a workaround... I'm applying this one. > > Didn't I just relay what MediaTek says is the correct fix? Disable USB3 > for this port on devices where the serial pairs are used for PCIe instead > of USB3. > I think there must've been some misunderstanding here. Yes you did relay what MediaTek is the correct fix, and I agree that the USB3 must be disabled on devices where those serial pairs are used for PCIe instead of USB, or on devices where those are completely unused. This, though, will fix the issue only on those devices (because we are disabling those lines entirely, so depending on how we see it, this might not be a fix but rather a workaround). If we don't apply this fix, any board that uses those pairs for USB 3 instead will still show the same "clocks are not stable" error, leaving them with a broken port. And I believe that because the clocks are not routed externally but rather are internal to the SoC, so, if INFRA_AO_PCIE_P1_TL_96M is necessary for that USB 3 port to work, a board that intends to use those pairs for USB3 would still need this exact clock to actually get that port to work. As for Tomato itself - I agree that we must add the u3p-dis-msk=0x1 flag, yes, and we will, but I'm purely talking about - again - an eventual board that would not have that property because USB3 is exposed/used for real. Cheers, Angelo > > Regards > ChenYu > >> Cheers, >> Angelo >> >>> >>> ChenYu >>> >>>> Cheers, >>>> Angelo >>>> >>>>> >>>>> ChenYu >>>>> >>>>> index 8b7307cdefc6..2dac9f706a58 >>>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi >>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi >>>>> @@ -1447,6 +1447,7 @@ >>>>> "xhci_ck"; >>>>> mediatek,syscon-wakeup = <&pericfg 0x400 104>; >>>>> wakeup-source; >>>>> + mediatek,u3p-dis-msk = <0x1>; >>>>> status = "disabled"; >>>>> }; >>>>> >>>>>> --- >>>>>> arch/arm64/boot/dts/mediatek/mt8195.dtsi | 10 ++++++++-- >>>>>> 1 file changed, 8 insertions(+), 2 deletions(-) >>>>>> >>>>>> diff --git a/arch/arm64/boot/dts/mediatek/mt8195.dtsi b/arch/arm64/boot/dts/mediatek/mt8195.dtsi >>>>>> index 2ee45752583c..cc5169871f1c 100644 >>>>>> --- a/arch/arm64/boot/dts/mediatek/mt8195.dtsi >>>>>> +++ b/arch/arm64/boot/dts/mediatek/mt8195.dtsi >>>>>> @@ -1453,9 +1453,15 @@ xhci1: usb@11290000 { >>>>>> <&topckgen CLK_TOP_SSUSB_P1_REF>, >>>>>> <&apmixedsys CLK_APMIXED_USB1PLL>, >>>>>> <&clk26m>, >>>>>> - <&pericfg_ao CLK_PERI_AO_SSUSB_1P_XHCI>; >>>>>> + <&pericfg_ao CLK_PERI_AO_SSUSB_1P_XHCI>, >>>>>> + /* >>>>>> + * This clock is required due to a hardware >>>>>> + * bug. The 'frmcnt_ck' clock name is used as a >>>>>> + * placeholder. >>>>>> + */ >>>>>> + <&infracfg_ao CLK_INFRA_AO_PCIE_P1_TL_96M>; >>>>>> clock-names = "sys_ck", "ref_ck", "mcu_ck", "dma_ck", >>>>>> - "xhci_ck"; >>>>>> + "xhci_ck", "frmcnt_ck"; >>>>>> mediatek,syscon-wakeup = <&pericfg 0x400 104>; >>>>>> wakeup-source; >>>>>> status = "disabled"; >>>>>> >>>>>> --- >>>>>> base-commit: dee7f101b64219f512bb2f842227bd04c14efe30 >>>>>> change-id: 20240722-usb-1129-probe-pci-clk-fix-ef8646f46aac >>>>>> >>>>>> Best regards, >>>>>> -- >>>>>> Nícolas F. R. A. Prado >>>>>> >>>>>> >>>> >>>> >>>> >>