With USB type C connectors, the vbus detect pin of the OTG controller
attached to it is pulled high by a USB Type C controller chip such as
the fusb302. This means USB enumeration on Type-C ports never works, as
the vbus is always seen as high.

Rockchip added some GRF register flags to deal with this situation. The
RK3576 TRM calls these "soft_vbusvalid_bvalid" (con0 bit index 15) and
"soft_vbusvalid_bvalid_sel" (con0 bit index 14).

Downstream introduces a new vendor property which tells the USB 2 PHY
that it's connected to a type C port, but we can do better. Since in
such an arrangement, we'll have an OF graph connection from the USB
controller to the USB connector anyway, we can walk said OF graph and
check the connector's compatible to determine this without adding any
further vendor properties.

Do keep in mind that the usbdp PHY driver seemingly fiddles with these
register fields as well, but what it does doesn't appear to be enough
for us to get working USB enumeration, presumably because the whole
vbus_attach logic needs to be adjusted as well either way.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 108 +++++++++++++++++++++++++-
 1 file changed, 104 insertions(+), 4 deletions(-)

diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
index b0f23690ec3002202c0f33a6988f5509622fa10e..71810c07e4150ea81f65a8a932541b144e95a137 100644
--- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
+++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/mutex.h>
 #include <linux/of.h>
+#include <linux/of_graph.h>
 #include <linux/of_irq.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
@@ -114,6 +115,8 @@ struct rockchip_chg_det_reg {
 /**
  * struct rockchip_usb2phy_port_cfg - usb-phy port configuration.
  * @phy_sus: phy suspend register.
+ * @svbus_en: soft vbus bvalid enable register.
+ * @svbus_sel: soft vbus bvalid selection register.
  * @bvalid_det_en: vbus valid rise detection enable register.
  * @bvalid_det_st: vbus valid rise detection status register.
  * @bvalid_det_clr: vbus valid rise detection clear register.
@@ -140,6 +143,8 @@ struct rockchip_chg_det_reg {
  */
 struct rockchip_usb2phy_port_cfg {
 	struct usb2phy_reg	phy_sus;
+	struct usb2phy_reg	svbus_en;
+	struct usb2phy_reg	svbus_sel;
 	struct usb2phy_reg	bvalid_det_en;
 	struct usb2phy_reg	bvalid_det_st;
 	struct usb2phy_reg	bvalid_det_clr;
@@ -203,6 +208,7 @@ struct rockchip_usb2phy_cfg {
  * @event_nb: hold event notification callback.
  * @state: define OTG enumeration states before device reset.
  * @mode: the dr_mode of the controller.
+ * @typec_vbus_det: whether to apply Type C logic to OTG vbus detection.
  */
 struct rockchip_usb2phy_port {
 	struct phy	*phy;
@@ -222,6 +228,7 @@ struct rockchip_usb2phy_port {
 	struct notifier_block	event_nb;
 	enum usb_otg_state	state;
 	enum usb_dr_mode	mode;
+	bool typec_vbus_det;
 };
 
 /**
@@ -495,6 +502,13 @@ static int rockchip_usb2phy_init(struct phy *phy)
 	mutex_lock(&rport->mutex);
 
 	if (rport->port_id == USB2PHY_PORT_OTG) {
+		if (rport->typec_vbus_det) {
+			if (rport->port_cfg->svbus_en.enable &&
+					rport->port_cfg->svbus_sel.enable) {
+				property_enable(rphy->grf, &rport->port_cfg->svbus_en, true);
+				property_enable(rphy->grf, &rport->port_cfg->svbus_sel, true);
+			}
+		}
 		if (rport->mode != USB_DR_MODE_HOST &&
 		    rport->mode != USB_DR_MODE_UNKNOWN) {
 			/* clear bvalid status and enable bvalid detect irq */
@@ -535,8 +549,7 @@ static int rockchip_usb2phy_init(struct phy *phy)
 			if (ret)
 				goto out;
 
-			schedule_delayed_work(&rport->otg_sm_work,
-					      OTG_SCHEDULE_DELAY * 3);
+			schedule_delayed_work(&rport->otg_sm_work, 0);
 		} else {
 			/* If OTG works in host only mode, do nothing. */
 			dev_dbg(&rport->phy->dev, "mode %d\n", rport->mode);
@@ -666,8 +679,12 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
 	unsigned long delay;
 	bool vbus_attach, sch_work, notify_charger;
 
-	vbus_attach = property_enabled(rphy->grf,
-				       &rport->port_cfg->utmi_bvalid);
+	if (rport->port_cfg->svbus_en.enable && rport->typec_vbus_det) {
+		vbus_attach = true;
+	} else {
+		vbus_attach = property_enabled(rphy->grf,
+					       &rport->port_cfg->utmi_bvalid);
+	}
 
 	sch_work = false;
 	notify_charger = false;
@@ -1276,6 +1293,83 @@ static int rockchip_otg_event(struct notifier_block *nb,
 	return NOTIFY_DONE;
 }
 
+static const char *const rockchip_usb2phy_typec_cons[] = {
+	"usb-c-connector",
+	NULL,
+};
+
+static struct device_node *rockchip_usb2phy_to_controller(struct rockchip_usb2phy *rphy)
+{
+	struct device_node *np;
+	struct device_node *parent;
+
+	for_each_node_with_property(np, "phys") {
+		struct of_phandle_iterator it;
+		int ret;
+
+		of_for_each_phandle(&it, ret, np, "phys", NULL, 0) {
+			parent = of_get_parent(it.node);
+			if (it.node != rphy->dev->of_node && rphy->dev->of_node != parent) {
+				if (parent)
+					of_node_put(parent);
+				continue;
+			}
+
+			/*
+			 * Either the PHY phandle we're iterating or its parent
+			 * matched, we don't care about which out of the two in
+			 * particular as we just need to know it's the right
+			 * USB controller for this PHY.
+			 */
+			of_node_put(it.node);
+			of_node_put(parent);
+			return np;
+		}
+	}
+
+	return NULL;
+}
+
+static bool rockchip_usb2phy_otg_is_type_c(struct rockchip_usb2phy *rphy)
+{
+	struct device_node *controller = rockchip_usb2phy_to_controller(rphy);
+	struct device_node *ports;
+	struct device_node *ep = NULL;
+	struct device_node *parent;
+
+	if (!controller)
+		return false;
+
+	ports = of_get_child_by_name(controller, "ports");
+	if (ports) {
+		of_node_put(controller);
+		controller = ports;
+	}
+
+	for_each_of_graph_port(controller, port) {
+		ep = of_get_child_by_name(port, "endpoint");
+		if (!ep)
+			continue;
+
+		parent = of_graph_get_remote_port_parent(ep);
+		of_node_put(ep);
+		if (!parent)
+			continue;
+
+		if (of_device_compatible_match(parent, rockchip_usb2phy_typec_cons)) {
+			of_node_put(parent);
+			of_node_put(controller);
+			return true;
+		}
+
+		of_node_put(parent);
+	}
+
+	of_node_put(controller);
+
+	return false;
+}
+
 static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
 					  struct rockchip_usb2phy_port *rport,
 					  struct device_node *child_np)
@@ -1297,6 +1391,8 @@ static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
 
 	mutex_init(&rport->mutex);
 
+	rport->typec_vbus_det = rockchip_usb2phy_otg_is_type_c(rphy);
+
 	rport->mode = of_usb_get_dr_mode_by_phy(child_np, -1);
 	if (rport->mode == USB_DR_MODE_HOST ||
 	    rport->mode == USB_DR_MODE_UNKNOWN) {
@@ -2050,6 +2146,8 @@ static const struct rockchip_usb2phy_cfg rk3576_phy_cfgs[] = {
 		.port_cfgs	= {
 			[USB2PHY_PORT_OTG] = {
 				.phy_sus	= { 0x0000, 8, 0, 0, 0x1d1 },
+				.svbus_en	= { 0x0000, 15, 15, 0, 1 },
+				.svbus_sel	= { 0x0000, 14, 14, 0, 1 },
 				.bvalid_det_en	= { 0x00c0, 1, 1, 0, 1 },
 				.bvalid_det_st	= { 0x00c4, 1, 1, 0, 1 },
 				.bvalid_det_clr = { 0x00c8, 1, 1, 0, 1 },
@@ -2087,6 +2185,8 @@ static const struct rockchip_usb2phy_cfg rk3576_phy_cfgs[] = {
 		.port_cfgs	= {
 			[USB2PHY_PORT_OTG] = {
 				.phy_sus	= { 0x2000, 8, 0, 0, 0x1d1 },
+				.svbus_en	= { 0x2000, 15, 15, 0, 1 },
+				.svbus_sel	= { 0x2000, 14, 14, 0, 1 },
 				.bvalid_det_en	= { 0x20c0, 1, 1, 0, 1 },
 				.bvalid_det_st	= { 0x20c4, 1, 1, 0, 1 },
 				.bvalid_det_clr = { 0x20c8, 1, 1, 0, 1 },

-- 
2.49.0

Hi Nicolas,

Am Donnerstag, 19. Juni 2025, 20:36:36 Mitteleuropäische Sommerzeit schrieb Nicolas Frattaroli:
> With USB type C connectors, the vbus detect pin of the OTG controller
> attached to it is pulled high by a USB Type C controller chip such as
> the fusb302. This means USB enumeration on Type-C ports never works, as
> the vbus is always seen as high.
> 
> Rockchip added some GRF register flags to deal with this situation. The
> RK3576 TRM calls these "soft_vbusvalid_bvalid" (con0 bit index 15) and
> "soft_vbusvalid_bvalid_sel" (con0 bit index 14).

I would expect a paragraph more about what those bits (and their
functionality) actually do here :-) 


> Downstream introduces a new vendor property which tells the USB 2 PHY
> that it's connected to a type C port, but we can do better. Since in
> such an arrangement, we'll have an OF graph connection from the USB
> controller to the USB connector anyway, we can walk said OF graph and
> check the connector's compatible to determine this without adding any
> further vendor properties.
> 
> Do keep in mind that the usbdp PHY driver seemingly fiddles with these
> register fields as well, but what it does doesn't appear to be enough
> for us to get working USB enumeration, presumably because the whole
> vbus_attach logic needs to be adjusted as well either way.


In the rk3588 TRM I find USB2PHY_GRF_CON4
bit3 - sft_vbus_sel (VBUS software control select)
bit2 - sft_vbus (When sft_vbus_sel is 1'b1, vbusvalid and bvalid is
                 controlled by sft_vbus)

Is that the same stuff as you're adding for rk3576 ?

My last dance with rk3588-type-c is already some months back, but I do
remember running into "some" issue - but don't remember which one ;-)


Heiko

> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 108 +++++++++++++++++++++++++-
>  1 file changed, 104 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> index b0f23690ec3002202c0f33a6988f5509622fa10e..71810c07e4150ea81f65a8a932541b144e95a137 100644
> --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> @@ -17,6 +17,7 @@
>  #include <linux/module.h>
>  #include <linux/mutex.h>
>  #include <linux/of.h>
> +#include <linux/of_graph.h>
>  #include <linux/of_irq.h>
>  #include <linux/phy/phy.h>
>  #include <linux/platform_device.h>
> @@ -114,6 +115,8 @@ struct rockchip_chg_det_reg {
>  /**
>   * struct rockchip_usb2phy_port_cfg - usb-phy port configuration.
>   * @phy_sus: phy suspend register.
> + * @svbus_en: soft vbus bvalid enable register.
> + * @svbus_sel: soft vbus bvalid selection register.
>   * @bvalid_det_en: vbus valid rise detection enable register.
>   * @bvalid_det_st: vbus valid rise detection status register.
>   * @bvalid_det_clr: vbus valid rise detection clear register.
> @@ -140,6 +143,8 @@ struct rockchip_chg_det_reg {
>   */
>  struct rockchip_usb2phy_port_cfg {
>  	struct usb2phy_reg	phy_sus;
> +	struct usb2phy_reg	svbus_en;
> +	struct usb2phy_reg	svbus_sel;
>  	struct usb2phy_reg	bvalid_det_en;
>  	struct usb2phy_reg	bvalid_det_st;
>  	struct usb2phy_reg	bvalid_det_clr;
> @@ -203,6 +208,7 @@ struct rockchip_usb2phy_cfg {
>   * @event_nb: hold event notification callback.
>   * @state: define OTG enumeration states before device reset.
>   * @mode: the dr_mode of the controller.
> + * @typec_vbus_det: whether to apply Type C logic to OTG vbus detection.
>   */
>  struct rockchip_usb2phy_port {
>  	struct phy	*phy;
> @@ -222,6 +228,7 @@ struct rockchip_usb2phy_port {
>  	struct notifier_block	event_nb;
>  	enum usb_otg_state	state;
>  	enum usb_dr_mode	mode;
> +	bool typec_vbus_det;
>  };
>  
>  /**
> @@ -495,6 +502,13 @@ static int rockchip_usb2phy_init(struct phy *phy)
>  	mutex_lock(&rport->mutex);
>  
>  	if (rport->port_id == USB2PHY_PORT_OTG) {
> +		if (rport->typec_vbus_det) {
> +			if (rport->port_cfg->svbus_en.enable &&
> +					rport->port_cfg->svbus_sel.enable) {
> +				property_enable(rphy->grf, &rport->port_cfg->svbus_en, true);
> +				property_enable(rphy->grf, &rport->port_cfg->svbus_sel, true);
> +			}
> +		}
>  		if (rport->mode != USB_DR_MODE_HOST &&
>  		    rport->mode != USB_DR_MODE_UNKNOWN) {
>  			/* clear bvalid status and enable bvalid detect irq */
> @@ -535,8 +549,7 @@ static int rockchip_usb2phy_init(struct phy *phy)
>  			if (ret)
>  				goto out;
>  
> -			schedule_delayed_work(&rport->otg_sm_work,
> -					      OTG_SCHEDULE_DELAY * 3);
> +			schedule_delayed_work(&rport->otg_sm_work, 0);
>  		} else {
>  			/* If OTG works in host only mode, do nothing. */
>  			dev_dbg(&rport->phy->dev, "mode %d\n", rport->mode);
> @@ -666,8 +679,12 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
>  	unsigned long delay;
>  	bool vbus_attach, sch_work, notify_charger;
>  
> -	vbus_attach = property_enabled(rphy->grf,
> -				       &rport->port_cfg->utmi_bvalid);
> +	if (rport->port_cfg->svbus_en.enable && rport->typec_vbus_det) {
> +		vbus_attach = true;
> +	} else {
> +		vbus_attach = property_enabled(rphy->grf,
> +					       &rport->port_cfg->utmi_bvalid);
> +	}
>  
>  	sch_work = false;
>  	notify_charger = false;
> @@ -1276,6 +1293,83 @@ static int rockchip_otg_event(struct notifier_block *nb,
>  	return NOTIFY_DONE;
>  }
>  
> +static const char *const rockchip_usb2phy_typec_cons[] = {
> +	"usb-c-connector",
> +	NULL,
> +};
> +
> +static struct device_node *rockchip_usb2phy_to_controller(struct rockchip_usb2phy *rphy)
> +{
> +	struct device_node *np;
> +	struct device_node *parent;
> +
> +	for_each_node_with_property(np, "phys") {
> +		struct of_phandle_iterator it;
> +		int ret;
> +
> +		of_for_each_phandle(&it, ret, np, "phys", NULL, 0) {
> +			parent = of_get_parent(it.node);
> +			if (it.node != rphy->dev->of_node && rphy->dev->of_node != parent) {
> +				if (parent)
> +					of_node_put(parent);
> +				continue;
> +			}
> +
> +			/*
> +			 * Either the PHY phandle we're iterating or its parent
> +			 * matched, we don't care about which out of the two in
> +			 * particular as we just need to know it's the right
> +			 * USB controller for this PHY.
> +			 */
> +			of_node_put(it.node);
> +			of_node_put(parent);
> +			return np;
> +		}
> +	}
> +
> +	return NULL;
> +}
> +
> +static bool rockchip_usb2phy_otg_is_type_c(struct rockchip_usb2phy *rphy)
> +{
> +	struct device_node *controller = rockchip_usb2phy_to_controller(rphy);
> +	struct device_node *ports;
> +	struct device_node *ep = NULL;
> +	struct device_node *parent;
> +
> +	if (!controller)
> +		return false;
> +
> +	ports = of_get_child_by_name(controller, "ports");
> +	if (ports) {
> +		of_node_put(controller);
> +		controller = ports;
> +	}
> +
> +	for_each_of_graph_port(controller, port) {
> +		ep = of_get_child_by_name(port, "endpoint");
> +		if (!ep)
> +			continue;
> +
> +		parent = of_graph_get_remote_port_parent(ep);
> +		of_node_put(ep);
> +		if (!parent)
> +			continue;
> +
> +		if (of_device_compatible_match(parent, rockchip_usb2phy_typec_cons)) {
> +			of_node_put(parent);
> +			of_node_put(controller);
> +			return true;
> +		}
> +
> +		of_node_put(parent);
> +	}
> +
> +	of_node_put(controller);
> +
> +	return false;
> +}
> +
>  static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
>  					  struct rockchip_usb2phy_port *rport,
>  					  struct device_node *child_np)
> @@ -1297,6 +1391,8 @@ static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
>  
>  	mutex_init(&rport->mutex);
>  
> +	rport->typec_vbus_det = rockchip_usb2phy_otg_is_type_c(rphy);
> +
>  	rport->mode = of_usb_get_dr_mode_by_phy(child_np, -1);
>  	if (rport->mode == USB_DR_MODE_HOST ||
>  	    rport->mode == USB_DR_MODE_UNKNOWN) {
> @@ -2050,6 +2146,8 @@ static const struct rockchip_usb2phy_cfg rk3576_phy_cfgs[] = {
>  		.port_cfgs	= {
>  			[USB2PHY_PORT_OTG] = {
>  				.phy_sus	= { 0x0000, 8, 0, 0, 0x1d1 },
> +				.svbus_en	= { 0x0000, 15, 15, 0, 1 },
> +				.svbus_sel	= { 0x0000, 14, 14, 0, 1 },
>  				.bvalid_det_en	= { 0x00c0, 1, 1, 0, 1 },
>  				.bvalid_det_st	= { 0x00c4, 1, 1, 0, 1 },
>  				.bvalid_det_clr = { 0x00c8, 1, 1, 0, 1 },
> @@ -2087,6 +2185,8 @@ static const struct rockchip_usb2phy_cfg rk3576_phy_cfgs[] = {
>  		.port_cfgs	= {
>  			[USB2PHY_PORT_OTG] = {
>  				.phy_sus	= { 0x2000, 8, 0, 0, 0x1d1 },
> +				.svbus_en	= { 0x2000, 15, 15, 0, 1 },
> +				.svbus_sel	= { 0x2000, 14, 14, 0, 1 },
>  				.bvalid_det_en	= { 0x20c0, 1, 1, 0, 1 },
>  				.bvalid_det_st	= { 0x20c4, 1, 1, 0, 1 },
>  				.bvalid_det_clr = { 0x20c8, 1, 1, 0, 1 },
> 
> 

Hi Heiko,

On Thursday, 19 June 2025 21:42:00 Central European Summer Time Heiko Stuebner wrote:
> Hi Nicolas,
> 
> Am Donnerstag, 19. Juni 2025, 20:36:36 Mitteleuropäische Sommerzeit schrieb Nicolas Frattaroli:
> > With USB type C connectors, the vbus detect pin of the OTG controller
> > attached to it is pulled high by a USB Type C controller chip such as
> > the fusb302. This means USB enumeration on Type-C ports never works, as
> > the vbus is always seen as high.
> > 
> > Rockchip added some GRF register flags to deal with this situation. The
> > RK3576 TRM calls these "soft_vbusvalid_bvalid" (con0 bit index 15) and
> > "soft_vbusvalid_bvalid_sel" (con0 bit index 14).
> 
> I would expect a paragraph more about what those bits (and their
> functionality) actually do here :-) 

:( fiiine

Quick non-patch explainer though, in case it helps you spot a problem in
the code: looks like svbus_sel to 1 tells the SoC that the OTG
controller's vbus valid and bvalid signal is controlled by the svbus_en
GRF flag instead of whatever the controller does based on what it sees
on the voltage lines.

> 
> 
> > Downstream introduces a new vendor property which tells the USB 2 PHY
> > that it's connected to a type C port, but we can do better. Since in
> > such an arrangement, we'll have an OF graph connection from the USB
> > controller to the USB connector anyway, we can walk said OF graph and
> > check the connector's compatible to determine this without adding any
> > further vendor properties.
> > 
> > Do keep in mind that the usbdp PHY driver seemingly fiddles with these
> > register fields as well, but what it does doesn't appear to be enough
> > for us to get working USB enumeration, presumably because the whole
> > vbus_attach logic needs to be adjusted as well either way.
> 
> 
> In the rk3588 TRM I find USB2PHY_GRF_CON4
> bit3 - sft_vbus_sel (VBUS software control select)
> bit2 - sft_vbus (When sft_vbus_sel is 1'b1, vbusvalid and bvalid is
>                  controlled by sft_vbus)
> 
> Is that the same stuff as you're adding for rk3576 ?

Yes, these appear to be the same ones. I'd need to check whether Type-C
USB devices enumerate on RK3588 first to see if we have the same problem
there though (it would be weird if it weren't a problem there).

If you pick the DT patch, I can send a new version of the soft vbusvalid
control patch with RK3588 addressed as well, if I manage to confirm it's
the same thing there.

> 
> My last dance with rk3588-type-c is already some months back, but I do
> remember running into "some" issue - but don't remember which one ;-)
> 
> 
> Heiko
> 

Kind regards,
Nicolas Frattaroli

> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >  drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 108 +++++++++++++++++++++++++-
> >  1 file changed, 104 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> > index b0f23690ec3002202c0f33a6988f5509622fa10e..71810c07e4150ea81f65a8a932541b144e95a137 100644
> > --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> > +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> > @@ -17,6 +17,7 @@
> >  #include <linux/module.h>
> >  #include <linux/mutex.h>
> >  #include <linux/of.h>
> > +#include <linux/of_graph.h>
> >  #include <linux/of_irq.h>
> >  #include <linux/phy/phy.h>
> >  #include <linux/platform_device.h>
> > @@ -114,6 +115,8 @@ struct rockchip_chg_det_reg {
> >  /**
> >   * struct rockchip_usb2phy_port_cfg - usb-phy port configuration.
> >   * @phy_sus: phy suspend register.
> > + * @svbus_en: soft vbus bvalid enable register.
> > + * @svbus_sel: soft vbus bvalid selection register.
> >   * @bvalid_det_en: vbus valid rise detection enable register.
> >   * @bvalid_det_st: vbus valid rise detection status register.
> >   * @bvalid_det_clr: vbus valid rise detection clear register.
> > @@ -140,6 +143,8 @@ struct rockchip_chg_det_reg {
> >   */
> >  struct rockchip_usb2phy_port_cfg {
> >  	struct usb2phy_reg	phy_sus;
> > +	struct usb2phy_reg	svbus_en;
> > +	struct usb2phy_reg	svbus_sel;
> >  	struct usb2phy_reg	bvalid_det_en;
> >  	struct usb2phy_reg	bvalid_det_st;
> >  	struct usb2phy_reg	bvalid_det_clr;
> > @@ -203,6 +208,7 @@ struct rockchip_usb2phy_cfg {
> >   * @event_nb: hold event notification callback.
> >   * @state: define OTG enumeration states before device reset.
> >   * @mode: the dr_mode of the controller.
> > + * @typec_vbus_det: whether to apply Type C logic to OTG vbus detection.
> >   */
> >  struct rockchip_usb2phy_port {
> >  	struct phy	*phy;
> > @@ -222,6 +228,7 @@ struct rockchip_usb2phy_port {
> >  	struct notifier_block	event_nb;
> >  	enum usb_otg_state	state;
> >  	enum usb_dr_mode	mode;
> > +	bool typec_vbus_det;
> >  };
> >  
> >  /**
> > @@ -495,6 +502,13 @@ static int rockchip_usb2phy_init(struct phy *phy)
> >  	mutex_lock(&rport->mutex);
> >  
> >  	if (rport->port_id == USB2PHY_PORT_OTG) {
> > +		if (rport->typec_vbus_det) {
> > +			if (rport->port_cfg->svbus_en.enable &&
> > +					rport->port_cfg->svbus_sel.enable) {
> > +				property_enable(rphy->grf, &rport->port_cfg->svbus_en, true);
> > +				property_enable(rphy->grf, &rport->port_cfg->svbus_sel, true);
> > +			}
> > +		}
> >  		if (rport->mode != USB_DR_MODE_HOST &&
> >  		    rport->mode != USB_DR_MODE_UNKNOWN) {
> >  			/* clear bvalid status and enable bvalid detect irq */
> > @@ -535,8 +549,7 @@ static int rockchip_usb2phy_init(struct phy *phy)
> >  			if (ret)
> >  				goto out;
> >  
> > -			schedule_delayed_work(&rport->otg_sm_work,
> > -					      OTG_SCHEDULE_DELAY * 3);
> > +			schedule_delayed_work(&rport->otg_sm_work, 0);
> >  		} else {
> >  			/* If OTG works in host only mode, do nothing. */
> >  			dev_dbg(&rport->phy->dev, "mode %d\n", rport->mode);
> > @@ -666,8 +679,12 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
> >  	unsigned long delay;
> >  	bool vbus_attach, sch_work, notify_charger;
> >  
> > -	vbus_attach = property_enabled(rphy->grf,
> > -				       &rport->port_cfg->utmi_bvalid);
> > +	if (rport->port_cfg->svbus_en.enable && rport->typec_vbus_det) {
> > +		vbus_attach = true;
> > +	} else {
> > +		vbus_attach = property_enabled(rphy->grf,
> > +					       &rport->port_cfg->utmi_bvalid);
> > +	}
> >  
> >  	sch_work = false;
> >  	notify_charger = false;
> > @@ -1276,6 +1293,83 @@ static int rockchip_otg_event(struct notifier_block *nb,
> >  	return NOTIFY_DONE;
> >  }
> >  
> > +static const char *const rockchip_usb2phy_typec_cons[] = {
> > +	"usb-c-connector",
> > +	NULL,
> > +};
> > +
> > +static struct device_node *rockchip_usb2phy_to_controller(struct rockchip_usb2phy *rphy)
> > +{
> > +	struct device_node *np;
> > +	struct device_node *parent;
> > +
> > +	for_each_node_with_property(np, "phys") {
> > +		struct of_phandle_iterator it;
> > +		int ret;
> > +
> > +		of_for_each_phandle(&it, ret, np, "phys", NULL, 0) {
> > +			parent = of_get_parent(it.node);
> > +			if (it.node != rphy->dev->of_node && rphy->dev->of_node != parent) {
> > +				if (parent)
> > +					of_node_put(parent);
> > +				continue;
> > +			}
> > +
> > +			/*
> > +			 * Either the PHY phandle we're iterating or its parent
> > +			 * matched, we don't care about which out of the two in
> > +			 * particular as we just need to know it's the right
> > +			 * USB controller for this PHY.
> > +			 */
> > +			of_node_put(it.node);
> > +			of_node_put(parent);
> > +			return np;
> > +		}
> > +	}
> > +
> > +	return NULL;
> > +}
> > +
> > +static bool rockchip_usb2phy_otg_is_type_c(struct rockchip_usb2phy *rphy)
> > +{
> > +	struct device_node *controller = rockchip_usb2phy_to_controller(rphy);
> > +	struct device_node *ports;
> > +	struct device_node *ep = NULL;
> > +	struct device_node *parent;
> > +
> > +	if (!controller)
> > +		return false;
> > +
> > +	ports = of_get_child_by_name(controller, "ports");
> > +	if (ports) {
> > +		of_node_put(controller);
> > +		controller = ports;
> > +	}
> > +
> > +	for_each_of_graph_port(controller, port) {
> > +		ep = of_get_child_by_name(port, "endpoint");
> > +		if (!ep)
> > +			continue;
> > +
> > +		parent = of_graph_get_remote_port_parent(ep);
> > +		of_node_put(ep);
> > +		if (!parent)
> > +			continue;
> > +
> > +		if (of_device_compatible_match(parent, rockchip_usb2phy_typec_cons)) {
> > +			of_node_put(parent);
> > +			of_node_put(controller);
> > +			return true;
> > +		}
> > +
> > +		of_node_put(parent);
> > +	}
> > +
> > +	of_node_put(controller);
> > +
> > +	return false;
> > +}
> > +
> >  static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
> >  					  struct rockchip_usb2phy_port *rport,
> >  					  struct device_node *child_np)
> > @@ -1297,6 +1391,8 @@ static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
> >  
> >  	mutex_init(&rport->mutex);
> >  
> > +	rport->typec_vbus_det = rockchip_usb2phy_otg_is_type_c(rphy);
> > +
> >  	rport->mode = of_usb_get_dr_mode_by_phy(child_np, -1);
> >  	if (rport->mode == USB_DR_MODE_HOST ||
> >  	    rport->mode == USB_DR_MODE_UNKNOWN) {
> > @@ -2050,6 +2146,8 @@ static const struct rockchip_usb2phy_cfg rk3576_phy_cfgs[] = {
> >  		.port_cfgs	= {
> >  			[USB2PHY_PORT_OTG] = {
> >  				.phy_sus	= { 0x0000, 8, 0, 0, 0x1d1 },
> > +				.svbus_en	= { 0x0000, 15, 15, 0, 1 },
> > +				.svbus_sel	= { 0x0000, 14, 14, 0, 1 },
> >  				.bvalid_det_en	= { 0x00c0, 1, 1, 0, 1 },
> >  				.bvalid_det_st	= { 0x00c4, 1, 1, 0, 1 },
> >  				.bvalid_det_clr = { 0x00c8, 1, 1, 0, 1 },
> > @@ -2087,6 +2185,8 @@ static const struct rockchip_usb2phy_cfg rk3576_phy_cfgs[] = {
> >  		.port_cfgs	= {
> >  			[USB2PHY_PORT_OTG] = {
> >  				.phy_sus	= { 0x2000, 8, 0, 0, 0x1d1 },
> > +				.svbus_en	= { 0x2000, 15, 15, 0, 1 },
> > +				.svbus_sel	= { 0x2000, 14, 14, 0, 1 },
> >  				.bvalid_det_en	= { 0x20c0, 1, 1, 0, 1 },
> >  				.bvalid_det_st	= { 0x20c4, 1, 1, 0, 1 },
> >  				.bvalid_det_clr = { 0x20c8, 1, 1, 0, 1 },
> > 
> > 
> 
> 
> 
> 
> 

Hi Heiko, a quick follow-up,

On Thursday, 19 June 2025 22:23:35 Central European Summer Time Nicolas Frattaroli wrote:
> Hi Heiko,
> 
> On Thursday, 19 June 2025 21:42:00 Central European Summer Time Heiko Stuebner wrote:
> > Hi Nicolas,
> > 
> > Am Donnerstag, 19. Juni 2025, 20:36:36 Mitteleuropäische Sommerzeit schrieb Nicolas Frattaroli:
> > > With USB type C connectors, the vbus detect pin of the OTG controller
> > > attached to it is pulled high by a USB Type C controller chip such as
> > > the fusb302. This means USB enumeration on Type-C ports never works, as
> > > the vbus is always seen as high.
> > > 
> > > Rockchip added some GRF register flags to deal with this situation. The
> > > RK3576 TRM calls these "soft_vbusvalid_bvalid" (con0 bit index 15) and
> > > "soft_vbusvalid_bvalid_sel" (con0 bit index 14).
> > 
> > I would expect a paragraph more about what those bits (and their
> > functionality) actually do here :-) 
> 
> :( fiiine
> 
> Quick non-patch explainer though, in case it helps you spot a problem in
> the code: looks like svbus_sel to 1 tells the SoC that the OTG
> controller's vbus valid and bvalid signal is controlled by the svbus_en
> GRF flag instead of whatever the controller does based on what it sees
> on the voltage lines.
> 
> > 
> > 
> > > Downstream introduces a new vendor property which tells the USB 2 PHY
> > > that it's connected to a type C port, but we can do better. Since in
> > > such an arrangement, we'll have an OF graph connection from the USB
> > > controller to the USB connector anyway, we can walk said OF graph and
> > > check the connector's compatible to determine this without adding any
> > > further vendor properties.
> > > 
> > > Do keep in mind that the usbdp PHY driver seemingly fiddles with these
> > > register fields as well, but what it does doesn't appear to be enough
> > > for us to get working USB enumeration, presumably because the whole
> > > vbus_attach logic needs to be adjusted as well either way.
> > 
> > 
> > In the rk3588 TRM I find USB2PHY_GRF_CON4
> > bit3 - sft_vbus_sel (VBUS software control select)
> > bit2 - sft_vbus (When sft_vbus_sel is 1'b1, vbusvalid and bvalid is
> >                  controlled by sft_vbus)
> > 
> > Is that the same stuff as you're adding for rk3576 ?
> 
> Yes, these appear to be the same ones. I'd need to check whether Type-C
> USB devices enumerate on RK3588 first to see if we have the same problem
> there though (it would be weird if it weren't a problem there).
> 
> If you pick the DT patch, I can send a new version of the soft vbusvalid
> control patch with RK3588 addressed as well, if I manage to confirm it's
> the same thing there.

So I tested this on RK3588, and could not reproduce the issue. Then, out of
curiosity, I reverted the patch and tested on the Sige5 again. I could also
not reproduce the issue anymore (?!). Even with the udphy line commented out
that sets those GRF regs as well, I can't get it to have issues enumerating
things on Type-C anymore.

The downstream commit this was based on is here:

https://github.com/rockchip-linux/kernel/commit/7d2237b0adc2e0a0105d63b645528993b44c8c36

So for now, this patch can be considered "abandoned, maybe unnecessary"
until the problem rears its head again for someone. I really don't get
why it works now :(

> 
> > 
> > My last dance with rk3588-type-c is already some months back, but I do
> > remember running into "some" issue - but don't remember which one ;-)
> > 
> > 
> > Heiko
> > 
> 
> Kind regards,
> Nicolas Frattaroli
> 
> > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > > ---
> > >  drivers/phy/rockchip/phy-rockchip-inno-usb2.c | 108 +++++++++++++++++++++++++-
> > >  1 file changed, 104 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> > > index b0f23690ec3002202c0f33a6988f5509622fa10e..71810c07e4150ea81f65a8a932541b144e95a137 100644
> > > --- a/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> > > +++ b/drivers/phy/rockchip/phy-rockchip-inno-usb2.c
> > > @@ -17,6 +17,7 @@
> > >  #include <linux/module.h>
> > >  #include <linux/mutex.h>
> > >  #include <linux/of.h>
> > > +#include <linux/of_graph.h>
> > >  #include <linux/of_irq.h>
> > >  #include <linux/phy/phy.h>
> > >  #include <linux/platform_device.h>
> > > @@ -114,6 +115,8 @@ struct rockchip_chg_det_reg {
> > >  /**
> > >   * struct rockchip_usb2phy_port_cfg - usb-phy port configuration.
> > >   * @phy_sus: phy suspend register.
> > > + * @svbus_en: soft vbus bvalid enable register.
> > > + * @svbus_sel: soft vbus bvalid selection register.
> > >   * @bvalid_det_en: vbus valid rise detection enable register.
> > >   * @bvalid_det_st: vbus valid rise detection status register.
> > >   * @bvalid_det_clr: vbus valid rise detection clear register.
> > > @@ -140,6 +143,8 @@ struct rockchip_chg_det_reg {
> > >   */
> > >  struct rockchip_usb2phy_port_cfg {
> > >  	struct usb2phy_reg	phy_sus;
> > > +	struct usb2phy_reg	svbus_en;
> > > +	struct usb2phy_reg	svbus_sel;
> > >  	struct usb2phy_reg	bvalid_det_en;
> > >  	struct usb2phy_reg	bvalid_det_st;
> > >  	struct usb2phy_reg	bvalid_det_clr;
> > > @@ -203,6 +208,7 @@ struct rockchip_usb2phy_cfg {
> > >   * @event_nb: hold event notification callback.
> > >   * @state: define OTG enumeration states before device reset.
> > >   * @mode: the dr_mode of the controller.
> > > + * @typec_vbus_det: whether to apply Type C logic to OTG vbus detection.
> > >   */
> > >  struct rockchip_usb2phy_port {
> > >  	struct phy	*phy;
> > > @@ -222,6 +228,7 @@ struct rockchip_usb2phy_port {
> > >  	struct notifier_block	event_nb;
> > >  	enum usb_otg_state	state;
> > >  	enum usb_dr_mode	mode;
> > > +	bool typec_vbus_det;
> > >  };
> > >  
> > >  /**
> > > @@ -495,6 +502,13 @@ static int rockchip_usb2phy_init(struct phy *phy)
> > >  	mutex_lock(&rport->mutex);
> > >  
> > >  	if (rport->port_id == USB2PHY_PORT_OTG) {
> > > +		if (rport->typec_vbus_det) {
> > > +			if (rport->port_cfg->svbus_en.enable &&
> > > +					rport->port_cfg->svbus_sel.enable) {
> > > +				property_enable(rphy->grf, &rport->port_cfg->svbus_en, true);
> > > +				property_enable(rphy->grf, &rport->port_cfg->svbus_sel, true);
> > > +			}
> > > +		}
> > >  		if (rport->mode != USB_DR_MODE_HOST &&
> > >  		    rport->mode != USB_DR_MODE_UNKNOWN) {
> > >  			/* clear bvalid status and enable bvalid detect irq */
> > > @@ -535,8 +549,7 @@ static int rockchip_usb2phy_init(struct phy *phy)
> > >  			if (ret)
> > >  				goto out;
> > >  
> > > -			schedule_delayed_work(&rport->otg_sm_work,
> > > -					      OTG_SCHEDULE_DELAY * 3);
> > > +			schedule_delayed_work(&rport->otg_sm_work, 0);
> > >  		} else {
> > >  			/* If OTG works in host only mode, do nothing. */
> > >  			dev_dbg(&rport->phy->dev, "mode %d\n", rport->mode);
> > > @@ -666,8 +679,12 @@ static void rockchip_usb2phy_otg_sm_work(struct work_struct *work)
> > >  	unsigned long delay;
> > >  	bool vbus_attach, sch_work, notify_charger;
> > >  
> > > -	vbus_attach = property_enabled(rphy->grf,
> > > -				       &rport->port_cfg->utmi_bvalid);
> > > +	if (rport->port_cfg->svbus_en.enable && rport->typec_vbus_det) {
> > > +		vbus_attach = true;
> > > +	} else {
> > > +		vbus_attach = property_enabled(rphy->grf,
> > > +					       &rport->port_cfg->utmi_bvalid);
> > > +	}
> > >  
> > >  	sch_work = false;
> > >  	notify_charger = false;
> > > @@ -1276,6 +1293,83 @@ static int rockchip_otg_event(struct notifier_block *nb,
> > >  	return NOTIFY_DONE;
> > >  }
> > >  
> > > +static const char *const rockchip_usb2phy_typec_cons[] = {
> > > +	"usb-c-connector",
> > > +	NULL,
> > > +};
> > > +
> > > +static struct device_node *rockchip_usb2phy_to_controller(struct rockchip_usb2phy *rphy)
> > > +{
> > > +	struct device_node *np;
> > > +	struct device_node *parent;
> > > +
> > > +	for_each_node_with_property(np, "phys") {
> > > +		struct of_phandle_iterator it;
> > > +		int ret;
> > > +
> > > +		of_for_each_phandle(&it, ret, np, "phys", NULL, 0) {
> > > +			parent = of_get_parent(it.node);
> > > +			if (it.node != rphy->dev->of_node && rphy->dev->of_node != parent) {
> > > +				if (parent)
> > > +					of_node_put(parent);
> > > +				continue;
> > > +			}
> > > +
> > > +			/*
> > > +			 * Either the PHY phandle we're iterating or its parent
> > > +			 * matched, we don't care about which out of the two in
> > > +			 * particular as we just need to know it's the right
> > > +			 * USB controller for this PHY.
> > > +			 */
> > > +			of_node_put(it.node);
> > > +			of_node_put(parent);
> > > +			return np;
> > > +		}
> > > +	}
> > > +
> > > +	return NULL;
> > > +}
> > > +
> > > +static bool rockchip_usb2phy_otg_is_type_c(struct rockchip_usb2phy *rphy)
> > > +{
> > > +	struct device_node *controller = rockchip_usb2phy_to_controller(rphy);
> > > +	struct device_node *ports;
> > > +	struct device_node *ep = NULL;
> > > +	struct device_node *parent;
> > > +
> > > +	if (!controller)
> > > +		return false;
> > > +
> > > +	ports = of_get_child_by_name(controller, "ports");
> > > +	if (ports) {
> > > +		of_node_put(controller);
> > > +		controller = ports;
> > > +	}
> > > +
> > > +	for_each_of_graph_port(controller, port) {
> > > +		ep = of_get_child_by_name(port, "endpoint");
> > > +		if (!ep)
> > > +			continue;
> > > +
> > > +		parent = of_graph_get_remote_port_parent(ep);
> > > +		of_node_put(ep);
> > > +		if (!parent)
> > > +			continue;
> > > +
> > > +		if (of_device_compatible_match(parent, rockchip_usb2phy_typec_cons)) {
> > > +			of_node_put(parent);
> > > +			of_node_put(controller);
> > > +			return true;
> > > +		}
> > > +
> > > +		of_node_put(parent);
> > > +	}
> > > +
> > > +	of_node_put(controller);
> > > +
> > > +	return false;
> > > +}
> > > +
> > >  static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
> > >  					  struct rockchip_usb2phy_port *rport,
> > >  					  struct device_node *child_np)
> > > @@ -1297,6 +1391,8 @@ static int rockchip_usb2phy_otg_port_init(struct rockchip_usb2phy *rphy,
> > >  
> > >  	mutex_init(&rport->mutex);
> > >  
> > > +	rport->typec_vbus_det = rockchip_usb2phy_otg_is_type_c(rphy);
> > > +
> > >  	rport->mode = of_usb_get_dr_mode_by_phy(child_np, -1);
> > >  	if (rport->mode == USB_DR_MODE_HOST ||
> > >  	    rport->mode == USB_DR_MODE_UNKNOWN) {
> > > @@ -2050,6 +2146,8 @@ static const struct rockchip_usb2phy_cfg rk3576_phy_cfgs[] = {
> > >  		.port_cfgs	= {
> > >  			[USB2PHY_PORT_OTG] = {
> > >  				.phy_sus	= { 0x0000, 8, 0, 0, 0x1d1 },
> > > +				.svbus_en	= { 0x0000, 15, 15, 0, 1 },
> > > +				.svbus_sel	= { 0x0000, 14, 14, 0, 1 },
> > >  				.bvalid_det_en	= { 0x00c0, 1, 1, 0, 1 },
> > >  				.bvalid_det_st	= { 0x00c4, 1, 1, 0, 1 },
> > >  				.bvalid_det_clr = { 0x00c8, 1, 1, 0, 1 },
> > > @@ -2087,6 +2185,8 @@ static const struct rockchip_usb2phy_cfg rk3576_phy_cfgs[] = {
> > >  		.port_cfgs	= {
> > >  			[USB2PHY_PORT_OTG] = {
> > >  				.phy_sus	= { 0x2000, 8, 0, 0, 0x1d1 },
> > > +				.svbus_en	= { 0x2000, 15, 15, 0, 1 },
> > > +				.svbus_sel	= { 0x2000, 14, 14, 0, 1 },
> > >  				.bvalid_det_en	= { 0x20c0, 1, 1, 0, 1 },
> > >  				.bvalid_det_st	= { 0x20c4, 1, 1, 0, 1 },
> > >  				.bvalid_det_clr = { 0x20c8, 1, 1, 0, 1 },
> > > 
> > > 
> > 
> > 
> > 
> > 
> > 
> 
>