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

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

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