Re: [PATCH 2/3] phy: rockchip-inno-usb2: add initial support for rk3588 PHY

2023-03-09 Thread Eugen Hristev

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

2023-03-09 Thread Xavier Drudis Ferran
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

2023-03-08 Thread Eugen Hristev

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

2023-03-08 Thread Xavier Drudis Ferran
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

2023-03-07 Thread Vasily Khoruzhick
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

2023-03-07 Thread Kever Yang



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

2023-03-02 Thread Eugen Hristev
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  =