Re: [PATCH 1/4] phy: qcom: qmp-combo: delay Type-C mux switch while DP PHY is initializing

[email protected]
Newsgroups org.freedesktop.lists.dri-devel,org.infradead.lists.linux-phy
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix for DP PHY state corruption during Type-C orientation changes.
- [High] Type-C mux state updates are silently dropped when DP PHY is active, breaking dynamic pin assignment.
- [High] Integer underflow of `usb_init_count` breaks USB PHY state tracking.
--

--- Patch [1]: [PATCH 1/4] phy: qcom: qmp-combo: delay Type-C mux switch while DP PHY is initializing ---
commit 35456fff470c94d1e75b9f16c438e1e8cf5df40f
Author: Saurabh Anand <[email protected]>

phy: qcom: qmp-combo: delay Type-C mux switch while DP PHY is initializing

This commit extends the Type-C mux switch guard to check dp_init_count in
addition to dp_powered_on. This prevents a race window during DP PHY
initialization where a Type-C orientation change could corrupt the PHY state.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index c39ced168d035..f23aff79ca08f 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -4842,7 +4842,8 @@ static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s
>  		return 0;
>  	}
>  
> -	if (qmp->qmpphy_mode != QMPPHY_MODE_USB3_ONLY && qmp->dp_powered_on) {
> +	if (qmp->qmpphy_mode != QMPPHY_MODE_USB3_ONLY &&
> +	    (qmp->dp_powered_on || qmp->dp_init_count)) {
>  		dev_dbg(qmp->dev, "typec_mux_set: DP PHY is still in use, delaying switch\n");
>  		return 0;
>  	}

[Severity: High]
This is a pre-existing issue, but does this early return cause the driver to
silently drop legitimate mux state updates?

When the Type-C framework negotiates a dynamic pin assignment and calls
qmp_combo_typec_mux_set(), returning 0 here without saving the requested
mode to qmp->qmpphy_mode or queuing it means the hardware could remain
stuck in the old lane configuration, causing subsequent DRM link
retraining to fail.

[Severity: High]
This is a pre-existing issue, but is a similar guard needed in
qmp_combo_typec_switch_set()?

The current patch fixes the race in qmp_combo_typec_mux_set(), but a
Type-C orientation change also triggers qmp_combo_typec_switch_set().
If DP is active or initializing, qmp_combo_typec_switch_set() forcefully
resets the COM block:

drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_typec_switch_set() {
        ...
        qmp_combo_com_exit(qmp, true);
        qmp_combo_com_init(qmp, true);
        ...
}

Does this hardware reset wipe out the DP PHY lane and link rate
configurations applied by qmp_combo_dp_power_on(), leaving the
software state (dp_powered_on) desynchronized from the hardware?

[Severity: High]
This is a pre-existing issue, but is there a risk of an integer
underflow with usb_init_count if the USB subsystem also decrements it?

When the Type-C mux switches to QMPPHY_MODE_DP_ONLY,
qmp_combo_typec_mux_set() manually decrements the counter:

drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_typec_mux_set() {
        ...
        if (qmp->usb_init_count)
                qmp->usb_init_count--;
        ...
}

If the USB controller later undergoes a suspend or role switch and
calls phy_exit(), qmp_combo_usb_exit() will decrement it again:

drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_usb_exit() {
        ...
        qmp->usb_init_count--;
        ...
}

Could this underflow usb_init_count to UINT_MAX, causing a subsequent
Type-C orientation flip to fail to power the USB PHY back on?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260824-qcom-dp-typec-reconnect-fixes-v1-0-2825e5bf8a96@oss.qualcomm.com?part=1
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.