From: Xavier Drudis Ferran <xdrudis@tinet.cat>
To: u-boot@lists.denx.de
Cc: Simon Glass <sjg@chromium.org>,
Philipp Tomsich <philipp.tomsich@vrull.eu>,
Kever Yang <kever.yang@rock-chips.com>,
Lukasz Majewski <lukma@denx.de>,
Sean Anderson <seanga2@gmail.com>, Marek Vasut <marex@denx.de>
Subject: [PATCH v3] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2.
Date: Thu, 8 Dec 2022 17:53:58 +0100 [thread overview]
Message-ID: <Y5IWpjYLB4aXMy9o@localhost> (raw)
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 the white port, 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 <wulf@rock-chips.com>
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.
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.
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.
This patch tries to implement option 5 as Marek Vasut requested in
December 5th. Options 1 and 3 didn't get through[2,3].
It just registers usb2phy as a clock driver (device_bind_driver()
didn't work but device_bind_driver_to_node() did), 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).
Link: [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
Cc: Simon Glass <sjg@chromium.org>
Cc: Philipp Tomsich <philipp.tomsich@vrull.eu>
Cc: Kever Yang <kever.yang@rock-chips.com>
Cc: Lukasz Majewski <lukma@denx.de>
Cc: Sean Anderson <seanga2@gmail.com>
Cc: Marek Vasut <marex@denx.de>
Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat>
---
Changes:
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).
---
drivers/usb/host/ehci-generic.c | 59 ++++++++++++++++++++++++++++++++-
1 file changed, 58 insertions(+), 1 deletion(-)
---
drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 23 ++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
index 62b8ba3a4a..97a1e11239 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
@@ -7,7 +7,7 @@
*/
#include <common.h>
-#include <clk.h>
+#include <clk-uclass.h>
#include <dm.h>
#include <asm/global_data.h>
#include <dm/device_compat.h>
@@ -168,6 +168,9 @@ static struct phy_ops rockchip_usb2phy_ops = {
.of_xlate = rockchip_usb2phy_of_xlate,
};
+static struct clk_ops rockchip_usb2phy_clk_ops = {
+};
+
static int rockchip_usb2phy_probe(struct udevice *dev)
{
struct rockchip_usb2phy *priv = dev_get_priv(dev);
@@ -240,6 +243,18 @@ static int rockchip_usb2phy_bind(struct udevice *dev)
}
}
+ if (!ret) {
+ node = dev_ofnode(dev);
+ name = ofnode_get_name(node);
+ dev_dbg(dev, "clk for node %s\n", name);
+ ret = device_bind_driver_to_node(dev, "rockchip_usb2phy_clock",
+ name, node, &usb2phy_dev);
+ if (ret) {
+ dev_err(dev,
+ "'%s' cannot bind 'rockchip_usb2phy_clock'\n", name);
+ }
+ }
+
return ret;
}
@@ -303,6 +318,12 @@ U_BOOT_DRIVER(rockchip_usb2phy_port) = {
.ops = &rockchip_usb2phy_ops,
};
+U_BOOT_DRIVER(rockchip_usb2phy_clock) = {
+ .name = "rockchip_usb2phy_clock",
+ .id = UCLASS_CLK,
+ .ops = &rockchip_usb2phy_clk_ops,
+};
+
U_BOOT_DRIVER(rockchip_usb2phy) = {
.name = "rockchip_usb2phy",
.id = UCLASS_PHY,
--
2.20.1
next reply other threads:[~2022-12-08 17:02 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-08 16:53 Xavier Drudis Ferran [this message]
2022-12-08 20:12 ` [PATCH v3] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2 Marek Vasut
2022-12-08 21:00 ` Xavier Drudis Ferran
2022-12-08 22:50 ` Marek Vasut
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Y5IWpjYLB4aXMy9o@localhost \
--to=xdrudis@tinet.cat \
--cc=kever.yang@rock-chips.com \
--cc=lukma@denx.de \
--cc=marex@denx.de \
--cc=philipp.tomsich@vrull.eu \
--cc=seanga2@gmail.com \
--cc=sjg@chromium.org \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.