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 phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 764F1C64ED6 for ; Mon, 27 Feb 2023 18:00:47 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 0D865844F5; Mon, 27 Feb 2023 19:00:45 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=tinet.cat Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Received: by phobos.denx.de (Postfix, from userid 109) id 7FEBC856C0; Mon, 27 Feb 2023 19:00:42 +0100 (CET) Received: from mx1.tinet.cat (mx1.dipta.cat [195.76.233.59]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 282FD809A6 for ; Mon, 27 Feb 2023 19:00:39 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=none (p=none dis=none) header.from=tinet.cat Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=xdrudis@tinet.cat X-ASG-Debug-ID: 1677520837-12aaf22d882406b0001-4l7tJC Received: from smtp01.tinet.cat (smtp01.tinet.org [195.77.216.131]) by mx1.tinet.cat with ESMTP id IgfpcAqO9bLqiw52; Mon, 27 Feb 2023 19:00:37 +0100 (CET) X-Barracuda-Envelope-From: xdrudis@tinet.cat X-Barracuda-Effective-Source-IP: smtp01.tinet.org[195.77.216.131] X-Barracuda-Apparent-Source-IP: 195.77.216.131 Received: from xdrudis.tinet.cat (9.red-79-152-187.dynamicip.rima-tde.net [79.152.187.9]) by smtp01.tinet.cat (Postfix) with ESMTPSA id 7ED8B605DE44; Mon, 27 Feb 2023 19:00:37 +0100 (CET) Date: Mon, 27 Feb 2023 19:00:24 +0100 From: Xavier Drudis Ferran To: u-boot@lists.denx.de Cc: Simon Glass , Philipp Tomsich , Kever Yang , Lukasz Majewski , Sean Anderson , Marek Vasut , Christoph Fritz Subject: [PATCH v5 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2. Message-ID: X-ASG-Orig-Subj: [PATCH v5 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2. MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline X-Barracuda-Connect: smtp01.tinet.org[195.77.216.131] X-Barracuda-Start-Time: 1677520837 X-Barracuda-URL: https://webmail.tinet.cat:443/cgi-mod/mark.cgi X-Barracuda-Scan-Msg-Size: 5156 X-Barracuda-BRTS-Status: 1 X-Barracuda-Spam-Score: 0.00 X-Barracuda-Spam-Status: No, SCORE=0.00 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=6.0 KILL_LEVEL=8.0 tests= X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.105356 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean arch/arm/dts/rk3399.dtsi has a node usb_host0_ehci: usb@fe380000 { compatible = "generic-ehci"; with clocks: clocks = <&cru HCLK_HOST0>, <&cru HCLK_HOST0_ARB>, <&u2phy0>; The first 2 refer to nodes with class UCLASS_CLK, but &u2phy0 has class UCLASS_PHY. u2phy0: usb2phy@e450 { compatible = "rockchip,rk3399-usb2phy"; Since clk_get_bulk() only looks for devices with UCLASS_CLK, it fails with -ENODEV and then ehci_usb_probe() aborts. The consequence is peripherals connected to a USB 2 port (e.g. in a Rock Pi 4 one of the two ports nearer the edge) not being detected. They're detected if CONFIG_USB_OHCI_GENERIC is selected in Kconfig, because ohci_usb_probe() does not abort when one clk_get_by_index() fails, but then they work in USB 1 mode,. rk3399.dtsi comes from linux and the u2phy0 was added[1] to the clock list in: commit b5d1c57299734f5b54035ef2e61706b83041f20c Author: William wu Date: Wed Dec 21 18:41:05 2016 +0800 arm64: dts: rockchip: add u2phy clock for ehci and ohci of rk3399 We found that the suspend process was blocked when it run into ehci/ohci module due to clk-480m of usb2-phy was disabled. [...] Suspend concerns don't apply to U-Boot, and the problem with U-Boot failing to probe EHCI doesn't apply to linux, because in linux rockchip_usb2phy_clk480m_register makes u2phy0 a proper clock provider when called by rockchip_usb2phy_probe(). So I can think of a few alternative solutions: 1- Change ehci_usb_probe() to make it more similar to ohci_usb_probe(), and survive failure to get one clock. Looks a little harder, and I don't know whether it could break something if it ignored a clock that was important for something else than suspend. (tried in v2) 2- Change rk3399.dtsi effecttively reverting the linux commit b5d1c57299734f5b54035ef2e61706b83041f20c. This dealigns the .dtsi from linux and seems fragile at the next synchronisation. 3- Change the clock list in rk3399-u-boot.dtsi or somewhere else. This survives .dts* sync but may survive "too much" and miss some change from linux that we might want. (tried in v1) 4- Enable CONFIG_USB_OHCI_GENERIC and use the ports in USB 1 mode. This would need to be made for all boards using rk3399. In a simple test reading one file from USB storage it gave 769.5 KiB/s instead of 20.5 MiB/s with solution 2. 5- Trying to replicate linux and have usb2phy somehow provide a clk, or have a separate clock device for usb2phy in addition to the phy device. (tried in v3 and this v5, v4 did it wrong) This series is a third attempt to implement option 5 as Marek Vasut requested in December 5th. Options 1 and 3 didn't get through[2,3]. The first patch in the series (identical to v3) just registers usb2phy as a clock driver, without any specific operations, so that ehci-generic.c finds it and is happy. It worked in my tests on a Rock Pi 4 B+ (rk3399). Since Marek Vasut objected to an operationless driver[4], the second patch adds enable and disable operations adapted from linux prepare and unprepare operations (and round_rate(), which doesn't seem very useful anyway since it's a fixed clock). Since there're no users of this clock in u-boot, I can't see any difference in my tests with only the first patch or both, so I can't be sure it really works if it's ever needed, but it's hopefully more complete. Links: [1] https://lkml.kernel.org/lkml/1731551.Q6cHK6n5ZM@phil/T/ [2] https://patchwork.ozlabs.org/project/uboot/patch/20220701185959.GC1700@begut/#2954536 [3] https://patchwork.ozlabs.org/project/uboot/patch/Y44+ayJfUlI08ptM@localhost/#3016099 [4] https://patchwork.ozlabs.org/project/uboot/patch/Y5IWpjYLB4aXMy9o@localhost/#3018135 Cc: Simon Glass Cc: Philipp Tomsich Cc: Kever Yang Cc: Lukasz Majewski Cc: Sean Anderson Cc: Marek Vasut Cc: Christoph Fritz Signed-off-by: Xavier Drudis Ferran --- Changes: v5: fixes a bug that Christoph Fritz discovered, consisting in the wrong eror code returned when enabling or disabling the clock because property_enable() returns an error code in linux but the modified register value in U-Boot. This caused the clk disable to abort before freeing the clock and it apparently left things bad enough to cause a hang or a reset. v4: move v3 to one patch in the series and add a second patch to add operations to enable disable the usb2phy 480Mhz clock. Also, honour clock-output-names for what is worth. v3: implement option 5 (bind usb2phy as a clk driver too) instead of option 1 (ehci-generic.c tolerates missing clocks). v2: implement option 1 (ehci-generic.c tolerates missing clocks) instead of option 3 (change dts node to remove the missing clock).