On Fri, Jul 26, 2024 at 8:11 PM AngeloGioacchino Del Regno <angelogioacchino.delregno@xxxxxxxxxxxxx> 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 > > <nfraprado@xxxxxxxxxxxxx> 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 <nfraprado@xxxxxxxxxxxxx> #KernelCI > >> Closes: https://lore.kernel.org/all/9fce9838-ef87-4d1b-b3df-63e1ddb0ec51@notapiano/ > >> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@xxxxxxxxxxxxx> > > > > 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? 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 <nfraprado@xxxxxxxxxxxxx> > >> > >> > > >