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 <w...@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 effectively 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/
      [3] 
https://patchwork.ozlabs.org/project/uboot/patch/Y44+ayJfUlI08ptM@localhost/

Cc: Simon Glass <s...@chromium.org>
Cc: Philipp Tomsich <philipp.toms...@vrull.eu>
Cc: Kever Yang <kever.y...@rock-chips.com>
Cc: Lukasz Majewski <lu...@denx.de>
Cc: Sean Anderson <sean...@gmail.com>
Cc: Marek Vasut <ma...@denx.de>
Cc: Christoph Fritz <chf.fr...@googlemail.com>
Cc: Jagan Teki <ja...@amarulasolutions.com>

Signed-off-by: Xavier Drudis Ferran <xdru...@tinet.cat>
---

 V7: improve error handling. Call device_chld_unbind() on error.
     Remove unnecessary if.

 v6: just retested over current next branch and some corrections
     to message and headers
     (no changes to code).
---
 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 33 +++++++++++++++++--
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c 
b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
index 55e1dbcfef..732d37201d 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
@@ -7,10 +7,11 @@
  */
 
 #include <common.h>
-#include <clk.h>
+#include <clk-uclass.h>
 #include <dm.h>
 #include <asm/global_data.h>
 #include <dm/device_compat.h>
+#include <dm/device-internal.h>
 #include <dm/lists.h>
 #include <generic-phy.h>
 #include <reset.h>
@@ -168,6 +169,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);
@@ -234,7 +238,8 @@ static int rockchip_usb2phy_bind(struct udevice *dev)
        dev_for_each_subnode(node, dev) {
                if (!ofnode_valid(node)) {
                        dev_info(dev, "subnode %s not found\n", dev->name);
-                       return -ENXIO;
+                       ret = -ENXIO;
+                       goto bind_fail;
                }
 
                name = ofnode_get_name(node);
@@ -245,10 +250,26 @@ static int rockchip_usb2phy_bind(struct udevice *dev)
                if (ret) {
                        dev_err(dev,
                                "'%s' cannot bind 'rockchip_usb2phy_port'\n", 
name);
-                       return ret;
+                       goto bind_fail;
                }
        }
 
+       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);
+               goto bind_fail;
+       }
+
+       return 0;
+
+bind_fail:
+       device_chld_unbind(dev, NULL);
+
        return ret;
 }
 
@@ -366,6 +387,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


Reply via email to