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

[Xu Yang]

Hi Arnaud,

On Tue, Feb 24, 2026 at 12:33:33PM +0100, Arnaud Ferraris wrote:
> 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.

I don't think 1366cd228b0c fix the real root problem because the description
should be wrong in the commit message. If -EPROBE_DEFER is returned by
fwnode_usb_role_switch_get(), the ports node should be in connector node
instead of tcpc node. However, you get the error when ports in tcpc node.

Could you double check the issue, so we can find a proper solution and avoid
the further regression?

> 
> > 
> > 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@xxxxxxxxxxxxxxx
> > Signed-off-by: Xu Yang <xu.yang_2@xxxxxxx>
> > 
> > ---
> > 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);
> 	}

It works but we'd better to get the thing clear firstly.

Thanks,
Xu Yang