public inbox for linux-usb@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Revert "tcpm: allow looking for role_sw device in the main node"
@ 2026-02-24 11:01 Xu Yang
  2026-02-24 11:33 ` Arnaud Ferraris
  0 siblings, 1 reply; 9+ messages in thread
From: Xu Yang @ 2026-02-24 11:01 UTC (permalink / raw)
  To: badhri, heikki.krogerus, gregkh, arnaud.ferraris, dsimic
  Cc: linux-usb, linux-kernel, imx, jun.li

This reverts commit 1366cd228b0c67b60a2c0c26ef37fe9f7cfedb7f.

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)
 		port->role_sw = usb_role_switch_get(port->dev);
 	if (IS_ERR(port->role_sw)) {
 		err = PTR_ERR(port->role_sw);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "tcpm: allow looking for role_sw device in the main node"
  2026-02-24 11:01 [PATCH] Revert "tcpm: allow looking for role_sw device in the main node" Xu Yang
@ 2026-02-24 11:33 ` Arnaud Ferraris
  2026-02-25  2:57   ` Xu Yang
  0 siblings, 1 reply; 9+ messages in thread
From: Arnaud Ferraris @ 2026-02-24 11:33 UTC (permalink / raw)
  To: Xu Yang, badhri, heikki.krogerus, gregkh, dsimic
  Cc: linux-usb, linux-kernel, imx, jun.li

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "tcpm: allow looking for role_sw device in the main node"
  2026-02-24 11:33 ` Arnaud Ferraris
@ 2026-02-25  2:57   ` Xu Yang
  2026-02-27 15:45     ` Arnaud Ferraris
  0 siblings, 1 reply; 9+ messages in thread
From: Xu Yang @ 2026-02-25  2:57 UTC (permalink / raw)
  To: Arnaud Ferraris
  Cc: badhri, heikki.krogerus, gregkh, dsimic, linux-usb, linux-kernel,
	imx, jun.li

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@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);
> 	}

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

Thanks,
Xu Yang

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "tcpm: allow looking for role_sw device in the main node"
  2026-02-25  2:57   ` Xu Yang
@ 2026-02-27 15:45     ` Arnaud Ferraris
  2026-03-05  9:40       ` Xu Yang
  0 siblings, 1 reply; 9+ messages in thread
From: Arnaud Ferraris @ 2026-02-27 15:45 UTC (permalink / raw)
  To: Xu Yang
  Cc: badhri, heikki.krogerus, gregkh, dsimic, linux-usb, linux-kernel,
	imx, jun.li

Hi Xu,

Le 25/02/2026 à 03:57, Xu Yang a écrit :
> 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?

Sure, I'll come up with more details asap, either tomorrow or early next 
week.

Best regards,
Arnaud

> 
>>
>>>
>>> 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);
>> 	}
> 
> It works but we'd better to get the thing clear firstly.
> 
> Thanks,
> Xu Yang


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "tcpm: allow looking for role_sw device in the main node"
  2026-02-27 15:45     ` Arnaud Ferraris
@ 2026-03-05  9:40       ` Xu Yang
  2026-03-05 16:36         ` Arnaud Ferraris
  0 siblings, 1 reply; 9+ messages in thread
From: Xu Yang @ 2026-03-05  9:40 UTC (permalink / raw)
  To: Arnaud Ferraris
  Cc: badhri, heikki.krogerus, gregkh, dsimic, linux-usb, linux-kernel,
	imx, jun.li

On Fri, Feb 27, 2026 at 04:45:30PM +0100, Arnaud Ferraris wrote:
> Hi Xu,
> 
> Le 25/02/2026 à 03:57, Xu Yang a écrit :
> > 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?
> 
> Sure, I'll come up with more details asap, either tomorrow or early next
> week.

Do you have any updates about this?

Thanks,
Xu Yang

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "tcpm: allow looking for role_sw device in the main node"
  2026-03-05  9:40       ` Xu Yang
@ 2026-03-05 16:36         ` Arnaud Ferraris
  2026-03-06  9:52           ` Xu Yang
  0 siblings, 1 reply; 9+ messages in thread
From: Arnaud Ferraris @ 2026-03-05 16:36 UTC (permalink / raw)
  To: Xu Yang
  Cc: badhri, heikki.krogerus, gregkh, dsimic, linux-usb, linux-kernel,
	imx, jun.li

Hi Xu,

Le 05/03/2026 à 10:40, Xu Yang a écrit :
> On Fri, Feb 27, 2026 at 04:45:30PM +0100, Arnaud Ferraris wrote:
>> Hi Xu,
>>
>> Le 25/02/2026 à 03:57, Xu Yang a écrit :
>>> 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?
>>
>> Sure, I'll come up with more details asap, either tomorrow or early next
>> week.
> 
> Do you have any updates about this?

I do, sorry it took so long...

So fwnode_usb_role_switch_get() does indeed return -EPROBE_DEFER, then
keeps doing so on later attempts if I revert my patch. However,
usb_role_switch_get() succeeds on first try.

Please note that:
1. I don't understand much (if any) of the Linux typec stack, and only
    noticed 2d8713f807 broke my device, hence my attempted fix
2. said device is the PinePhone Pro, using an out-of-tree dts (and many
    drivers) from https://codeberg.org/megi/linux

The proper solution likely lies somewhere in the "get proper drivers
and upstream dts for this device" land, although I definitely can't
commit to this.

I think saving the fwnode_usb_role_switch_get() return value and
restoring it if usb_role_switch_get() fails would be a decent
workaround, although I'm definitely open to suggestions.

Feel free to let me know if there's any other test I could run, I'll do
my best at replying promptly.

Best regards,
Arnaud

> 
> Thanks,
> Xu Yang


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "tcpm: allow looking for role_sw device in the main node"
  2026-03-05 16:36         ` Arnaud Ferraris
@ 2026-03-06  9:52           ` Xu Yang
  2026-03-06 16:24             ` Arnaud Ferraris
  0 siblings, 1 reply; 9+ messages in thread
From: Xu Yang @ 2026-03-06  9:52 UTC (permalink / raw)
  To: Arnaud Ferraris
  Cc: badhri, heikki.krogerus, gregkh, dsimic, linux-usb, linux-kernel,
	imx, jun.li

Hi Arnaud,

On Thu, Mar 05, 2026 at 05:36:08PM +0100, Arnaud Ferraris wrote:
> Hi Xu,
> 
> Le 05/03/2026 à 10:40, Xu Yang a écrit :
> > On Fri, Feb 27, 2026 at 04:45:30PM +0100, Arnaud Ferraris wrote:
> > > Hi Xu,
> > > 
> > > Le 25/02/2026 à 03:57, Xu Yang a écrit :
> > > > 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?
> > > 
> > > Sure, I'll come up with more details asap, either tomorrow or early next
> > > week.
> > 
> > Do you have any updates about this?
> 
> I do, sorry it took so long...
> 
> So fwnode_usb_role_switch_get() does indeed return -EPROBE_DEFER, then
> keeps doing so on later attempts if I revert my patch. However,
> usb_role_switch_get() succeeds on first try.
> 
> Please note that:
> 1. I don't understand much (if any) of the Linux typec stack, and only
>    noticed 2d8713f807 broke my device, hence my attempted fix
> 2. said device is the PinePhone Pro, using an out-of-tree dts (and many
>    drivers) from https://codeberg.org/megi/linux
> 
> The proper solution likely lies somewhere in the "get proper drivers
> and upstream dts for this device" land, although I definitely can't
> commit to this.
> 
> I think saving the fwnode_usb_role_switch_get() return value and
> restoring it if usb_role_switch_get() fails would be a decent
> workaround, although I'm definitely open to suggestions.
> 
> Feel free to let me know if there's any other test I could run, I'll do
> my best at replying promptly.

Thanks for your update!

After review the dts file, I finally find you meet the issue with:

	usb-role-switch = <&typec_extcon_bridge>;

instead of using below device graph port node.

	fusb0: typec-portc@22 {
		compatible = "fcs,fusb302";
		...

		port {
			remote-endpoint = <&typec_extcon_bridge>;
		};

		connector {
			...
		};
	};

The commit 1366cd228b0 message "If ports are defined in the tcpc main node"
has confused me at the beginning.

Anyway, it should be another potential issue. Can you test whether below
patch fix your issue?

diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
index b8e28ceca51e..edec139b68b5 100644
--- a/drivers/usb/roles/class.c
+++ b/drivers/usb/roles/class.c
@@ -139,9 +139,14 @@ static void *usb_role_switch_match(const struct fwnode_handle *fwnode, const cha
 static struct usb_role_switch *
 usb_role_switch_is_parent(struct fwnode_handle *fwnode)
 {
-       struct fwnode_handle *parent = fwnode_get_parent(fwnode);
+       struct fwnode_handle *parent;
        struct device *dev;

+       if (!fwnode_device_is_compatible(fwnode, "usb-b-connector"))
+               return NULL;
+
+       parent = fwnode_get_parent(fwnode);
+
        if (!fwnode_property_present(parent, "usb-role-switch")) {
                fwnode_handle_put(parent);
                return NULL;

Thanks,
Xu Yang

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "tcpm: allow looking for role_sw device in the main node"
  2026-03-06  9:52           ` Xu Yang
@ 2026-03-06 16:24             ` Arnaud Ferraris
  2026-03-09  7:36               ` Xu Yang
  0 siblings, 1 reply; 9+ messages in thread
From: Arnaud Ferraris @ 2026-03-06 16:24 UTC (permalink / raw)
  To: Xu Yang
  Cc: badhri, heikki.krogerus, gregkh, dsimic, linux-usb, linux-kernel,
	imx, jun.li

Hi Xu,

Le 06/03/2026 à 10:52, Xu Yang a écrit :
> Hi Arnaud,
> 
> On Thu, Mar 05, 2026 at 05:36:08PM +0100, Arnaud Ferraris wrote:
>> Hi Xu,
>>
>> Le 05/03/2026 à 10:40, Xu Yang a écrit :
>>> On Fri, Feb 27, 2026 at 04:45:30PM +0100, Arnaud Ferraris wrote:
>>>> Hi Xu,
>>>>
>>>> Le 25/02/2026 à 03:57, Xu Yang a écrit :
>>>>> 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?
>>>>
>>>> Sure, I'll come up with more details asap, either tomorrow or early next
>>>> week.
>>>
>>> Do you have any updates about this?
>>
>> I do, sorry it took so long...
>>
>> So fwnode_usb_role_switch_get() does indeed return -EPROBE_DEFER, then
>> keeps doing so on later attempts if I revert my patch. However,
>> usb_role_switch_get() succeeds on first try.
>>
>> Please note that:
>> 1. I don't understand much (if any) of the Linux typec stack, and only
>>     noticed 2d8713f807 broke my device, hence my attempted fix
>> 2. said device is the PinePhone Pro, using an out-of-tree dts (and many
>>     drivers) from https://codeberg.org/megi/linux
>>
>> The proper solution likely lies somewhere in the "get proper drivers
>> and upstream dts for this device" land, although I definitely can't
>> commit to this.
>>
>> I think saving the fwnode_usb_role_switch_get() return value and
>> restoring it if usb_role_switch_get() fails would be a decent
>> workaround, although I'm definitely open to suggestions.
>>
>> Feel free to let me know if there's any other test I could run, I'll do
>> my best at replying promptly.
> 
> Thanks for your update!
> 
> After review the dts file, I finally find you meet the issue with:
> 
> 	usb-role-switch = <&typec_extcon_bridge>;
> 
> instead of using below device graph port node.
> 
> 	fusb0: typec-portc@22 {
> 		compatible = "fcs,fusb302";
> 		...
> 
> 		port {
> 			remote-endpoint = <&typec_extcon_bridge>;
> 		};
> 
> 		connector {
> 			...
> 		};
> 	};
> 
> The commit 1366cd228b0 message "If ports are defined in the tcpc main node"
> has confused me at the beginning.

Right, that's where my lack of understanding played its part, sorry for 
the confusion...

> 
> Anyway, it should be another potential issue. Can you test whether below
> patch fix your issue?
> 
> diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
> index b8e28ceca51e..edec139b68b5 100644
> --- a/drivers/usb/roles/class.c
> +++ b/drivers/usb/roles/class.c
> @@ -139,9 +139,14 @@ static void *usb_role_switch_match(const struct fwnode_handle *fwnode, const cha
>   static struct usb_role_switch *
>   usb_role_switch_is_parent(struct fwnode_handle *fwnode)
>   {
> -       struct fwnode_handle *parent = fwnode_get_parent(fwnode);
> +       struct fwnode_handle *parent;
>          struct device *dev;
> 
> +       if (!fwnode_device_is_compatible(fwnode, "usb-b-connector"))
> +               return NULL;
> +
> +       parent = fwnode_get_parent(fwnode);
> +
>          if (!fwnode_property_present(parent, "usb-role-switch")) {
>                  fwnode_handle_put(parent);
>                  return NULL;

I can confirm that reverting 1366cd228b0 and applying the above works on 
this device.

Thanks,
Arnaud

> 
> Thanks,
> Xu Yang


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Revert "tcpm: allow looking for role_sw device in the main node"
  2026-03-06 16:24             ` Arnaud Ferraris
@ 2026-03-09  7:36               ` Xu Yang
  0 siblings, 0 replies; 9+ messages in thread
From: Xu Yang @ 2026-03-09  7:36 UTC (permalink / raw)
  To: Arnaud Ferraris
  Cc: badhri, heikki.krogerus, gregkh, dsimic, linux-usb, linux-kernel,
	imx, jun.li

On Fri, Mar 06, 2026 at 05:24:43PM +0100, Arnaud Ferraris wrote:
> > 
> > Anyway, it should be another potential issue. Can you test whether below
> > patch fix your issue?
> > 
> > diff --git a/drivers/usb/roles/class.c b/drivers/usb/roles/class.c
> > index b8e28ceca51e..edec139b68b5 100644
> > --- a/drivers/usb/roles/class.c
> > +++ b/drivers/usb/roles/class.c
> > @@ -139,9 +139,14 @@ static void *usb_role_switch_match(const struct fwnode_handle *fwnode, const cha
> >   static struct usb_role_switch *
> >   usb_role_switch_is_parent(struct fwnode_handle *fwnode)
> >   {
> > -       struct fwnode_handle *parent = fwnode_get_parent(fwnode);
> > +       struct fwnode_handle *parent;
> >          struct device *dev;
> > 
> > +       if (!fwnode_device_is_compatible(fwnode, "usb-b-connector"))
> > +               return NULL;
> > +
> > +       parent = fwnode_get_parent(fwnode);
> > +
> >          if (!fwnode_property_present(parent, "usb-role-switch")) {
> >                  fwnode_handle_put(parent);
> >                  return NULL;
> 
> I can confirm that reverting 1366cd228b0 and applying the above works on
> this device.

Good to know. 
Thank you!

Best Regards,
Xu Yang

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-03-09  7:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-24 11:01 [PATCH] Revert "tcpm: allow looking for role_sw device in the main node" Xu Yang
2026-02-24 11:33 ` Arnaud Ferraris
2026-02-25  2:57   ` Xu Yang
2026-02-27 15:45     ` Arnaud Ferraris
2026-03-05  9:40       ` Xu Yang
2026-03-05 16:36         ` Arnaud Ferraris
2026-03-06  9:52           ` Xu Yang
2026-03-06 16:24             ` Arnaud Ferraris
2026-03-09  7:36               ` Xu Yang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox