Message-ID: <7131451.G0QQBjFxQf@phil>
Date: Thu, 19 Jun 2025 21:42:00 +0200
From: Heiko Stuebner <heiko@...ech.de>
To: Vinod Koul <vkoul@...nel.org>, Kishon Vijay Abraham I <kishon@...nel.org>,
 Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
 Conor Dooley <conor+dt@...nel.org>, Kever Yang <kever.yang@...k-chips.com>,
 Frank Wang <frank.wang@...k-chips.com>,
 Neil Armstrong <neil.armstrong@...aro.org>,
 Nicolas Frattaroli <nicolas.frattaroli@...labora.com>,
 Sebastian Reichel <sebastian.reichel@...labora.com>
Cc: Alexey Charkov <alchark@...il.com>, kernel@...labora.com,
 linux-phy@...ts.infradead.org, devicetree@...r.kernel.org,
 linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
 linux-kernel@...r.kernel.org,
 Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
Subject:
 Re: [PATCH v5 1/2] phy: rockchip: inno-usb2: add soft vbusvalid control

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@...labora.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 },
> 
> 

Powered by blists - more mailing lists