[PATCH v2 0/2] improve usb_role_switch_is_parent()

[PATCH v2 0/2] improve usb_role_switch_is_parent()

[Xu Yang]

This patchset will revert a recent fix patch which will introduce another
regression, and fix the true underlying issue.

Arnaud pushed a fix patch commit 1366cd228b0c ("tcpm: allow looking for
role_sw device in the main node") because commit 2d8713f807a4
("tcpm: switch check for role_sw device with fw_node") swap the order of
usb_role_switch_get() and fwnode_usb_role_switch_get() and this cause the
fwnode_usb_role_switch_get() return "-EPROBE_DEFER" all the time in his
usecase.

The said dts use below property to get role switch device: 

  fusb0: typec-portc@22 {
  	  compatible = "fcs,fusb302";
  	  usb-role-switch = <&typec_extcon_bridge>;
  	  ...
   
  	  connector {
  	  }
  }

So the fix patch works because usb_role_switch_get() will correctly find
the role switch device in case fwnode_usb_role_switch_get() is failed.
However, usb_role_switch_get() shouldn't be called at all in this case
because tcpc->fwnode is already correctly set to @connector. And worse
more, this bring another issue that is once fwnode_usb_role_switch_get()
return "-EPROBE_DEFER" this info may be lost if usb_role_switch_get()
then return NULL.

After further research into this issue, finally I found
usb_role_switch_is_parent() not work properly when it parse non-boolean
"usb-role-switch" property which is for back compatibility purpose. To fix
the issue, patch#2 is added.

Xu Yang (2):
  Revert "tcpm: allow looking for role_sw device in the main node"
  usb: roles: get usb role switch from parent only for usb-b-connector

 drivers/usb/roles/class.c     | 7 ++++++-
 drivers/usb/typec/tcpm/tcpm.c | 2 +-
 2 files changed, 7 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH v2 1/2] Revert "tcpm: allow looking for role_sw device in the main node"

[Xu Yang]

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 a NULL check to IS_ERR_OR_NULL(), usb_role_switch_get()
returns NULL and overwrites the ERR_PTR(-EPROBE_DEFER) returned by
fwnode_usb_role_switch_get(). This causes the deferred probe indication to
be lost, preventing the USB role switch from ever being retrieved.

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>

---
Changes in v2:
 - no changes
---
 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

[PATCH v2 2/2] usb: roles: get usb role switch from parent only for usb-b-connector

[Xu Yang]

usb_role_switch_is_parent() was walking up to the parent node and checking
for the "usb-role-switch" property regardless of the type of the passed
fwnode. This could cause unrelated device nodes to be probed as potential
role switch parent, leading to spurious matches and "-EPROBE_DEFER" being
returned infinitely.

Till now only Type-B connector node will have a parent node which may
present "usb-role-switch" property and register the role switch device.
For Type-C connector node, its parent node will always be a Type-C chip
device which will never register the role switch device. However, it may
still present a non-boolean "usb-role-switch = <&usb_controller>" property
for historical compatibility.

So restrict the helper to only operate on Type-B connector when attempting
to get the role switch from parent node.

Fixes: 6fadd72943b8 ("usb: roles: get usb-role-switch from parent")
Cc: stable@vger.kernel.org
Signed-off-by: Xu Yang <xu.yang_2@nxp.com>

---
Changes in v2:
 - new patch
---
 drivers/usb/roles/class.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

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;
-- 
2.34.1

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

[Arnaud Ferraris]

Le 09/03/2026 à 08:43, Xu Yang a écrit :
> 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 a NULL check to IS_ERR_OR_NULL(), usb_role_switch_get()
> returns NULL and overwrites the ERR_PTR(-EPROBE_DEFER) returned by
> fwnode_usb_role_switch_get(). This causes the deferred probe indication to
> be lost, preventing the USB role switch from ever being retrieved.
> 
> 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>
> 
> ---
> Changes in v2:
>   - no changes
> ---
>   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);

Tested-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>

Cheers,
Arnaud

Re: [PATCH v2 2/2] usb: roles: get usb role switch from parent only for usb-b-connector

[Arnaud Ferraris]

Le 09/03/2026 à 08:43, Xu Yang a écrit :
> usb_role_switch_is_parent() was walking up to the parent node and checking
> for the "usb-role-switch" property regardless of the type of the passed
> fwnode. This could cause unrelated device nodes to be probed as potential
> role switch parent, leading to spurious matches and "-EPROBE_DEFER" being
> returned infinitely.
> 
> Till now only Type-B connector node will have a parent node which may
> present "usb-role-switch" property and register the role switch device.
> For Type-C connector node, its parent node will always be a Type-C chip
> device which will never register the role switch device. However, it may
> still present a non-boolean "usb-role-switch = <&usb_controller>" property
> for historical compatibility.
> 
> So restrict the helper to only operate on Type-B connector when attempting
> to get the role switch from parent node.
> 
> Fixes: 6fadd72943b8 ("usb: roles: get usb-role-switch from parent")
> Cc: stable@vger.kernel.org
> Signed-off-by: Xu Yang <xu.yang_2@nxp.com>
> 
> ---
> Changes in v2:
>   - new patch
> ---
>   drivers/usb/roles/class.c | 7 ++++++-
>   1 file changed, 6 insertions(+), 1 deletion(-)
> 
> 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;

Tested-by: Arnaud Ferraris <arnaud.ferraris@collabora.com>

Cheers,
Arnaud

Re: [PATCH v2 0/2] improve usb_role_switch_is_parent()

[Heikki Krogerus]

Mon, Mar 09, 2026 at 03:43:11PM +0800, Xu Yang kirjoitti:
> This patchset will revert a recent fix patch which will introduce another
> regression, and fix the true underlying issue.
> 
> Arnaud pushed a fix patch commit 1366cd228b0c ("tcpm: allow looking for
> role_sw device in the main node") because commit 2d8713f807a4
> ("tcpm: switch check for role_sw device with fw_node") swap the order of
> usb_role_switch_get() and fwnode_usb_role_switch_get() and this cause the
> fwnode_usb_role_switch_get() return "-EPROBE_DEFER" all the time in his
> usecase.
> 
> The said dts use below property to get role switch device: 
> 
>   fusb0: typec-portc@22 {
>   	  compatible = "fcs,fusb302";
>   	  usb-role-switch = <&typec_extcon_bridge>;
>   	  ...
>    
>   	  connector {
>   	  }
>   }
> 
> So the fix patch works because usb_role_switch_get() will correctly find
> the role switch device in case fwnode_usb_role_switch_get() is failed.
> However, usb_role_switch_get() shouldn't be called at all in this case
> because tcpc->fwnode is already correctly set to @connector. And worse
> more, this bring another issue that is once fwnode_usb_role_switch_get()
> return "-EPROBE_DEFER" this info may be lost if usb_role_switch_get()
> then return NULL.
> 
> After further research into this issue, finally I found
> usb_role_switch_is_parent() not work properly when it parse non-boolean
> "usb-role-switch" property which is for back compatibility purpose. To fix
> the issue, patch#2 is added.
> 
> Xu Yang (2):
>   Revert "tcpm: allow looking for role_sw device in the main node"
>   usb: roles: get usb role switch from parent only for usb-b-connector
> 
>  drivers/usb/roles/class.c     | 7 ++++++-
>  drivers/usb/typec/tcpm/tcpm.c | 2 +-
>  2 files changed, 7 insertions(+), 2 deletions(-)

For the series:

Reviewed-by: Heikki Krogerus <heikki.krogerus@linux.intel.com>

Thanks Xu!

-- 
heikki

Re: [PATCH v2 2/2] usb: roles: get usb role switch from parent only for usb-b-connector

[Frank Li]

From: Frank Li (AI-BOT) <frank.li@nxp.com>

> if (!fwnode_device_is_compatible(fwnode, "usb-b-connector"))
> 		return NULL;

AI: Nit: inconsistent indentation. Should use a single tab, not two spaces.
Use '\t' for the return statement to match kernel style.

> 	parent = fwnode_get_parent(fwnode);
>
> 	if (!fwnode_property_present(parent, "usb-role-switch")) {

AI: Missing NULL check on parent before passing to fwnode_property_present().
If fwnode_get_parent() returns NULL, the next call will dereference it.
Add: if (!parent) return NULL;

Frank

Re: [PATCH v2 2/2] usb: roles: get usb role switch from parent only for usb-b-connector

[Xu Yang]

Hi Frank,

On Mon, Mar 09, 2026 at 03:48:59PM -0400, Frank Li wrote:
> From: Frank Li (AI-BOT) <frank.li@nxp.com>
> 
> > if (!fwnode_device_is_compatible(fwnode, "usb-b-connector"))
> > 		return NULL;
> 
> AI: Nit: inconsistent indentation. Should use a single tab, not two spaces.
> Use '\t' for the return statement to match kernel style.
> 
> > 	parent = fwnode_get_parent(fwnode);
> >
> > 	if (!fwnode_property_present(parent, "usb-role-switch")) {
> 
> AI: Missing NULL check on parent before passing to fwnode_property_present().
> If fwnode_get_parent() returns NULL, the next call will dereference it.
> Add: if (!parent) return NULL;

Ignore this!

Both above AI reported issues are false-alarm. Please refine the AI-BOT.
And the message he sent didn't reach to me (I'm not in --to list). So the
issue may not be resolved timely.

Thanks,
Xu Yang