Re: [PATCH 2/3] phy: rockchip-inno-usb2: add initial support for rk3588 PHY
On 3/9/23 11:11, Xavier Drudis Ferran wrote: El Wed, Mar 08, 2023 at 01:59:54PM +0200, Eugen Hristev deia: On 3/8/23 13:30, Xavier Drudis Ferran wrote: El Fri, Mar 03, 2023 at 09:31:33AM +0200, Eugen Hristev deia: @@ -105,6 +130,17 @@ static int rockchip_usb2phy_power_off(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); + struct udevice *vbus = NULL; + int ret; + + vbus = rockchip_usb2phy_check_vbus(phy); + if (vbus) { + ret = regulator_set_enable(vbus, false); + if (ret) { Could we have if (ret && ret != -EACCES ) { instead here ? (reason below) Hi, I have nothing against it, the regulator should be all the way optional IMO Well, at least if it is always-on for whatever reason, then it is not an error that it cannot be turned off. The apparent reason is that arch/arm/dts/rk3399-rock-pi-4.dtsi says vcc5v0_host: vcc5v0-host-regulator { compatible = "regulator-fixed"; enable-active-high; gpio = < RK_PD1 GPIO_ACTIVE_HIGH>; pinctrl-names = "default"; pinctrl-0 = <_host_en>; regulator-name = "vcc5v0_host"; /*/ regulator-always-on; /*/ Pretty weird that a regulator that can be turned on/off via a GPIO is 'regulator-always-on'. I find this odd and i think it's not correctly described at DT level. I don't know enough to tell. I've just looked a little and it seems to be used for USB only (on rock pi 4, firefly, eaidk-610, khadas-edge, leez-p710, nanopc-t4, orangepi, puma, rock960, rockpro64) Curiously rk3399-evb does NOT have regulator-always-on in vcc5v0_host and roc-pc seems to add it in u-boot.dtsi only, since it was preserved at some u-boot - linux sync. pinebook-pro has regulator-always-on, but then has regulator-state-mem, regulator-off-in-suspend... Anyway, maybe we should move on even if we can't disable the regulator in any case ? We should just dev_err and continue ? dev_err or not dev_err depends on whether always-on is always a bug there or may be a feature, I don't know. But moving on would be nice, yes. Have you tested your case with if (ret && ret != -EACCES ) { and it solves your usb reset problem ? Kever, do you have any preference ? Eugen Thanks
Re: [PATCH 2/3] phy: rockchip-inno-usb2: add initial support for rk3588 PHY
El Wed, Mar 08, 2023 at 01:59:54PM +0200, Eugen Hristev deia: > On 3/8/23 13:30, Xavier Drudis Ferran wrote: > > El Fri, Mar 03, 2023 at 09:31:33AM +0200, Eugen Hristev deia: > > > @@ -105,6 +130,17 @@ static int rockchip_usb2phy_power_off(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); > > > + struct udevice *vbus = NULL; > > > + int ret; > > > + > > > + vbus = rockchip_usb2phy_check_vbus(phy); > > > + if (vbus) { > > > + ret = regulator_set_enable(vbus, false); > > > + if (ret) { > > > > Could we have > > if (ret && ret != -EACCES ) { > > instead here ? > > (reason below) > Hi, > > I have nothing against it, the regulator should be all the way optional IMO > Well, at least if it is always-on for whatever reason, then it is not an error that it cannot be turned off. > > The apparent reason is that arch/arm/dts/rk3399-rock-pi-4.dtsi > > says > > > > vcc5v0_host: vcc5v0-host-regulator { > > compatible = "regulator-fixed"; > > enable-active-high; > > gpio = < RK_PD1 GPIO_ACTIVE_HIGH>; > > pinctrl-names = "default"; > > pinctrl-0 = <_host_en>; > > regulator-name = "vcc5v0_host"; > > > > /*/ regulator-always-on; /*/ > > Pretty weird that a regulator that can be turned on/off via a GPIO is > 'regulator-always-on'. I find this odd and i think it's not correctly > described at DT level. > I don't know enough to tell. I've just looked a little and it seems to be used for USB only (on rock pi 4, firefly, eaidk-610, khadas-edge, leez-p710, nanopc-t4, orangepi, puma, rock960, rockpro64) Curiously rk3399-evb does NOT have regulator-always-on in vcc5v0_host and roc-pc seems to add it in u-boot.dtsi only, since it was preserved at some u-boot - linux sync. pinebook-pro has regulator-always-on, but then has regulator-state-mem, regulator-off-in-suspend... > > Anyway, maybe we should move on even if we can't disable the regulator in > any case ? We should just dev_err and continue ? > dev_err or not dev_err depends on whether always-on is always a bug there or may be a feature, I don't know. But moving on would be nice, yes. > Kever, do you have any preference ? > > Eugen Thanks
Re: [PATCH 2/3] phy: rockchip-inno-usb2: add initial support for rk3588 PHY
On 3/8/23 13:30, Xavier Drudis Ferran wrote: El Fri, Mar 03, 2023 at 09:31:33AM +0200, Eugen Hristev deia: @@ -105,6 +130,17 @@ static int rockchip_usb2phy_power_off(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); + struct udevice *vbus = NULL; + int ret; + + vbus = rockchip_usb2phy_check_vbus(phy); + if (vbus) { + ret = regulator_set_enable(vbus, false); + if (ret) { Could we have if (ret && ret != -EACCES ) { instead here ? (reason below) Hi, I have nothing against it, the regulator should be all the way optional IMO + dev_err(phy->dev, "vbus disable failed: %d\n", ret); + return ret; + } + } property_enable(priv->reg_base, _cfg->phy_sus, true); Thank you. I've tested a little your patch (on an older master, 4eb7c5030d3f3c70 (2023-02-19) and some minor config changes that don't seem to matter). I could try on current master if needed, but I thought I'd ask already. In a Rock-pi 4B the ehci ports weren't working without your patch and they still don't work with your patch. With my patch (v5, [1]) they worked, but with my patch and yours usb reset fails: resetting USB... rockchip_usb2phy_port host-port: vbus disable failed: -13 rockchip_usb2phy_port host-port: PHY: Failed to power off host-port: -13. device_remove: Device 'usb@fe38' failed to remove, but children are gone rockchip_usb2phy_port host-port: vbus disable failed: -13 rockchip_usb2phy_port host-port: PHY: Failed to power off host-port: -13. device_remove: Device 'usb@fe3c' failed to remove, but children are gone Bus usb@fe38: Bus usb@fe3c: Bus usb@fe80: Register 2000140 NbrPorts 2 Starting the controller USB XHCI 1.10 Bus usb@fe90: Register 2000140 NbrPorts 2 Starting the controller USB XHCI 1.10 scanning bus usb@fe38 for devices... "Synchronous Abort" handler, esr 0x9610 elr: 0021de20 lr : 0021de20 (reloc) elr: f3f56e20 lr : f3f56e20 x0 : x1 : f1f19fef x2 : f1f19fee x3 : f1f53bc0 x4 : x5 : x6 : x7 : f1f6 x8 : 0100 x9 : 0008 x10: 0006 x11: 0080 x12: 0001869f x13: 869c x14: x15: 0002 x16: 7e4f2113 x17: 9a11f13e x18: f1f30d90 x19: f1f323c0 x20: f1f1a680 x21: f1f19fee x22: f1f19fef x23: 88404000 x24: f1f1a280 x25: 0001 x26: f1f1a680 x27: f1f1a0c0 x28: 0840 x29: f1f19f90 Code: aa0203f5 f944a413 aa1303e0 94003d8f (b9400401) Resetting CPU ... resetting ... The apparent reason is that arch/arm/dts/rk3399-rock-pi-4.dtsi says vcc5v0_host: vcc5v0-host-regulator { compatible = "regulator-fixed"; enable-active-high; gpio = < RK_PD1 GPIO_ACTIVE_HIGH>; pinctrl-names = "default"; pinctrl-0 = <_host_en>; regulator-name = "vcc5v0_host"; /*/ regulator-always-on; /*/ Pretty weird that a regulator that can be turned on/off via a GPIO is 'regulator-always-on'. I find this odd and i think it's not correctly described at DT level. vin-supply = <_sys>; }; [...] { status = "okay"; u2phy0_otg: otg-port { status = "okay"; }; u2phy0_host: host-port { phy-supply = <_host>; status = "okay"; }; }; { status = "okay"; u2phy1_otg: otg-port { status = "okay"; }; u2phy1_host: host-port { phy-supply = <_host>; status = "okay"; }; }; and the regulator_set_enable() in regulator-uclass.c when passed enable==0 will return -EACCES (meaning you can't turn off an always-on regulator). Anyway, maybe we should move on even if we can't disable the regulator in any case ? We should just dev_err and continue ? Kever, do you have any preference ? Eugen Now my patch is not accepted, so EHCI isn't working on Rock-pi-4, so the error won't show. But I suspect it may bite if v5 is ever accepted. And I think it would fail if EHCI is fixed in any other way. I may try my v1, v2 or v3 with your patch if you think it's worth it. Now I don't know what the right solution is : - ignore -EACCES in rockchip_usb2phy_power_off() (as above, seems easiest?) - check always_on in rockchip_usb2phy_power_off() before calling regulator_set_enable() - change .dts (but they come from linux, don't they?) - make sure somehow that things don't break even if
Re: [PATCH 2/3] phy: rockchip-inno-usb2: add initial support for rk3588 PHY
El Fri, Mar 03, 2023 at 09:31:33AM +0200, Eugen Hristev deia: > @@ -105,6 +130,17 @@ static int rockchip_usb2phy_power_off(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); > + struct udevice *vbus = NULL; > + int ret; > + > + vbus = rockchip_usb2phy_check_vbus(phy); > + if (vbus) { > + ret = regulator_set_enable(vbus, false); > + if (ret) { Could we have if (ret && ret != -EACCES ) { instead here ? (reason below) > + dev_err(phy->dev, "vbus disable failed: %d\n", ret); > + return ret; > + } > + } > > property_enable(priv->reg_base, _cfg->phy_sus, true); > Thank you. I've tested a little your patch (on an older master, 4eb7c5030d3f3c70 (2023-02-19) and some minor config changes that don't seem to matter). I could try on current master if needed, but I thought I'd ask already. In a Rock-pi 4B the ehci ports weren't working without your patch and they still don't work with your patch. With my patch (v5, [1]) they worked, but with my patch and yours usb reset fails: resetting USB... rockchip_usb2phy_port host-port: vbus disable failed: -13 rockchip_usb2phy_port host-port: PHY: Failed to power off host-port: -13. device_remove: Device 'usb@fe38' failed to remove, but children are gone rockchip_usb2phy_port host-port: vbus disable failed: -13 rockchip_usb2phy_port host-port: PHY: Failed to power off host-port: -13. device_remove: Device 'usb@fe3c' failed to remove, but children are gone Bus usb@fe38: Bus usb@fe3c: Bus usb@fe80: Register 2000140 NbrPorts 2 Starting the controller USB XHCI 1.10 Bus usb@fe90: Register 2000140 NbrPorts 2 Starting the controller USB XHCI 1.10 scanning bus usb@fe38 for devices... "Synchronous Abort" handler, esr 0x9610 elr: 0021de20 lr : 0021de20 (reloc) elr: f3f56e20 lr : f3f56e20 x0 : x1 : f1f19fef x2 : f1f19fee x3 : f1f53bc0 x4 : x5 : x6 : x7 : f1f6 x8 : 0100 x9 : 0008 x10: 0006 x11: 0080 x12: 0001869f x13: 869c x14: x15: 0002 x16: 7e4f2113 x17: 9a11f13e x18: f1f30d90 x19: f1f323c0 x20: f1f1a680 x21: f1f19fee x22: f1f19fef x23: 88404000 x24: f1f1a280 x25: 0001 x26: f1f1a680 x27: f1f1a0c0 x28: 0840 x29: f1f19f90 Code: aa0203f5 f944a413 aa1303e0 94003d8f (b9400401) Resetting CPU ... resetting ... The apparent reason is that arch/arm/dts/rk3399-rock-pi-4.dtsi says vcc5v0_host: vcc5v0-host-regulator { compatible = "regulator-fixed"; enable-active-high; gpio = < RK_PD1 GPIO_ACTIVE_HIGH>; pinctrl-names = "default"; pinctrl-0 = <_host_en>; regulator-name = "vcc5v0_host"; /*/ regulator-always-on; /*/ vin-supply = <_sys>; }; [...] { status = "okay"; u2phy0_otg: otg-port { status = "okay"; }; u2phy0_host: host-port { phy-supply = <_host>; status = "okay"; }; }; { status = "okay"; u2phy1_otg: otg-port { status = "okay"; }; u2phy1_host: host-port { phy-supply = <_host>; status = "okay"; }; }; and the regulator_set_enable() in regulator-uclass.c when passed enable==0 will return -EACCES (meaning you can't turn off an always-on regulator). Now my patch is not accepted, so EHCI isn't working on Rock-pi-4, so the error won't show. But I suspect it may bite if v5 is ever accepted. And I think it would fail if EHCI is fixed in any other way. I may try my v1, v2 or v3 with your patch if you think it's worth it. Now I don't know what the right solution is : - ignore -EACCES in rockchip_usb2phy_power_off() (as above, seems easiest?) - check always_on in rockchip_usb2phy_power_off() before calling regulator_set_enable() - change .dts (but they come from linux, don't they?) - make sure somehow that things don't break even if rockchip_usb2phy_power_off() exits early with error. Btw, my v5 seems not to be correctly in patchwork, should I resend ? I guess I messed up subject lines ? I think there are other rk3399 boards that have similar .dtsi files, but I haven't investigated thoroughly. Do they all have broken EHCI ? If someone has a rk3399 board with working EHCI maybe they should test your patch and try usb start ; usb stop and usb start ; usb reset ? [1]
Re: [PATCH 2/3] phy: rockchip-inno-usb2: add initial support for rk3588 PHY
On Thu, Mar 2, 2023 at 11:32 PM Eugen Hristev wrote: > > Add initial support for the rk3588 PHY variant. > The driver now looks for phy-supply and enables/disables the vbus > accordingly. > The lookup for the host-port reg inside the struct now does a do {} while() > instead of a while() {} in order to allow a first check for reg == 0. > > Co-developed-by: Frank Wang > Signed-off-by: Frank Wang > Signed-off-by: Eugen Hristev Tested-by: Vasily Khoruzhick # rk3568-based board > --- > drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 109 +- > 1 file changed, 104 insertions(+), 5 deletions(-) > > diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > index 55e1dbcfef7e..0551876436d5 100644 > --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c > @@ -13,6 +13,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -61,6 +62,7 @@ struct rockchip_usb2phy_cfg { > struct rockchip_usb2phy { > void *reg_base; > struct clk phyclk; > + struct udevice *vbus_supply[USB2PHY_NUM_PORTS]; > const struct rockchip_usb2phy_cfg *phy_cfg; > }; > > @@ -86,11 +88,34 @@ struct rockchip_usb2phy_port_cfg *us2phy_get_port(struct > phy *phy) > return _cfg->port_cfgs[phy->id]; > } > > +static struct udevice *rockchip_usb2phy_check_vbus(struct phy *phy) > +{ > + struct udevice *parent = phy->dev->parent; > + struct rockchip_usb2phy *priv = dev_get_priv(parent); > + struct udevice *vbus = NULL; > + > + if (phy->id == USB2PHY_PORT_HOST) > + vbus = priv->vbus_supply[USB2PHY_PORT_HOST]; > + > + return vbus; > +} > + > 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); > + struct udevice *vbus = NULL; > + int ret; > + > + vbus = rockchip_usb2phy_check_vbus(phy); > + if (vbus) { > + ret = regulator_set_enable(vbus, true); > + if (ret) { > + dev_err(phy->dev, "vbus enable failed: %d\n", ret); > + return ret; > + } > + } > > property_enable(priv->reg_base, _cfg->phy_sus, false); > > @@ -105,6 +130,17 @@ static int rockchip_usb2phy_power_off(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); > + struct udevice *vbus = NULL; > + int ret; > + > + vbus = rockchip_usb2phy_check_vbus(phy); > + if (vbus) { > + ret = regulator_set_enable(vbus, false); > + if (ret) { > + dev_err(phy->dev, "vbus disable failed: %d\n", ret); > + return ret; > + } > + } > > property_enable(priv->reg_base, _cfg->phy_sus, true); > > @@ -149,13 +185,20 @@ static int rockchip_usb2phy_of_xlate(struct phy *phy, > struct ofnode_phandle_args *args) > { > const char *name = phy->dev->name; > + struct udevice *parent = phy->dev->parent; > + struct rockchip_usb2phy *priv = dev_get_priv(parent); > > - if (!strcasecmp(name, "host-port")) > + if (!strcasecmp(name, "host-port")) { > phy->id = USB2PHY_PORT_HOST; > - else if (!strcasecmp(name, "otg-port")) > + device_get_supply_regulator(phy->dev, "phy-supply", > + > >vbus_supply[USB2PHY_PORT_HOST]); > + } else if (!strcasecmp(name, "otg-port")) { > phy->id = USB2PHY_PORT_OTG; > - else > + device_get_supply_regulator(phy->dev, "phy-supply", > + > >vbus_supply[USB2PHY_PORT_OTG]); > + } else { > dev_err(phy->dev, "improper %s device\n", name); > + } > > return 0; > } > @@ -201,14 +244,14 @@ static int rockchip_usb2phy_probe(struct udevice *dev) > > /* find out a proper config which can be matched with dt. */ > index = 0; > - while (phy_cfgs[index].reg) { > + do { > if (phy_cfgs[index].reg == reg) { > priv->phy_cfg = _cfgs[index]; > break; > } > > ++index; > - } > + } while (phy_cfgs[index].reg); > > if (!priv->phy_cfg) { > dev_err(dev, "failed find proper phy-cfg\n"); > @@ -348,6 +391,58 @@ static const struct rockchip_usb2phy_cfg > rk3568_phy_cfgs[] = { > { /* sentinel */ } > }; > > +static const struct
Re: [PATCH 2/3] phy: rockchip-inno-usb2: add initial support for rk3588 PHY
On 2023/3/3 15:31, Eugen Hristev wrote: Add initial support for the rk3588 PHY variant. The driver now looks for phy-supply and enables/disables the vbus accordingly. The lookup for the host-port reg inside the struct now does a do {} while() instead of a while() {} in order to allow a first check for reg == 0. Co-developed-by: Frank Wang Signed-off-by: Frank Wang Signed-off-by: Eugen Hristev Reviewed-by: Kever Yang Thanks, - Kever --- drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 109 +- 1 file changed, 104 insertions(+), 5 deletions(-) diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c index 55e1dbcfef7e..0551876436d5 100644 --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -61,6 +62,7 @@ struct rockchip_usb2phy_cfg { struct rockchip_usb2phy { void *reg_base; struct clk phyclk; + struct udevice *vbus_supply[USB2PHY_NUM_PORTS]; const struct rockchip_usb2phy_cfg *phy_cfg; }; @@ -86,11 +88,34 @@ struct rockchip_usb2phy_port_cfg *us2phy_get_port(struct phy *phy) return _cfg->port_cfgs[phy->id]; } +static struct udevice *rockchip_usb2phy_check_vbus(struct phy *phy) +{ + struct udevice *parent = phy->dev->parent; + struct rockchip_usb2phy *priv = dev_get_priv(parent); + struct udevice *vbus = NULL; + + if (phy->id == USB2PHY_PORT_HOST) + vbus = priv->vbus_supply[USB2PHY_PORT_HOST]; + + return vbus; +} + 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); + struct udevice *vbus = NULL; + int ret; + + vbus = rockchip_usb2phy_check_vbus(phy); + if (vbus) { + ret = regulator_set_enable(vbus, true); + if (ret) { + dev_err(phy->dev, "vbus enable failed: %d\n", ret); + return ret; + } + } property_enable(priv->reg_base, _cfg->phy_sus, false); @@ -105,6 +130,17 @@ static int rockchip_usb2phy_power_off(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); + struct udevice *vbus = NULL; + int ret; + + vbus = rockchip_usb2phy_check_vbus(phy); + if (vbus) { + ret = regulator_set_enable(vbus, false); + if (ret) { + dev_err(phy->dev, "vbus disable failed: %d\n", ret); + return ret; + } + } property_enable(priv->reg_base, _cfg->phy_sus, true); @@ -149,13 +185,20 @@ static int rockchip_usb2phy_of_xlate(struct phy *phy, struct ofnode_phandle_args *args) { const char *name = phy->dev->name; + struct udevice *parent = phy->dev->parent; + struct rockchip_usb2phy *priv = dev_get_priv(parent); - if (!strcasecmp(name, "host-port")) + if (!strcasecmp(name, "host-port")) { phy->id = USB2PHY_PORT_HOST; - else if (!strcasecmp(name, "otg-port")) + device_get_supply_regulator(phy->dev, "phy-supply", + >vbus_supply[USB2PHY_PORT_HOST]); + } else if (!strcasecmp(name, "otg-port")) { phy->id = USB2PHY_PORT_OTG; - else + device_get_supply_regulator(phy->dev, "phy-supply", + >vbus_supply[USB2PHY_PORT_OTG]); + } else { dev_err(phy->dev, "improper %s device\n", name); + } return 0; } @@ -201,14 +244,14 @@ static int rockchip_usb2phy_probe(struct udevice *dev) /* find out a proper config which can be matched with dt. */ index = 0; - while (phy_cfgs[index].reg) { + do { if (phy_cfgs[index].reg == reg) { priv->phy_cfg = _cfgs[index]; break; } ++index; - } + } while (phy_cfgs[index].reg); if (!priv->phy_cfg) { dev_err(dev, "failed find proper phy-cfg\n"); @@ -348,6 +391,58 @@ static const struct rockchip_usb2phy_cfg rk3568_phy_cfgs[] = { { /* sentinel */ } }; +static const struct rockchip_usb2phy_cfg rk3588_phy_cfgs[] = { + { + .reg= 0x, + .port_cfgs = { + [USB2PHY_PORT_OTG] = { + .phy_sus= { 0x000c, 11, 11, 0, 1 }, +
[PATCH 2/3] phy: rockchip-inno-usb2: add initial support for rk3588 PHY
Add initial support for the rk3588 PHY variant. The driver now looks for phy-supply and enables/disables the vbus accordingly. The lookup for the host-port reg inside the struct now does a do {} while() instead of a while() {} in order to allow a first check for reg == 0. Co-developed-by: Frank Wang Signed-off-by: Frank Wang Signed-off-by: Eugen Hristev --- drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 109 +- 1 file changed, 104 insertions(+), 5 deletions(-) diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c index 55e1dbcfef7e..0551876436d5 100644 --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c @@ -13,6 +13,7 @@ #include #include #include +#include #include #include #include @@ -61,6 +62,7 @@ struct rockchip_usb2phy_cfg { struct rockchip_usb2phy { void *reg_base; struct clk phyclk; + struct udevice *vbus_supply[USB2PHY_NUM_PORTS]; const struct rockchip_usb2phy_cfg *phy_cfg; }; @@ -86,11 +88,34 @@ struct rockchip_usb2phy_port_cfg *us2phy_get_port(struct phy *phy) return _cfg->port_cfgs[phy->id]; } +static struct udevice *rockchip_usb2phy_check_vbus(struct phy *phy) +{ + struct udevice *parent = phy->dev->parent; + struct rockchip_usb2phy *priv = dev_get_priv(parent); + struct udevice *vbus = NULL; + + if (phy->id == USB2PHY_PORT_HOST) + vbus = priv->vbus_supply[USB2PHY_PORT_HOST]; + + return vbus; +} + 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); + struct udevice *vbus = NULL; + int ret; + + vbus = rockchip_usb2phy_check_vbus(phy); + if (vbus) { + ret = regulator_set_enable(vbus, true); + if (ret) { + dev_err(phy->dev, "vbus enable failed: %d\n", ret); + return ret; + } + } property_enable(priv->reg_base, _cfg->phy_sus, false); @@ -105,6 +130,17 @@ static int rockchip_usb2phy_power_off(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); + struct udevice *vbus = NULL; + int ret; + + vbus = rockchip_usb2phy_check_vbus(phy); + if (vbus) { + ret = regulator_set_enable(vbus, false); + if (ret) { + dev_err(phy->dev, "vbus disable failed: %d\n", ret); + return ret; + } + } property_enable(priv->reg_base, _cfg->phy_sus, true); @@ -149,13 +185,20 @@ static int rockchip_usb2phy_of_xlate(struct phy *phy, struct ofnode_phandle_args *args) { const char *name = phy->dev->name; + struct udevice *parent = phy->dev->parent; + struct rockchip_usb2phy *priv = dev_get_priv(parent); - if (!strcasecmp(name, "host-port")) + if (!strcasecmp(name, "host-port")) { phy->id = USB2PHY_PORT_HOST; - else if (!strcasecmp(name, "otg-port")) + device_get_supply_regulator(phy->dev, "phy-supply", + >vbus_supply[USB2PHY_PORT_HOST]); + } else if (!strcasecmp(name, "otg-port")) { phy->id = USB2PHY_PORT_OTG; - else + device_get_supply_regulator(phy->dev, "phy-supply", + >vbus_supply[USB2PHY_PORT_OTG]); + } else { dev_err(phy->dev, "improper %s device\n", name); + } return 0; } @@ -201,14 +244,14 @@ static int rockchip_usb2phy_probe(struct udevice *dev) /* find out a proper config which can be matched with dt. */ index = 0; - while (phy_cfgs[index].reg) { + do { if (phy_cfgs[index].reg == reg) { priv->phy_cfg = _cfgs[index]; break; } ++index; - } + } while (phy_cfgs[index].reg); if (!priv->phy_cfg) { dev_err(dev, "failed find proper phy-cfg\n"); @@ -348,6 +391,58 @@ static const struct rockchip_usb2phy_cfg rk3568_phy_cfgs[] = { { /* sentinel */ } }; +static const struct rockchip_usb2phy_cfg rk3588_phy_cfgs[] = { + { + .reg= 0x, + .port_cfgs = { + [USB2PHY_PORT_OTG] = { + .phy_sus= { 0x000c, 11, 11, 0, 1 }, + .ls_det_en = { 0x0080, 0, 0, 0, 1 }, + .ls_det_st =