[PATCH v2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2.

[Xavier Drudis Ferran <xdrudis@tinet.cat>]

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. I just can't seem to imagine how to achieve this with the
   U-Boot driver model, maybe because of my limited familiarity with
   it.

This patch is option 1 as Kever Yang requested on August 27th[2] when
option 3 didn't get through. Sorry for the delay.

Yet maybe the get_some_clks() function should be added to clk-uclass.c
if it may be useful elsewhere. Or alternatively, clk_get_bulk() could
be changed to the lenient behaviour of get_some_clks(), but that seems
too invasive to me. Either of these changes can always be done in a
later patch if needed.

If so, one possibility would be to call it from ohci-generic.c. As it
is now it looks like if it ever misses a clock but finds a subsequent
clock, assigned to a higher index in the clock table, it may leave
clock_count too low to release all found clocks. I don't know of a
case where this happens, for rk3399 usb, even with non-default
CONFIG_OHCI_GENERIC, the missing clock is the last one, and so the
release iteration happens to find all found clocks.

Link: [1] https://lkml.kernel.org/lkml/1731551.Q6cHK6n5ZM@phil/T/
      [2] https://patchwork.ozlabs.org/project/uboot/patch/20220701185959.GC1700@begut/#2954536
      
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:
  
  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(-)

diff --git a/drivers/usb/host/ehci-generic.c b/drivers/usb/host/ehci-generic.c
index a765a307a3..aa86dcc435 100644
--- a/drivers/usb/host/ehci-generic.c
+++ b/drivers/usb/host/ehci-generic.c
@@ -60,6 +60,63 @@ static int ehci_disable_vbus_supply(struct generic_ehci *priv)
 		return 0;
 }
 
+/**
+ * get_some_clks() - Get/request all available clocks of a
+ *                    device. Leave other as null.
+ *
+ * @dev:	The client device.
+ * @bulk:	A pointer to a clock bulk struct to initialize.
+ *
+ * This looks up and requests all clocks of the client device; each
+ * device is assumed to have n clocks associated with it somehow, and
+ * this function tries to find and request all of them in a separate
+ * structure. The mapping of client device clock indices to provider
+ * clocks may be via device-tree properties, board-provided mapping
+ * tables, or some other mechanism.
+ *
+ * This is like clk_get_bulk() in clk uclass, but failure to get some
+ * of the clocks is accepted and only the available ones are assigned
+ * to bulk->clks. So bulk->clks may contain invalid (zeroed) clk
+ * structs. clk_release_bulk(), clk_enable_bulk() and
+ * clk_disable_bulk() can deal with that.
+ *
+ * bulk->count will contain the number of attempted clocks (size of
+ * bulk->clks). Or an error when getting the clocks phandle if
+ * negative.
+ *
+ * Return: If some clocks could be successfully located and requested,
+ * it returns 0.  If all failed, then returns the last error code.
+ */
+static int get_some_clks(struct udevice *dev, struct clk_bulk *bulk)
+{
+	int i, ret, count;
+
+	count = 0;
+
+	bulk->count = dev_count_phandle_with_args(dev, "clocks", "#clock-cells", 0);
+	if (bulk->count < 1)
+		return bulk->count;
+
+	bulk->clks = devm_kcalloc(dev, bulk->count, sizeof(struct clk), GFP_KERNEL);
+	if (!bulk->clks)
+		return -ENOMEM;
+
+	for (i = 0; i < bulk->count; i++) {
+		ret = clk_get_by_index(dev, i, &bulk->clks[i]);
+		if (ret < 0) {
+			dev_warn(dev, "Failed to get clock %i/%i (ret=%d)\n", i, bulk->count, ret);
+			break;
+		}
+
+		++count;
+	}
+
+	if (!count)
+		return ret;
+
+	return 0;
+}
+
 static int ehci_usb_probe(struct udevice *dev)
 {
 	struct generic_ehci *priv = dev_get_priv(dev);
@@ -68,7 +125,7 @@ static int ehci_usb_probe(struct udevice *dev)
 	int err, ret;
 
 	err = 0;
-	ret = clk_get_bulk(dev, &priv->clocks);
+	ret = get_some_clks(dev, &priv->clocks);
 	if (ret && ret != -ENOENT) {
 		dev_err(dev, "Failed to get clocks (ret=%d)\n", ret);
 		return ret;
-- 
2.20.1