usb: typec: ucsi: Skip CAM query when partner has no alt modes
Subsystems (3)
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.
--- drivers/usb/typec/ucsi/ucsi.c | 9 +++++++++ 1 file changed, 9 insertions(+)
drivers/usb/typec/ucsi/ucsi.c +9 -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 ]) +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
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 sendUCSI_GET_CURRENT_CAMto 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")Cc [email protected] # v7.0+Assisted-by Codex:deepseek-v4.1-flashSigned-off-by Gris Ge <[email protected]>--- drivers/usb/typec/ucsi/ucsi.c | 9 +++++++++ 1 file changed, 9 insertions(+)drivers/usb/typec/ucsi/ucsi.c +0
-0
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
Heikki Krogerus
maintainerQuoted 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 sendUCSI_GET_CURRENT_CAMto 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")Cc [email protected] # v7.0+Assisted-by Codex:deepseek-v4.1-flashSigned-off-by Gris Ge <[email protected]>--- drivers/usb/typec/ucsi/ucsi.c | 9 +++++++++ 1 file changed, 9 insertions(+)drivers/usb/typec/ucsi/ucsi.c +0
-0drivers/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
PATCH v2 1/1 usb: typec: ucsi: Skip CAM query when partner has no alt modes
In reply to Gris GeSince 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.
---
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
@@ -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
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 sendUCSI_GET_CURRENT_CAMto 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")Cc [email protected] # v7.0+Assisted-by Codex:deepseek-v4.1-flashSigned-off-by Gris Ge <[email protected]>---
v2:
- skip the invoke ofucsi_altmode_update_active()inucsi_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
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