Re: [PATCH 2/5] usb: typec: ps883x: Return -EOPNOTSUPP for USB4 when parade,disable-usb4 is set

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Encoding a temporary software workaround into the Device Tree using a custom property (`parade,disable-usb4`).
--

--- Patch [2]: [PATCH 2/5] usb: typec: ps883x: Return -EOPNOTSUPP for USB4 when parade,disable-usb4 is set ---
commit e075e4adfa41dd9a2eea759527c1bbbc8a97f7a9
Author: Jens Glathe <jens.glathe@oldschoolsolutions.biz>
Subject: usb: typec: ps883x: Return -EOPNOTSUPP for USB4 when parade,disable-usb4 is set

This commit adds a temporary workaround to disable USB4 support on the
PS883x retimer using a new device tree property. It addresses an issue on
X1E80100 platforms where negotiating USB4 results in no DisplayPort output
due to missing software support in the qmp_combo PHY stack.

Link: https://patch.msgid.link/20260312101431.2375709-1-krishna.kurapati@oss.qualcomm.com

> diff --git a/drivers/usb/typec/mux/ps883x.c b/drivers/usb/typec/mux/ps883x.c
> index 64e0a61b776a1..3fa26ce01a9c9 100644
> --- a/drivers/usb/typec/mux/ps883x.c
> +++ b/drivers/usb/typec/mux/ps883x.c
[ ... ]
> @@ -249,6 +250,13 @@ static int ps883x_set(struct ps883x_retimer *retimer, struct typec_retimer_state
>  			cfg0 |= CONN_STATUS_0_USB_3_1_CONNECTED;
>  			break;
>  		case TYPEC_MODE_USB4:
> +			if (retimer->disable_usb4) {
> +				dev_info(&retimer->client->dev,
> +					 "USB4 disabled via DT property, rejecting USB4 mode\n");
> +				return -EOPNOTSUPP;
> +			}

[Severity: High]
Since the commit message notes this is a temporary workaround for missing OS
support in the X1E USB4 controller and PHY stack, does this approach conflict
with the principle that the device tree is a stable hardware description ABI?

If this workaround is deployed in user device trees or firmware, wouldn't
those boards permanently have USB4 disabled on this retimer even after the
kernel drivers are eventually updated to fully support it?

Could this be implemented as a software quirk instead, perhaps by matching
the platform's compatible string or inferring the limitation dynamically
from the dependent drivers, rather than altering the hardware description?

> +
> +			/* Normal USB4 handling */
>  			eudo_data = state->data;
>  
>  			cfg2 |= CONN_STATUS_2_USB4_CONNECTED;
[ ... ]
> @@ -378,6 +386,8 @@ static int ps883x_retimer_probe(struct i2c_client *client)
>  
>  	retimer->client = client;
>  
> +	retimer->disable_usb4 = device_property_read_bool(dev, "parade,disable-usb4");
> +
>  	mutex_init(&retimer->lock);
>  
>  	retimer->regmap = devm_regmap_init_i2c(client, &ps883x_retimer_regmap);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718-ps883x-disable-usb4-v1-0-cec86d0b909e@oldschoolsolutions.biz?part=2