[v2] usb: typec: ucsi: Set orientation as none when connector is unplugged

Commit Message

Abel Vesa Dec. 12, 2024, 5:37 p.m. UTC

The current implementation of the ucsi glink client connector_status()
callback is only relying on the state of the gpio. This means that even
when the cable is unplugged, the orientation propagated to the switches
along the graph is "orientation normal", instead of "orientation none",
which would be the correct one in this case.

One of the Qualcomm DP-USB PHY combo drivers, which needs to be aware of
the orientation change, is relying on the "orientation none" to skip
the reinitialization of the entire PHY. Since the ucsi glink client
advertises "orientation normal" even when the cable is unplugged, the
mentioned PHY is taken down and reinitialized when in fact it should be
left as-is. This triggers a crash within the displayport controller driver
in turn, which brings the whole system down on some Qualcomm platforms.
Propagating "orientation none" from the ucsi glink client on the
connector_status() callback hides the problem of the mentioned PHY driver
away for now. But the "orientation none" is nonetheless the correct one
to be used in this case.

So propagate the "orientation none" instead when the connector status
flags says cable is disconnected.

Fixes: 76716fd5bf09 ("usb: typec: ucsi: glink: move GPIO reading into connector_status callback")
Cc: [e-mail] # 6.10
Reviewed-by: Bryan O'Donoghue <[e-mail]>
Reviewed-by: Heikki Krogerus <[e-mail]>
Reviewed-by: Neil Armstrong <[e-mail]>
Signed-off-by: Abel Vesa <[e-mail]>
---
Changes in v2:
- Re-worded the commit message to explain a bit more what is happening.
- Added Fixes tag and CC'ed stable.
- Dropped the RFC prefix.
- Used the new UCSI_CONSTAT macro which did not exist when v1 was sent.
- Link to v1: https://lore.kernel.org/r/2024[e-mail]
---
 drivers/usb/typec/ucsi/ucsi_glink.c | 5 +++++
 1 file changed, 5 insertions(+)


---
base-commit: 3e42dc9229c5950e84b1ed705f94ed75ed208228
change-id: 20241017-usb-typec-ucsi-glink-add-orientation-none-73f1f2522999

Best regards,

Comments

Johan Hovold Dec. 13, 2024, 9 a.m. UTC | #1

On Thu, Dec 12, 2024 at 07:37:43PM +0200, Abel Vesa wrote:
> The current implementation of the ucsi glink client connector_status()
> callback is only relying on the state of the gpio. This means that even
> when the cable is unplugged, the orientation propagated to the switches
> along the graph is "orientation normal", instead of "orientation none",
> which would be the correct one in this case.
> 
> One of the Qualcomm DP-USB PHY combo drivers, which needs to be aware of
> the orientation change, is relying on the "orientation none" to skip
> the reinitialization of the entire PHY. Since the ucsi glink client
> advertises "orientation normal" even when the cable is unplugged, the
> mentioned PHY is taken down and reinitialized when in fact it should be
> left as-is. This triggers a crash within the displayport controller driver
> in turn, which brings the whole system down on some Qualcomm platforms.
> Propagating "orientation none" from the ucsi glink client on the
> connector_status() callback hides the problem of the mentioned PHY driver
> away for now. But the "orientation none" is nonetheless the correct one
> to be used in this case.
> 
> So propagate the "orientation none" instead when the connector status
> flags says cable is disconnected.
> 
> Fixes: 76716fd5bf09 ("usb: typec: ucsi: glink: move GPIO reading into connector_status callback")
> Cc: [e-mail] # 6.10
> Reviewed-by: Bryan O'Donoghue <[e-mail]>
> Reviewed-by: Heikki Krogerus <[e-mail]>
> Reviewed-by: Neil Armstrong <[e-mail]>
> Signed-off-by: Abel Vesa <[e-mail]>
> ---
> Changes in v2:
> - Re-worded the commit message to explain a bit more what is happening.
> - Added Fixes tag and CC'ed stable.
> - Dropped the RFC prefix.
> - Used the new UCSI_CONSTAT macro which did not exist when v1 was sent.
> - Link to v1: https://lore.kernel.org/r/2024[e-mail]

Thanks for the update.

Reviewed-by: Johan Hovold <[e-mail]>
Tested-by: Johan Hovold <[e-mail]>

Johan

Patch

diff --git a/drivers/usb/typec/ucsi/ucsi_glink.c b/drivers/usb/typec/ucsi/ucsi_glink.c
index 90948cd6d2972402465a2adaba3e1ed055cf0cfa..fed39d45809050f1e08dc1d34008b5c561461391 100644
--- a/drivers/usb/typec/ucsi/ucsi_glink.c
+++ b/drivers/usb/typec/ucsi/ucsi_glink.c
@@ -185,6 +185,11 @@  static void pmic_glink_ucsi_connector_status(struct ucsi_connector *con)
 	struct pmic_glink_ucsi *ucsi = ucsi_get_drvdata(con->ucsi);
 	int orientation;
 
+	if (!UCSI_CONSTAT(con, CONNECTED)) {
+		typec_set_orientation(con->port, TYPEC_ORIENTATION_NONE);
+		return;
+	}
+
 	if (con->num > PMIC_GLINK_MAX_PORTS ||
 	    !ucsi->port_orientation[con->num - 1])
 		return;