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

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

[Xavier Drudis Ferran]

EHCI probing in Rock pi 4 currently fails. 

Add a clock driver for usb2phy so that probing EHCI does not fail when
missing one of the clocks in the bundle for usb_host0_ehci, since
usb2phy is UCLASS_PHY but not UCLASS_CLK.

Xavier Drudis Ferran (2):
  phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock
  phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock

 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 109 +++++++++++++++++-
 1 file changed, 106 insertions(+), 3 deletions(-)

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>
Cc: Christoph Fritz <chf.fritz@googlemail.com>
Cc: Jagan Teki <jagan@amarulasolutions.com>
Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat>

---

   Changes:

   v7: Error handling. Remove unnecessary if.
       Adding config data for rk3568 (untested).

   v6: just retested over current next branch and some corrections
       to message and headers
       (no changes to code).

   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.

   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).

-- 
2.20.1

[PATCH v7 1/2] phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock

[Xavier Drudis Ferran]

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 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 <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>
Cc: Christoph Fritz <chf.fritz@googlemail.com>
Cc: Jagan Teki <jagan@amarulasolutions.com>

Signed-off-by: Xavier Drudis Ferran <xdrudis@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

[PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock

[Xavier Drudis Ferran]

This clock doesn't seem needed but appears in a phandle list used by
ehci-generic.c to bulk enable it. The phandle list comes from linux,
where it is needed for suspend/resume to work [1].

My tests give the same results with or without this patch, but Marek
Vasut found it weird to declare an empty clk_ops [2].

So I adapted the code from linux 6.1-rc8 so that it hopefully works
if it ever has some user. For now, without real use, it seems to
at least not give any errors when called.

Link: [1] https://lkml.kernel.org/lkml/1731551.Q6cHK6n5ZM@phil/T/
      [2] https://patchwork.ozlabs.org/project/uboot/patch/Y5IWpjYLB4aXMy9o@localhost/

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>
Cc: Christoph Fritz <chf.fritz@googlemail.com>
Cc: Jagan Teki <jagan@amarulasolutions.com>

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

 v7: add clkout_ctl values for rk3568 (from linux).
     UNTESTED (I don't have the hardware).

 v6: just retested over current next branch and some corrections
     to message and headers
     (no changes to code).

 v5: ignores the return value from property_enable() which is not
     an error code in U-Boot (unlike in linux). This avoid a false
     failure of rockchip_usb2phy_clk_disable() that interfered with
     clock disable and appeared to cause hang or reset.
---
 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 80 ++++++++++++++++++-
 1 file changed, 78 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
index 732d37201d..be5f79490c 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
@@ -56,6 +56,7 @@ struct rockchip_usb2phy_port_cfg {
 
 struct rockchip_usb2phy_cfg {
 	unsigned int reg;
+	struct usb2phy_reg	clkout_ctl;
 	const struct rockchip_usb2phy_port_cfg port_cfgs[USB2PHY_NUM_PORTS];
 };
 
@@ -77,6 +78,18 @@ static inline int property_enable(void *reg_base,
 	return writel(val, reg_base + reg->offset);
 }
 
+static inline bool property_enabled(void *reg_base,
+				    const struct usb2phy_reg *reg)
+{
+	unsigned int tmp, orig;
+	unsigned int mask = GENMASK(reg->bitend, reg->bitstart);
+
+	orig = readl(reg_base + reg->offset);
+
+	tmp = (orig & mask) >> reg->bitstart;
+	return tmp != reg->disable;
+}
+
 static const
 struct rockchip_usb2phy_port_cfg *us2phy_get_port(struct phy *phy)
 {
@@ -169,7 +182,63 @@ static struct phy_ops rockchip_usb2phy_ops = {
 	.of_xlate = rockchip_usb2phy_of_xlate,
 };
 
+/**
+ * round_rate() - Adjust a rate to the exact rate a clock can provide.
+ * @clk:	The clock to manipulate.
+ * @rate:	Desidered clock rate in Hz.
+ *
+ * Return: rounded rate in Hz, or -ve error code.
+ */
+ulong rockchip_usb2phy_clk_round_rate(struct clk *clk, ulong rate)
+{
+	return 480000000;
+}
+
+/**
+ * enable() - Enable a clock.
+ * @clk:	The clock to manipulate.
+ *
+ * Return: zero on success, or -ve error code.
+ */
+int rockchip_usb2phy_clk_enable(struct clk *clk)
+{
+	struct udevice *parent = dev_get_parent(clk->dev);
+	struct rockchip_usb2phy *priv = dev_get_priv(parent);
+	const struct rockchip_usb2phy_cfg *phy_cfg = priv->phy_cfg;
+
+	/* turn on 480m clk output if it is off */
+	if (!property_enabled(priv->reg_base, &phy_cfg->clkout_ctl)) {
+		property_enable(priv->reg_base, &phy_cfg->clkout_ctl, true);
+
+		/* waiting for the clk become stable */
+		usleep_range(1200, 1300);
+	}
+
+	return 0;
+}
+
+/**
+ * disable() - Disable a clock.
+ * @clk:	The clock to manipulate.
+ *
+ * Return: zero on success, or -ve error code.
+ */
+int rockchip_usb2phy_clk_disable(struct clk *clk)
+{
+	struct udevice *parent = dev_get_parent(clk->dev);
+	struct rockchip_usb2phy *priv = dev_get_priv(parent);
+	const struct rockchip_usb2phy_cfg *phy_cfg = priv->phy_cfg;
+
+	/* turn off 480m clk output */
+	property_enable(priv->reg_base, &phy_cfg->clkout_ctl, false);
+
+	return 0;
+}
+
 static struct clk_ops rockchip_usb2phy_clk_ops = {
+	.enable = rockchip_usb2phy_clk_enable,
+	.disable = rockchip_usb2phy_clk_disable,
+	.round_rate = rockchip_usb2phy_clk_round_rate
 };
 
 static int rockchip_usb2phy_probe(struct udevice *dev)
@@ -255,8 +324,11 @@ static int rockchip_usb2phy_bind(struct udevice *dev)
 	}
 
 	node = dev_ofnode(dev);
-	name = ofnode_get_name(node);
-	dev_dbg(dev, "clk for node %s\n", name);
+	name = "clk_usbphy_480m";
+	dev_read_string_index(dev, "clock-output-names", 0, &name);
+
+	dev_dbg(dev, "clk %s for node %s\n", name, ofnode_get_name(node));
+
 	ret = device_bind_driver_to_node(dev, "rockchip_usb2phy_clock",
 					 name, node, &usb2phy_dev);
 	if (ret) {
@@ -276,6 +348,7 @@ bind_fail:
 static const struct rockchip_usb2phy_cfg rk3399_usb2phy_cfgs[] = {
 	{
 		.reg		= 0xe450,
+		.clkout_ctl	= { 0xe450, 4, 4, 1, 0 },
 		.port_cfgs	= {
 			[USB2PHY_PORT_OTG] = {
 				.phy_sus	= { 0xe454, 1, 0, 2, 1 },
@@ -297,6 +370,7 @@ static const struct rockchip_usb2phy_cfg rk3399_usb2phy_cfgs[] = {
 	},
 	{
 		.reg		= 0xe460,
+		.clkout_ctl	= { 0xe460, 4, 4, 1, 0 },
 		.port_cfgs	= {
 			[USB2PHY_PORT_OTG] = {
 				.phy_sus        = { 0xe464, 1, 0, 2, 1 },
@@ -322,6 +396,7 @@ static const struct rockchip_usb2phy_cfg rk3399_usb2phy_cfgs[] = {
 static const struct rockchip_usb2phy_cfg rk3568_phy_cfgs[] = {
 	{
 		.reg		= 0xfe8a0000,
+		.clkout_ctl	= { 0x0008, 4, 4, 1, 0 },
 		.port_cfgs	= {
 			[USB2PHY_PORT_OTG] = {
 				.phy_sus	= { 0x0000, 8, 0, 0x052, 0x1d1 },
@@ -347,6 +422,7 @@ static const struct rockchip_usb2phy_cfg rk3568_phy_cfgs[] = {
 	},
 	{
 		.reg		= 0xfe8b0000,
+		.clkout_ctl	= { 0x0008, 4, 4, 1, 0 },
 		.port_cfgs	= {
 			[USB2PHY_PORT_OTG] = {
 				.phy_sus	= { 0x0000, 8, 0, 0x1d2, 0x1d1 },
-- 
2.20.1

Re: [PATCH v7 1/2] phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock

[Kever Yang]

On 2023/6/5 23:05, Xavier Drudis Ferran wrote:
> 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 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 <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>
> Cc: Christoph Fritz <chf.fritz@googlemail.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
>
> Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>
>   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,

Re: [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock

[Kever Yang]

On 2023/6/5 23:06, Xavier Drudis Ferran wrote:
> This clock doesn't seem needed but appears in a phandle list used by
> ehci-generic.c to bulk enable it. The phandle list comes from linux,
> where it is needed for suspend/resume to work [1].
>
> My tests give the same results with or without this patch, but Marek
> Vasut found it weird to declare an empty clk_ops [2].
>
> So I adapted the code from linux 6.1-rc8 so that it hopefully works
> if it ever has some user. For now, without real use, it seems to
> at least not give any errors when called.
>
> Link: [1] https://lkml.kernel.org/lkml/1731551.Q6cHK6n5ZM@phil/T/
>        [2] https://patchwork.ozlabs.org/project/uboot/patch/Y5IWpjYLB4aXMy9o@localhost/
>
> 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>
> Cc: Christoph Fritz <chf.fritz@googlemail.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
>
> Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat>
Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Thanks,
- Kever
> ---
>
>   v7: add clkout_ctl values for rk3568 (from linux).
>       UNTESTED (I don't have the hardware).
>
>   v6: just retested over current next branch and some corrections
>       to message and headers
>       (no changes to code).
>
>   v5: ignores the return value from property_enable() which is not
>       an error code in U-Boot (unlike in linux). This avoid a false
>       failure of rockchip_usb2phy_clk_disable() that interfered with
>       clock disable and appeared to cause hang or reset.
> ---
>   drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 80 ++++++++++++++++++-
>   1 file changed, 78 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> index 732d37201d..be5f79490c 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> @@ -56,6 +56,7 @@ struct rockchip_usb2phy_port_cfg {
>   
>   struct rockchip_usb2phy_cfg {
>   	unsigned int reg;
> +	struct usb2phy_reg	clkout_ctl;
>   	const struct rockchip_usb2phy_port_cfg port_cfgs[USB2PHY_NUM_PORTS];
>   };
>   
> @@ -77,6 +78,18 @@ static inline int property_enable(void *reg_base,
>   	return writel(val, reg_base + reg->offset);
>   }
>   
> +static inline bool property_enabled(void *reg_base,
> +				    const struct usb2phy_reg *reg)
> +{
> +	unsigned int tmp, orig;
> +	unsigned int mask = GENMASK(reg->bitend, reg->bitstart);
> +
> +	orig = readl(reg_base + reg->offset);
> +
> +	tmp = (orig & mask) >> reg->bitstart;
> +	return tmp != reg->disable;
> +}
> +
>   static const
>   struct rockchip_usb2phy_port_cfg *us2phy_get_port(struct phy *phy)
>   {
> @@ -169,7 +182,63 @@ static struct phy_ops rockchip_usb2phy_ops = {
>   	.of_xlate = rockchip_usb2phy_of_xlate,
>   };
>   
> +/**
> + * round_rate() - Adjust a rate to the exact rate a clock can provide.
> + * @clk:	The clock to manipulate.
> + * @rate:	Desidered clock rate in Hz.
> + *
> + * Return: rounded rate in Hz, or -ve error code.
> + */
> +ulong rockchip_usb2phy_clk_round_rate(struct clk *clk, ulong rate)
> +{
> +	return 480000000;
> +}
> +
> +/**
> + * enable() - Enable a clock.
> + * @clk:	The clock to manipulate.
> + *
> + * Return: zero on success, or -ve error code.
> + */
> +int rockchip_usb2phy_clk_enable(struct clk *clk)
> +{
> +	struct udevice *parent = dev_get_parent(clk->dev);
> +	struct rockchip_usb2phy *priv = dev_get_priv(parent);
> +	const struct rockchip_usb2phy_cfg *phy_cfg = priv->phy_cfg;
> +
> +	/* turn on 480m clk output if it is off */
> +	if (!property_enabled(priv->reg_base, &phy_cfg->clkout_ctl)) {
> +		property_enable(priv->reg_base, &phy_cfg->clkout_ctl, true);
> +
> +		/* waiting for the clk become stable */
> +		usleep_range(1200, 1300);
> +	}
> +
> +	return 0;
> +}
> +
> +/**
> + * disable() - Disable a clock.
> + * @clk:	The clock to manipulate.
> + *
> + * Return: zero on success, or -ve error code.
> + */
> +int rockchip_usb2phy_clk_disable(struct clk *clk)
> +{
> +	struct udevice *parent = dev_get_parent(clk->dev);
> +	struct rockchip_usb2phy *priv = dev_get_priv(parent);
> +	const struct rockchip_usb2phy_cfg *phy_cfg = priv->phy_cfg;
> +
> +	/* turn off 480m clk output */
> +	property_enable(priv->reg_base, &phy_cfg->clkout_ctl, false);
> +
> +	return 0;
> +}
> +
>   static struct clk_ops rockchip_usb2phy_clk_ops = {
> +	.enable = rockchip_usb2phy_clk_enable,
> +	.disable = rockchip_usb2phy_clk_disable,
> +	.round_rate = rockchip_usb2phy_clk_round_rate
>   };
>   
>   static int rockchip_usb2phy_probe(struct udevice *dev)
> @@ -255,8 +324,11 @@ static int rockchip_usb2phy_bind(struct udevice *dev)
>   	}
>   
>   	node = dev_ofnode(dev);
> -	name = ofnode_get_name(node);
> -	dev_dbg(dev, "clk for node %s\n", name);
> +	name = "clk_usbphy_480m";
> +	dev_read_string_index(dev, "clock-output-names", 0, &name);
> +
> +	dev_dbg(dev, "clk %s for node %s\n", name, ofnode_get_name(node));
> +
>   	ret = device_bind_driver_to_node(dev, "rockchip_usb2phy_clock",
>   					 name, node, &usb2phy_dev);
>   	if (ret) {
> @@ -276,6 +348,7 @@ bind_fail:
>   static const struct rockchip_usb2phy_cfg rk3399_usb2phy_cfgs[] = {
>   	{
>   		.reg		= 0xe450,
> +		.clkout_ctl	= { 0xe450, 4, 4, 1, 0 },
>   		.port_cfgs	= {
>   			[USB2PHY_PORT_OTG] = {
>   				.phy_sus	= { 0xe454, 1, 0, 2, 1 },
> @@ -297,6 +370,7 @@ static const struct rockchip_usb2phy_cfg rk3399_usb2phy_cfgs[] = {
>   	},
>   	{
>   		.reg		= 0xe460,
> +		.clkout_ctl	= { 0xe460, 4, 4, 1, 0 },
>   		.port_cfgs	= {
>   			[USB2PHY_PORT_OTG] = {
>   				.phy_sus        = { 0xe464, 1, 0, 2, 1 },
> @@ -322,6 +396,7 @@ static const struct rockchip_usb2phy_cfg rk3399_usb2phy_cfgs[] = {
>   static const struct rockchip_usb2phy_cfg rk3568_phy_cfgs[] = {
>   	{
>   		.reg		= 0xfe8a0000,
> +		.clkout_ctl	= { 0x0008, 4, 4, 1, 0 },
>   		.port_cfgs	= {
>   			[USB2PHY_PORT_OTG] = {
>   				.phy_sus	= { 0x0000, 8, 0, 0x052, 0x1d1 },
> @@ -347,6 +422,7 @@ static const struct rockchip_usb2phy_cfg rk3568_phy_cfgs[] = {
>   	},
>   	{
>   		.reg		= 0xfe8b0000,
> +		.clkout_ctl	= { 0x0008, 4, 4, 1, 0 },
>   		.port_cfgs	= {
>   			[USB2PHY_PORT_OTG] = {
>   				.phy_sus	= { 0x0000, 8, 0, 0x1d2, 0x1d1 },

Re: [PATCH v7 1/2] phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock

[Jagan Teki]

On Mon, Jun 5, 2023 at 8:35 PM Xavier Drudis Ferran <xdrudis@tinet.cat> wrote:
>
> 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 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 <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>
> Cc: Christoph Fritz <chf.fritz@googlemail.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
>
> Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat>
> ---

Thanks for fixing USB from the last couple of releases.

Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com> # rk3399, rk3328, rv1126

Re: [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock

[Jagan Teki]

On Mon, Jun 5, 2023 at 8:37 PM Xavier Drudis Ferran <xdrudis@tinet.cat> wrote:
>
> This clock doesn't seem needed but appears in a phandle list used by
> ehci-generic.c to bulk enable it. The phandle list comes from linux,
> where it is needed for suspend/resume to work [1].
>
> My tests give the same results with or without this patch, but Marek
> Vasut found it weird to declare an empty clk_ops [2].
>
> So I adapted the code from linux 6.1-rc8 so that it hopefully works
> if it ever has some user. For now, without real use, it seems to
> at least not give any errors when called.
>
> Link: [1] https://lkml.kernel.org/lkml/1731551.Q6cHK6n5ZM@phil/T/
>       [2] https://patchwork.ozlabs.org/project/uboot/patch/Y5IWpjYLB4aXMy9o@localhost/
>
> 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>
> Cc: Christoph Fritz <chf.fritz@googlemail.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
>
> Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat>
> ---

Reviewed-by: Jagan Teki <jagan@amarulasolutions.com>
Tested-by: Jagan Teki <jagan@amarulasolutions.com> # rk3399, rk3328, rv1126

Re: [PATCH v7 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2

[Marek Vasut]

On 6/5/23 17:04, Xavier Drudis Ferran wrote:
> EHCI probing in Rock pi 4 currently fails.
> 
> Add a clock driver for usb2phy so that probing EHCI does not fail when
> missing one of the clocks in the bundle for usb_host0_ehci, since
> usb2phy is UCLASS_PHY but not UCLASS_CLK.
> 
> Xavier Drudis Ferran (2):
>    phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock
>    phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock
> 
>   drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 109 +++++++++++++++++-
>   1 file changed, 106 insertions(+), 3 deletions(-)

Applied both to usb/master .

btw the cover letter subject should not have 'arm: dts:' tags, but 
rather 'phy:' tag , since this does not touch any DTs .

Re: [PATCH v7 0/2] arm: dts: rockchip: rk3399: usb: ehci: Fix EHCI probe in rk3399 to access peripherals by USB 2

[Xavier Drudis Ferran]

El Wed, Jun 07, 2023 at 11:42:40PM +0200, Marek Vasut deia:
> On 6/5/23 17:04, Xavier Drudis Ferran wrote:
> > EHCI probing in Rock pi 4 currently fails.
> > 
> > Add a clock driver for usb2phy so that probing EHCI does not fail when
> > missing one of the clocks in the bundle for usb_host0_ehci, since
> > usb2phy is UCLASS_PHY but not UCLASS_CLK.
> > 
> > Xavier Drudis Ferran (2):
> >    phy: rockchip-inno-usb2: Add usb2phy clock provider of 480MHz clock
> >    phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock
> > 
> >   drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 109 +++++++++++++++++-
> >   1 file changed, 106 insertions(+), 3 deletions(-)
> 
> Applied both to usb/master .
> 
> btw the cover letter subject should not have 'arm: dts:' tags, but rather
> 'phy:' tag , since this does not touch any DTs .


Yes, sorry. v1 did touch *-u-boot.dts. I hesitated about what was more
confusing, changing subject on different versions of a patch with the
same intent or keeping the old subject.

Thanks for merging.

Re: [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock

[Jagan Teki]

On Mon, Jun 5, 2023 at 8:37 PM Xavier Drudis Ferran <xdrudis@tinet.cat> wrote:
>
> This clock doesn't seem needed but appears in a phandle list used by
> ehci-generic.c to bulk enable it. The phandle list comes from linux,
> where it is needed for suspend/resume to work [1].
>
> My tests give the same results with or without this patch, but Marek
> Vasut found it weird to declare an empty clk_ops [2].
>
> So I adapted the code from linux 6.1-rc8 so that it hopefully works
> if it ever has some user. For now, without real use, it seems to
> at least not give any errors when called.
>
> Link: [1] https://lkml.kernel.org/lkml/1731551.Q6cHK6n5ZM@phil/T/
>       [2] https://patchwork.ozlabs.org/project/uboot/patch/Y5IWpjYLB4aXMy9o@localhost/
>
> 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>
> Cc: Christoph Fritz <chf.fritz@googlemail.com>
> Cc: Jagan Teki <jagan@amarulasolutions.com>
>
> Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat>
> ---
>
>  v7: add clkout_ctl values for rk3568 (from linux).
>      UNTESTED (I don't have the hardware).
>
>  v6: just retested over current next branch and some corrections
>      to message and headers
>      (no changes to code).
>
>  v5: ignores the return value from property_enable() which is not
>      an error code in U-Boot (unlike in linux). This avoid a false
>      failure of rockchip_usb2phy_clk_disable() that interfered with
>      clock disable and appeared to cause hang or reset.
> ---
>  drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 80 ++++++++++++++++++-
>  1 file changed, 78 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> index 732d37201d..be5f79490c 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> @@ -56,6 +56,7 @@ struct rockchip_usb2phy_port_cfg {
>
>  struct rockchip_usb2phy_cfg {
>         unsigned int reg;
> +       struct usb2phy_reg      clkout_ctl;
>         const struct rockchip_usb2phy_port_cfg port_cfgs[USB2PHY_NUM_PORTS];
>  };
>
> @@ -77,6 +78,18 @@ static inline int property_enable(void *reg_base,
>         return writel(val, reg_base + reg->offset);
>  }
>
> +static inline bool property_enabled(void *reg_base,
> +                                   const struct usb2phy_reg *reg)
> +{
> +       unsigned int tmp, orig;
> +       unsigned int mask = GENMASK(reg->bitend, reg->bitstart);
> +
> +       orig = readl(reg_base + reg->offset);
> +
> +       tmp = (orig & mask) >> reg->bitstart;
> +       return tmp != reg->disable;
> +}
> +
>  static const
>  struct rockchip_usb2phy_port_cfg *us2phy_get_port(struct phy *phy)
>  {
> @@ -169,7 +182,63 @@ static struct phy_ops rockchip_usb2phy_ops = {
>         .of_xlate = rockchip_usb2phy_of_xlate,
>  };
>
> +/**
> + * round_rate() - Adjust a rate to the exact rate a clock can provide.
> + * @clk:       The clock to manipulate.
> + * @rate:      Desidered clock rate in Hz.
> + *
> + * Return: rounded rate in Hz, or -ve error code.
> + */

I forgot to comment, this last time. I feel these explicit comments
wouldn't be required as clk-uclass clearly documented.

> +ulong rockchip_usb2phy_clk_round_rate(struct clk *clk, ulong rate)

static

> +{
> +       return 480000000;
> +}
> +
> +/**
> + * enable() - Enable a clock.
> + * @clk:       The clock to manipulate.
> + *
> + * Return: zero on success, or -ve error code.
> + */

ditto.

> +int rockchip_usb2phy_clk_enable(struct clk *clk)

ditto.

> +{
> +       struct udevice *parent = dev_get_parent(clk->dev);
> +       struct rockchip_usb2phy *priv = dev_get_priv(parent);
> +       const struct rockchip_usb2phy_cfg *phy_cfg = priv->phy_cfg;
> +
> +       /* turn on 480m clk output if it is off */
> +       if (!property_enabled(priv->reg_base, &phy_cfg->clkout_ctl)) {
> +               property_enable(priv->reg_base, &phy_cfg->clkout_ctl, true);
> +
> +               /* waiting for the clk become stable */
> +               usleep_range(1200, 1300);
> +       }
> +
> +       return 0;
> +}
> +
> +/**
> + * disable() - Disable a clock.
> + * @clk:       The clock to manipulate.
> + *
> + * Return: zero on success, or -ve error code.
> + */

ditto.

> +int rockchip_usb2phy_clk_disable(struct clk *clk)

ditto.

Thanks,
Jagan.

Re: [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock

[Jagan Teki]

Hi Kever,

On Tue, Jun 6, 2023 at 6:24 AM Kever Yang <kever.yang@rock-chips.com> wrote:
>
>
> On 2023/6/5 23:06, Xavier Drudis Ferran wrote:
> > This clock doesn't seem needed but appears in a phandle list used by
> > ehci-generic.c to bulk enable it. The phandle list comes from linux,
> > where it is needed for suspend/resume to work [1].
> >
> > My tests give the same results with or without this patch, but Marek
> > Vasut found it weird to declare an empty clk_ops [2].
> >
> > So I adapted the code from linux 6.1-rc8 so that it hopefully works
> > if it ever has some user. For now, without real use, it seems to
> > at least not give any errors when called.
> >
> > Link: [1] https://lkml.kernel.org/lkml/1731551.Q6cHK6n5ZM@phil/T/
> >        [2] https://patchwork.ozlabs.org/project/uboot/patch/Y5IWpjYLB4aXMy9o@localhost/
> >
> > 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>
> > Cc: Christoph Fritz <chf.fritz@googlemail.com>
> > Cc: Jagan Teki <jagan@amarulasolutions.com>
> >
> > Signed-off-by: Xavier Drudis Ferran <xdrudis@tinet.cat>
> Reviewed-by: Kever Yang <kever.yang@rock-chips.com>

Please merge these two asap. Better would these two be part of the
coming release?

Thanks,
Jagan.

Re: [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock

[Xavier Drudis Ferran]

El Mon, Jun 19, 2023 at 12:04:51PM +0530, Jagan Teki deia:
> 
> Please merge these two asap. Better would these two be part of the
> coming release?
>

How do you mean ?

They're both in master and next now.

see commits:

e81512ac30c154c320b54036919cd3a6f4cc1516
40359c94405b103d25233d8d727d671748b751b9

Or do you mean I should send fixes to comments and static qualifiers
for 3 functions as you pointed out ?

https://lists.denx.de/pipermail/u-boot/2023-June/519708.html

I wasn't sure if that was a notice for me to do it better next time or
it required a clean up patch.

Re: [PATCH v7 2/2] phy: rockchip-inno-usb2: Implement clock operations for usb2phy clock

[Jagan Teki]

On Mon, Jun 19, 2023 at 12:38 PM Xavier Drudis Ferran <xdrudis@tinet.cat> wrote:
>
> El Mon, Jun 19, 2023 at 12:04:51PM +0530, Jagan Teki deia:
> >
> > Please merge these two asap. Better would these two be part of the
> > coming release?
> >
>
> How do you mean ?
>
> They're both in master and next now.

Ohh. Okay, I didn't see that.
>
> see commits:
>
> e81512ac30c154c320b54036919cd3a6f4cc1516
> 40359c94405b103d25233d8d727d671748b751b9
>
> Or do you mean I should send fixes to comments and static qualifiers
> for 3 functions as you pointed out ?

Yes, may be second one. No problem.

Thanks,
Jagan.