[PATCH 1/2] phy: rockchip-inno-usb2: add support for phy-supply

PHY driver needs to enable PHY supply, otherwise port will remain unpowered.
Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com --- drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 59 ++++++++++++++++++- 1 file changed, 57 insertions(+), 2 deletions(-)
diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c index 55e1dbcfef..1ef40b448e 100644 --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c @@ -19,6 +19,7 @@ #include <asm/io.h> #include <linux/iopoll.h> #include <asm/arch-rockchip/clock.h> +#include <power/regulator.h>
DECLARE_GLOBAL_DATA_PTR;
@@ -62,6 +63,10 @@ struct rockchip_usb2phy { void *reg_base; struct clk phyclk; const struct rockchip_usb2phy_cfg *phy_cfg; +#if CONFIG_IS_ENABLED(DM_REGULATOR) + struct udevice *host_supply; + struct udevice *otg_supply; +#endif };
static inline int property_enable(void *reg_base, @@ -86,12 +91,40 @@ struct rockchip_usb2phy_port_cfg *us2phy_get_port(struct phy *phy) return &phy_cfg->port_cfgs[phy->id]; }
+#if CONFIG_IS_ENABLED(DM_REGULATOR) +static int rockchip_usb2phy_regulator_set_enable(struct phy *phy, bool enable) +{ + struct udevice *parent = dev_get_parent(phy->dev); + struct rockchip_usb2phy *priv = dev_get_priv(parent); + struct udevice *supply; + int ret = 0; + if (phy->id == USB2PHY_PORT_HOST) + supply = priv->host_supply; + else + supply = priv->otg_supply; + + if (supply) + ret = regulator_set_enable(supply, enable); + + return ret; +} +#else +static int rockchip_usb2phy_regulator_set_enable(struct phy *phy, bool enable) +{ + return 0; +} +#endif + static int rockchip_usb2phy_power_on(struct phy *phy) { struct udevice *parent = dev_get_parent(phy->dev); struct rockchip_usb2phy *priv = dev_get_priv(parent); const struct rockchip_usb2phy_port_cfg *port_cfg = us2phy_get_port(phy);
+ int ret = rockchip_usb2phy_regulator_set_enable(phy, true); + if (ret) + return ret; + property_enable(priv->reg_base, &port_cfg->phy_sus, false);
/* waiting for the utmi_clk to become stable */ @@ -108,6 +141,10 @@ static int rockchip_usb2phy_power_off(struct phy *phy)
property_enable(priv->reg_base, &port_cfg->phy_sus, true);
+ int ret = rockchip_usb2phy_regulator_set_enable(phy, false); + if (ret) + return ret; + return 0; }
@@ -149,11 +186,29 @@ static int rockchip_usb2phy_of_xlate(struct phy *phy, struct ofnode_phandle_args *args) { const char *name = phy->dev->name; + struct udevice *parent = dev_get_parent(phy->dev); + struct rockchip_usb2phy *priv = dev_get_priv(parent); +#if CONFIG_IS_ENABLED(DM_REGULATOR) + struct udevice *supply; + int ret = device_get_supply_regulator(phy->dev, "phy-supply", &supply); + if (ret && ret != -ENOENT) { + pr_err("Failed to get PHY regulator\n"); + return ret; + } +#endif
- if (!strcasecmp(name, "host-port")) + if (!strcasecmp(name, "host-port")) { phy->id = USB2PHY_PORT_HOST; - else if (!strcasecmp(name, "otg-port")) +#if CONFIG_IS_ENABLED(DM_REGULATOR) + priv->host_supply = supply; +#endif + } + else if (!strcasecmp(name, "otg-port")) { phy->id = USB2PHY_PORT_OTG; +#if CONFIG_IS_ENABLED(DM_REGULATOR) + priv->otg_supply = supply; +#endif + } else dev_err(phy->dev, "improper %s device\n", name);

combphy0 is failing to probe due to unhandled assigned-clocks and assigned-clocks-rates.
commit 5bec4b0de785 ("arm64: dts: rk356x-u-boot: Drop combphy1 assigned-clocks/rates") dropped these properties for combphy1, so let's drop them for combphy0 as well.
Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com --- arch/arm/dts/rk3568-evb-u-boot.dtsi | 1 + arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 1 + arch/arm/dts/rk3568-u-boot.dtsi | 11 +++++++++++ 3 files changed, 13 insertions(+) create mode 100644 arch/arm/dts/rk3568-u-boot.dtsi
diff --git a/arch/arm/dts/rk3568-evb-u-boot.dtsi b/arch/arm/dts/rk3568-evb-u-boot.dtsi index 17503d3d27..77430da3ba 100644 --- a/arch/arm/dts/rk3568-evb-u-boot.dtsi +++ b/arch/arm/dts/rk3568-evb-u-boot.dtsi @@ -4,6 +4,7 @@ */
#include "rk356x-u-boot.dtsi" +#include "rk3568-u-boot.dtsi"
/ { chosen { diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi index ed47efa44b..44cf33ed4b 100644 --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi @@ -5,6 +5,7 @@ */
#include "rk356x-u-boot.dtsi" +#include "rk3568-u-boot.dtsi"
/ { chosen { diff --git a/arch/arm/dts/rk3568-u-boot.dtsi b/arch/arm/dts/rk3568-u-boot.dtsi new file mode 100644 index 0000000000..c6776666df --- /dev/null +++ b/arch/arm/dts/rk3568-u-boot.dtsi @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * Copyright (c) 2023 Vasily Khoruzhick anarsoul@gmail.com + */ + +#include "rockchip-u-boot.dtsi" + +&combphy0 { + /delete-property/ assigned-clocks; + /delete-property/ assigned-clock-rates; +};

From: Vasily Khoruzhick anarsoul@gmail.com Date: Tue, 7 Mar 2023 11:37:48 -0800
combphy0 is failing to probe due to unhandled assigned-clocks and assigned-clocks-rates.
That is probably the wrong approach. It should be possible to boot an OS with the device tree provided by U-Boot. Removing these properties means the OS doesn't see them either. But if the assigned-clocks property isn't needed it wouldn't be there would it?
commit 5bec4b0de785 ("arm64: dts: rk356x-u-boot: Drop combphy1 assigned-clocks/rates") dropped these properties for combphy1, so let's drop them for combphy0 as well.
Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com
arch/arm/dts/rk3568-evb-u-boot.dtsi | 1 + arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 1 + arch/arm/dts/rk3568-u-boot.dtsi | 11 +++++++++++ 3 files changed, 13 insertions(+) create mode 100644 arch/arm/dts/rk3568-u-boot.dtsi
diff --git a/arch/arm/dts/rk3568-evb-u-boot.dtsi b/arch/arm/dts/rk3568-evb-u-boot.dtsi index 17503d3d27..77430da3ba 100644 --- a/arch/arm/dts/rk3568-evb-u-boot.dtsi +++ b/arch/arm/dts/rk3568-evb-u-boot.dtsi @@ -4,6 +4,7 @@ */
#include "rk356x-u-boot.dtsi" +#include "rk3568-u-boot.dtsi"
/ { chosen { diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi index ed47efa44b..44cf33ed4b 100644 --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi @@ -5,6 +5,7 @@ */
#include "rk356x-u-boot.dtsi" +#include "rk3568-u-boot.dtsi"
/ { chosen { diff --git a/arch/arm/dts/rk3568-u-boot.dtsi b/arch/arm/dts/rk3568-u-boot.dtsi new file mode 100644 index 0000000000..c6776666df --- /dev/null +++ b/arch/arm/dts/rk3568-u-boot.dtsi @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/*
- Copyright (c) 2023 Vasily Khoruzhick anarsoul@gmail.com
- */
+#include "rockchip-u-boot.dtsi"
+&combphy0 {
- /delete-property/ assigned-clocks;
- /delete-property/ assigned-clock-rates;
+};
2.39.2

On Tue, Mar 7, 2023 at 11:53 AM Mark Kettenis mark.kettenis@xs4all.nl wrote:
That is probably the wrong approach. It should be possible to boot an OS with the device tree provided by U-Boot. Removing these properties means the OS doesn't see them either. But if the assigned-clocks property isn't needed it wouldn't be there would it?
Well, should we revert 5bec4b0de785 in this case?
commit 5bec4b0de785 ("arm64: dts: rk356x-u-boot: Drop combphy1 assigned-clocks/rates") dropped these properties for combphy1, so let's drop them for combphy0 as well.
Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com
arch/arm/dts/rk3568-evb-u-boot.dtsi | 1 + arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 1 + arch/arm/dts/rk3568-u-boot.dtsi | 11 +++++++++++ 3 files changed, 13 insertions(+) create mode 100644 arch/arm/dts/rk3568-u-boot.dtsi
diff --git a/arch/arm/dts/rk3568-evb-u-boot.dtsi b/arch/arm/dts/rk3568-evb-u-boot.dtsi index 17503d3d27..77430da3ba 100644 --- a/arch/arm/dts/rk3568-evb-u-boot.dtsi +++ b/arch/arm/dts/rk3568-evb-u-boot.dtsi @@ -4,6 +4,7 @@ */
#include "rk356x-u-boot.dtsi" +#include "rk3568-u-boot.dtsi"
/ { chosen { diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi index ed47efa44b..44cf33ed4b 100644 --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi @@ -5,6 +5,7 @@ */
#include "rk356x-u-boot.dtsi" +#include "rk3568-u-boot.dtsi"
/ { chosen { diff --git a/arch/arm/dts/rk3568-u-boot.dtsi b/arch/arm/dts/rk3568-u-boot.dtsi new file mode 100644 index 0000000000..c6776666df --- /dev/null +++ b/arch/arm/dts/rk3568-u-boot.dtsi @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/*
- Copyright (c) 2023 Vasily Khoruzhick anarsoul@gmail.com
- */
+#include "rockchip-u-boot.dtsi"
+&combphy0 {
/delete-property/ assigned-clocks;
/delete-property/ assigned-clock-rates;
+};
2.39.2

From: Vasily Khoruzhick anarsoul@gmail.com Date: Tue, 7 Mar 2023 12:34:48 -0800
On Tue, Mar 7, 2023 at 11:53 AM Mark Kettenis mark.kettenis@xs4all.nl wrote:
That is probably the wrong approach. It should be possible to boot an OS with the device tree provided by U-Boot. Removing these properties means the OS doesn't see them either. But if the assigned-clocks property isn't needed it wouldn't be there would it?
Well, should we revert 5bec4b0de785 in this case?
And implement support for the CLK_PCIEPHYn_REF clocks in drivers/clk/rockchip/clk_rk3568.c:rk3568_pmuclk_set_rate()?
Yes, I'd say so.
If U-Boot doesn't actually need these clocks to run at the frequency provided by assigned-clock-rates, that could be as simple as returning 0 for these clocks.
commit 5bec4b0de785 ("arm64: dts: rk356x-u-boot: Drop combphy1 assigned-clocks/rates") dropped these properties for combphy1, so let's drop them for combphy0 as well.
Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com
arch/arm/dts/rk3568-evb-u-boot.dtsi | 1 + arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 1 + arch/arm/dts/rk3568-u-boot.dtsi | 11 +++++++++++ 3 files changed, 13 insertions(+) create mode 100644 arch/arm/dts/rk3568-u-boot.dtsi
diff --git a/arch/arm/dts/rk3568-evb-u-boot.dtsi b/arch/arm/dts/rk3568-evb-u-boot.dtsi index 17503d3d27..77430da3ba 100644 --- a/arch/arm/dts/rk3568-evb-u-boot.dtsi +++ b/arch/arm/dts/rk3568-evb-u-boot.dtsi @@ -4,6 +4,7 @@ */
#include "rk356x-u-boot.dtsi" +#include "rk3568-u-boot.dtsi"
/ { chosen { diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi index ed47efa44b..44cf33ed4b 100644 --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi @@ -5,6 +5,7 @@ */
#include "rk356x-u-boot.dtsi" +#include "rk3568-u-boot.dtsi"
/ { chosen { diff --git a/arch/arm/dts/rk3568-u-boot.dtsi b/arch/arm/dts/rk3568-u-boot.dtsi new file mode 100644 index 0000000000..c6776666df --- /dev/null +++ b/arch/arm/dts/rk3568-u-boot.dtsi @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/*
- Copyright (c) 2023 Vasily Khoruzhick anarsoul@gmail.com
- */
+#include "rockchip-u-boot.dtsi"
+&combphy0 {
/delete-property/ assigned-clocks;
/delete-property/ assigned-clock-rates;
+};
2.39.2

On Tue, Mar 7, 2023 at 1:04 PM Mark Kettenis mark.kettenis@xs4all.nl wrote:
And implement support for the CLK_PCIEPHYn_REF clocks in drivers/clk/rockchip/clk_rk3568.c:rk3568_pmuclk_set_rate()?
Yes, I'd say so.
If U-Boot doesn't actually need these clocks to run at the frequency provided by assigned-clock-rates, that could be as simple as returning 0 for these clocks.
Sounds good. I'll send v2 that reverts 5bec4b0de785 and implements stubs for CLK_PCIEPHYn_REF clocks.
FWIW, I tried booting linux-6.2.2 with u-boot dtb and these clocks dropped, and USB works just fine.
commit 5bec4b0de785 ("arm64: dts: rk356x-u-boot: Drop combphy1 assigned-clocks/rates") dropped these properties for combphy1, so let's drop them for combphy0 as well.
Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com
arch/arm/dts/rk3568-evb-u-boot.dtsi | 1 + arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 1 + arch/arm/dts/rk3568-u-boot.dtsi | 11 +++++++++++ 3 files changed, 13 insertions(+) create mode 100644 arch/arm/dts/rk3568-u-boot.dtsi
diff --git a/arch/arm/dts/rk3568-evb-u-boot.dtsi b/arch/arm/dts/rk3568-evb-u-boot.dtsi index 17503d3d27..77430da3ba 100644 --- a/arch/arm/dts/rk3568-evb-u-boot.dtsi +++ b/arch/arm/dts/rk3568-evb-u-boot.dtsi @@ -4,6 +4,7 @@ */
#include "rk356x-u-boot.dtsi" +#include "rk3568-u-boot.dtsi"
/ { chosen { diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi index ed47efa44b..44cf33ed4b 100644 --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi @@ -5,6 +5,7 @@ */
#include "rk356x-u-boot.dtsi" +#include "rk3568-u-boot.dtsi"
/ { chosen { diff --git a/arch/arm/dts/rk3568-u-boot.dtsi b/arch/arm/dts/rk3568-u-boot.dtsi new file mode 100644 index 0000000000..c6776666df --- /dev/null +++ b/arch/arm/dts/rk3568-u-boot.dtsi @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/*
- Copyright (c) 2023 Vasily Khoruzhick anarsoul@gmail.com
- */
+#include "rockchip-u-boot.dtsi"
+&combphy0 {
/delete-property/ assigned-clocks;
/delete-property/ assigned-clock-rates;
+};
2.39.2

Hi Vasily,
On 2023/3/8 05:34, Vasily Khoruzhick wrote:
On Tue, Mar 7, 2023 at 1:04 PM Mark Kettenis mark.kettenis@xs4all.nl wrote:
And implement support for the CLK_PCIEPHYn_REF clocks in drivers/clk/rockchip/clk_rk3568.c:rk3568_pmuclk_set_rate()?
Yes, I'd say so.
If U-Boot doesn't actually need these clocks to run at the frequency provided by assigned-clock-rates, that could be as simple as returning 0 for these clocks.
Sounds good. I'll send v2 that reverts 5bec4b0de785 and implements stubs for CLK_PCIEPHYn_REF clocks.
FWIW, I tried booting linux-6.2.2 with u-boot dtb and these clocks dropped, and USB works just fine.
The assigned-clock is a software set default value, the hardware can works fine because
these clocks have a correct default hardware value.
So for the PHY, it works in U-Boot without assigned clock in most case.
Returing 0 for those clocks is also OK.
Thanks,
- Kever
commit 5bec4b0de785 ("arm64: dts: rk356x-u-boot: Drop combphy1 assigned-clocks/rates") dropped these properties for combphy1, so let's drop them for combphy0 as well.
Signed-off-by: Vasily Khoruzhick anarsoul@gmail.com
arch/arm/dts/rk3568-evb-u-boot.dtsi | 1 + arch/arm/dts/rk3568-rock-3a-u-boot.dtsi | 1 + arch/arm/dts/rk3568-u-boot.dtsi | 11 +++++++++++ 3 files changed, 13 insertions(+) create mode 100644 arch/arm/dts/rk3568-u-boot.dtsi
diff --git a/arch/arm/dts/rk3568-evb-u-boot.dtsi b/arch/arm/dts/rk3568-evb-u-boot.dtsi index 17503d3d27..77430da3ba 100644 --- a/arch/arm/dts/rk3568-evb-u-boot.dtsi +++ b/arch/arm/dts/rk3568-evb-u-boot.dtsi @@ -4,6 +4,7 @@ */
#include "rk356x-u-boot.dtsi" +#include "rk3568-u-boot.dtsi"
/ { chosen { diff --git a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi index ed47efa44b..44cf33ed4b 100644 --- a/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi +++ b/arch/arm/dts/rk3568-rock-3a-u-boot.dtsi @@ -5,6 +5,7 @@ */
#include "rk356x-u-boot.dtsi" +#include "rk3568-u-boot.dtsi"
/ { chosen { diff --git a/arch/arm/dts/rk3568-u-boot.dtsi b/arch/arm/dts/rk3568-u-boot.dtsi new file mode 100644 index 0000000000..c6776666df --- /dev/null +++ b/arch/arm/dts/rk3568-u-boot.dtsi @@ -0,0 +1,11 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/*
- Copyright (c) 2023 Vasily Khoruzhick anarsoul@gmail.com
- */
+#include "rockchip-u-boot.dtsi"
+&combphy0 {
/delete-property/ assigned-clocks;
/delete-property/ assigned-clock-rates;
+};
2.39.2
participants (3)
-
Kever Yang
-
Mark Kettenis
-
Vasily Khoruzhick