Re: [PATCH 2/4] usb: typec: wire USB role changes to OTG device

[Ahmad Fatoum <a.fatoum@pengutronix.de>]

Hi,

On 4/20/26 11:02 AM, Sascha Hauer wrote:
> When a Type-C controller reports a USB role change via typec_set_role(),
> find the associated OTG device via the OF graph and set its dr_mode
> accordingly.
> 
> Since the Type-C controller may probe before the OTG device registers,
> a late_initcall sweeps all known ports and applies any role that was not
> yet forwarded.
> 
> Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
> Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
> ---
>  drivers/usb/typec/class.c | 60 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 60 insertions(+)
> 
> diff --git a/drivers/usb/typec/class.c b/drivers/usb/typec/class.c
> index e03eaccbb9..b88e1c1e46 100644
> --- a/drivers/usb/typec/class.c
> +++ b/drivers/usb/typec/class.c
> @@ -8,10 +8,15 @@
>  
>  #include <module.h>
>  #include <driver.h>
> +#include <init.h>
>  #include <linux/kernel.h>
> +#include <linux/list.h>
>  #include <linux/usb/role.h>
>  #include <linux/usb/typec.h>
>  #include <linux/usb/typec_altmode.h>
> +#include <linux/usb/usb.h>
> +#include <linux/device/bus.h>
> +#include <of_graph.h>
>  #include <param.h>
>  
>  enum typec_param_accessory {
> @@ -22,12 +27,16 @@ enum typec_param_accessory {
>  
>  struct typec_port {
>  	struct device dev;
> +	struct list_head list;
>  	const struct typec_operations *ops;
>  	int pwr_role;		/* enum typec_role */
>  	int usb_role;		/* enum usb_role role */
>  	int accessory;		/* enum typec_param_accessory */
> +	bool otg_role_applied;
>  };
>  
> +static LIST_HEAD(typec_port_list);
> +
>  /**
>   * typec_set_pwr_role - Report power role change
>   * @port: The USB Type-C Port where the role was changed
> @@ -68,6 +77,41 @@ int typec_set_mode(struct typec_port *port, int mode)
>  }
>  EXPORT_SYMBOL_GPL(typec_set_mode);
>  
> +static bool typec_apply_otg_role(struct typec_port *port)
> +{
> +	struct device_node *connector = port->dev.of_node;
> +	struct device_node *ep, *remote;
> +	struct device *dev;
> +	enum usb_dr_mode dr_mode;
> +	bool applied = false;
> +
> +	if (!IS_ENABLED(CONFIG_USB_OTGDEV) || !connector)
> +		return false;
> +
> +	if (port->usb_role == USB_ROLE_HOST)
> +		dr_mode = USB_DR_MODE_HOST;
> +	else if (port->usb_role == USB_ROLE_DEVICE)
> +		dr_mode = USB_DR_MODE_PERIPHERAL;
> +	else
> +		return false;
> +
> +	for_each_endpoint_of_node(connector, ep) {
> +		remote = of_graph_get_remote_port_parent(ep);
> +		if (!remote)
> +			continue;
> +
> +		bus_for_each_device(&otg_bus_type, dev) {
> +			if (dev->parent && dev->parent->of_node == remote) {

I believe board code could've set the mode before we come here.

Also, I think there must be a way to opt out of this.

Let's assume you load barebox via USB on a Type C port. You may not be
able to yank the cable away fast enough before barebox does the one-way
otg -> peripheral transition.

> +				otg_device_set_dr_mode(dev, dr_mode);
> +				applied = true;
> +				break;
> +			}
> +		}
> +	}
> +
> +	return applied;
> +}
> +
>  /**
>   * typec_set_role - Set USB role for a Type-C connector
>   * @port: USB Type-C connector
> @@ -79,10 +123,24 @@ EXPORT_SYMBOL_GPL(typec_set_mode);
>  int typec_set_role(struct typec_port *port, enum usb_role role)
>  {
>  	port->usb_role = role;
> +	port->otg_role_applied = typec_apply_otg_role(port);
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(typec_set_role);
>  
> +static int typec_apply_otg_roles(void)
> +{
> +	struct typec_port *port;
> +
> +	list_for_each_entry(port, &typec_port_list, list) {
> +		if (!port->otg_role_applied)
> +			port->otg_role_applied = typec_apply_otg_role(port);
> +	}
> +
> +	return 0;
> +}
> +late_initcall(typec_apply_otg_roles);

Does postenvironment not make more sense, so the environment can
override this?

> +
>  /**
>   * typec_get_drvdata - Return private driver data pointer
>   * @port: USB Type-C port
> @@ -165,6 +223,8 @@ struct typec_port *typec_register_port(struct device *parent,
>  	if (dev->of_node)
>  		dev->of_node->dev = dev;
>  
> +	list_add_tail(&port->list, &typec_port_list);
> +
>  	dev_add_param_enum(dev, "usb_role", param_set_readonly, typec_param_update,
>  			   &port->usb_role, usb_role_names,
>  			   ARRAY_SIZE(usb_role_names), port);
> 

-- 
Pengutronix e.K.                  |                             |
Steuerwalder Str. 21              | http://www.pengutronix.de/  |
31137 Hildesheim, Germany         | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686  | Fax:   +49-5121-206917-5555 |