Skip to contentSkip to conversation

usb: typec: ucsi: Skip CAM query when partner has no alt modes

PATCH Superseded Gris Ge 3 participants 5 messages Sep 27 – Sep 29

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Gris Ge

author
#0 lore raw

PATCH 1/1 usb: typec: ucsi: Skip CAM query when partner has no alt modes

Since Thunderbolt alternate mode support was added, ucsi_altmode_update_active() is called on every Connector Partner Changed event. This made the driver send UCSI_GET_CURRENT_CAM to PPMs even when the partner had no alternate modes registered. Firmware that does not support the optional alternate mode details answered Not Supported, which produced a spurious error on every partner change:

```
ucsi_acpi USBC000:00: GET_CURRENT_CAM command failed
```

Fixed by skipping the command when there is nothing to update.

Fixes da87d45b1951 ("usb: typec: ucsi: Add Thunderbolt alternate mode support")
Assisted-by Codex:deepseek-v4.1-flash
---
 drivers/usb/typec/ucsi/ucsi.c | 9 +++++++++
 1 file changed, 9 insertions(+)
drivers/usb/typec/ucsi/ucsi.c +9 -0
index bef3f9b71d71..71b28e4df470 100644
@@ -360,6 +360,15 @@ void ucsi_altmode_update_active(struct ucsi_connector *con)
 	u8 cur;
 	int i;
 
+	/*
+	 * Alternate mode support is optional, and the PPM is not required to
+	 * support GET_CURRENT_CAM when it does not support alternate modes.
+	 * There is also nothing to update when the partner has not registered
+	 * any alternate modes, so don't send the command in that case.
+	 */
+	if (!con->partner_altmode[0])
+		return;
+
 	command = UCSI_GET_CURRENT_CAM | UCSI_CONNECTOR_NUMBER(con->num);
 	ret = ucsi_send_command(con->ucsi, command, &cur, sizeof(cur));
 	if (ret < 0) {
Signature · 1 line
2.55.0
#1 lore raw
Quoted 27 lines · Gris Ge

Since Thunderbolt alternate mode support was added, ucsi_altmode_update_active() is called on every Connector Partner Changed event. This made the driver send UCSI_GET_CURRENT_CAM to PPMs even when the partner had no alternate modes registered. Firmware that does not support the optional alternate mode details answered Not Supported, which produced a spurious error on every partner change:

```
ucsi_acpi USBC000:00: GET_CURRENT_CAM command failed
```

Fixed by skipping the command when there is nothing to update.

Fixes da87d45b1951 ("usb: typec: ucsi: Add Thunderbolt alternate mode support")
Assisted-by Codex:deepseek-v4.1-flash
---
 drivers/usb/typec/ucsi/ucsi.c | 9 +++++++++
 1 file changed, 9 insertions(+)
drivers/usb/typec/ucsi/ucsi.c +0 -0
index bef3f9b71d71..71b28e4df470 100644

drivers/usb/typec/ucsi/ucsi.c void ucsi_altmode_update_active(struct ucsi_connector *con) · line 369

Quoted 10 lines
drivers/usb/typec/ucsi/ucsi.c +7 -0
@@ -360,6 +360,15 @@ void ucsi_altmode_update_active(struct ucsi_connector *con)
        u8 cur;
        int i;
 
+       /*
+        * Alternate mode support is optional, and the PPM is not required to
+        * support GET_CURRENT_CAM when it does not support alternate modes.
+        * There is also nothing to update when the partner has not registered
+        * any alternate modes, so don't send the command in that case.
+        */
+       if (!con->partner_altmode[0])

Should we move this validation into ucsi_handle_connector_change?

        if (con->partner_altmode[0])
                ucsi_altmode_update_active(con);

All other call sites are already guarded by this condition. Besides, I don't think we need that comment above.

Thanks,
Andrei

maintainer
#2 lore raw
Quoted 41 lines · Andrei Kuchynski
Quoted 27 lines · Gris Ge

Since Thunderbolt alternate mode support was added, ucsi_altmode_update_active() is called on every Connector Partner Changed event. This made the driver send UCSI_GET_CURRENT_CAM to PPMs even when the partner had no alternate modes registered. Firmware that does not support the optional alternate mode details answered Not Supported, which produced a spurious error on every partner change:

```
ucsi_acpi USBC000:00: GET_CURRENT_CAM command failed
```

Fixed by skipping the command when there is nothing to update.

Fixes da87d45b1951 ("usb: typec: ucsi: Add Thunderbolt alternate mode support")
Assisted-by Codex:deepseek-v4.1-flash
---
 drivers/usb/typec/ucsi/ucsi.c | 9 +++++++++
 1 file changed, 9 insertions(+)
drivers/usb/typec/ucsi/ucsi.c +0 -0
index bef3f9b71d71..71b28e4df470 100644

drivers/usb/typec/ucsi/ucsi.c void ucsi_altmode_update_active(struct ucsi_connector *con) · line 369

Quoted 10 lines
drivers/usb/typec/ucsi/ucsi.c +7 -0
@@ -360,6 +360,15 @@ void ucsi_altmode_update_active(struct ucsi_connector *con)
        u8 cur;
        int i;
 
+       /*
+        * Alternate mode support is optional, and the PPM is not required to
+        * support GET_CURRENT_CAM when it does not support alternate modes.
+        * There is also nothing to update when the partner has not registered
+        * any alternate modes, so don't send the command in that case.
+        */
+       if (!con->partner_altmode[0])

Should we move this validation into ucsi_handle_connector_change?

Quoted 4 lines
        if (con->partner_altmode[0])
                ucsi_altmode_update_active(con);

All other call sites are already guarded by this condition.

Makes sense to me.

Quoted 1 line

Besides, I don't think we need that comment above.

I agree.

Signature · 1 line
heikki
author
#3 lore raw

PATCH v2 1/1 usb: typec: ucsi: Skip CAM query when partner has no alt modes

In reply to Gris Ge

Since Thunderbolt alternate mode support was added, ucsi_altmode_update_active() is called on every Connector Partner Changed event. This made the driver send UCSI_GET_CURRENT_CAM to PPMs even when the partner had no alternate modes registered. Firmware that does not support the optional alternate mode details answered Not Supported, which produced a spurious error on every partner change:

```
ucsi_acpi USBC000:00: GET_CURRENT_CAM command failed
```

Fixed by skipping the command when there is nothing to update.

Fixes da87d45b1951 ("usb: typec: ucsi: Add Thunderbolt alternate mode support")
Assisted-by Codex:deepseek-v4.1-flash

---
v2:
- skip the invoke of ucsi_altmode_update_active() in ucsi_handle_connector_change()
v1: https://lore.kernel.org/linux-usb/[email protected]/

 drivers/usb/typec/ucsi/ucsi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/usb/typec/ucsi/ucsi.c +2 -1
index bef3f9b71d71..c392afdc27ae 100644
@@ -1519,7 +1519,8 @@ static void ucsi_handle_connector_change(struct work_struct *work)
 
 	if (con->partner && (change & UCSI_CONSTAT_PARTNER_CHANGE)) {
 		ucsi_partner_change(con);
-		ucsi_altmode_update_active(con);
+		if (con->partner_altmode[0])
+			ucsi_altmode_update_active(con);
 
 		/* Complete pending data role swap */
 		if (!completion_done(&con->complete))

--
2.55.0

#4 lore raw
Quoted 32 lines · Gris Ge

Since Thunderbolt alternate mode support was added, ucsi_altmode_update_active() is called on every Connector Partner Changed event. This made the driver send UCSI_GET_CURRENT_CAM to PPMs even when the partner had no alternate modes registered. Firmware that does not support the optional alternate mode details answered Not Supported, which produced a spurious error on every partner change:

```
ucsi_acpi USBC000:00: GET_CURRENT_CAM command failed
```

Fixed by skipping the command when there is nothing to update.

Fixes da87d45b1951 ("usb: typec: ucsi: Add Thunderbolt alternate mode support")
Assisted-by Codex:deepseek-v4.1-flash

---
v2:
- skip the invoke of ucsi_altmode_update_active() in ucsi_handle_connector_change()
v1: https://lore.kernel.org/linux-usb/[email protected]/

 drivers/usb/typec/ucsi/ucsi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/usb/typec/ucsi/ucsi.c +0 -0
index bef3f9b71d71..c392afdc27ae 100644

drivers/usb/typec/ucsi/ucsi.c static void ucsi_handle_connector_change(struct work_struct *work) · line 1523

Quoted 6 lines
drivers/usb/typec/ucsi/ucsi.c +2 -1
@@ -1519,7 +1519,8 @@ static void ucsi_handle_connector_change(struct work_struct *work)
 
        if (con->partner && (change & UCSI_CONSTAT_PARTNER_CHANGE)) {
                ucsi_partner_change(con);
-               ucsi_altmode_update_active(con);
+               if (con->partner_altmode[0])
+                       ucsi_altmode_update_active(con);

Thanks for the patch.
Could you please send v2 as a new email thread?

Thanks,
Andrei