Re: [PATCH] Revert "tcpm: allow looking for role_sw device in the main node"

[Arnaud Ferraris <arnaud.ferraris@collabora.com>]

Hi,

Le 24/02/2026 à 12:01, Xu Yang a écrit :
> This reverts commit 1366cd228b0c67b60a2c0c26ef37fe9f7cfedb7f.

I believe a plain revert isn't the right solution here, as we'll get to 
the same point as we were before 1366cd228b0c, where some devices 
stopped working properly with newer kernels.

> 
> The fwnode_usb_role_switch_get() returns NULL only if no connection is
> found, returns ERR_PTR(-EPROBE_DEFER) if connection is found but deferred
> probe is needed, or a valid pointer of usb_role_switch.
> 
> When switching from NULL check to IS_ERR_OR_NULL(), usb_role_switch_get()
> will return NULL pointer which will override ERR_PTR(-EPROBE_DEFER) which
> is returned by fwnode_usb_role_switch_get(). Then usb role switch can't be
> obtained because the defer probe info is lost. So the unique error should
> not be regarded the same as NULL.
> 
> Fixes: 1366cd228b0c ("tcpm: allow looking for role_sw device in the main node")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Also correct a description in 1366cd228b0c ("tcpm: allow looking for
> role_sw device in the main node"), if the ports are defined in the tcpc
> main node, NULL pointer is returned by fwnode_usb_role_switch_get()
> instead of an error.
> ---
>   drivers/usb/typec/tcpm/tcpm.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
> index 1d2f3af034c5..8e0e14a2704e 100644
> --- a/drivers/usb/typec/tcpm/tcpm.c
> +++ b/drivers/usb/typec/tcpm/tcpm.c
> @@ -7890,7 +7890,7 @@ struct tcpm_port *tcpm_register_port(struct device *dev, struct tcpc_dev *tcpc)
>   	port->partner_desc.identity = &port->partner_ident;
>   
>   	port->role_sw = fwnode_usb_role_switch_get(tcpc->fwnode);
> -	if (IS_ERR_OR_NULL(port->role_sw))
> +	if (!port->role_sw)

It might be worth saving the error and restoring it after the call to 
usb_role_switch_get() instead, something like:

	if (IS_ERR_OR_NULL(port->role_sw)) {
		err = PTR_ERR(port->role_sw);
		port->role_sw = usb_role_switch_get(port->dev);
		if (!port->role_sw)
			port->role_sw = ERR_PTR(err);
	}


>   		port->role_sw = usb_role_switch_get(port->dev);
>   	if (IS_ERR(port->role_sw)) {
>   		err = PTR_ERR(port->role_sw);

Best regards,
Arnaud