All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Saurabh Anand" <saurabh.anand@oss.qualcomm.com>
Cc: dri-devel@lists.freedesktop.org, vkoul@kernel.org,
	neil.armstrong@linaro.org, linux-phy@lists.infradead.org,
	olteanv@gmail.com
Subject: Re: [PATCH 1/4] phy: qcom: qmp-combo: delay Type-C mux switch while DP PHY is initializing
Date: Mon, 24 Aug 2026 11:44:01 +0000	[thread overview]
Message-ID: <20260824114402.128B01F00A3D@smtp.kernel.org> (raw)
In-Reply-To: <20260824-qcom-dp-typec-reconnect-fixes-v1-1-2825e5bf8a96@oss.qualcomm.com>

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 <saurabh.anand@oss.qualcomm.com>

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

WARNING: multiple messages have this Message-ID (diff)
From: sashiko-bot@kernel.org
To: "Saurabh Anand" <saurabh.anand@oss.qualcomm.com>
Cc: dri-devel@lists.freedesktop.org, vkoul@kernel.org,
	neil.armstrong@linaro.org, linux-phy@lists.infradead.org,
	olteanv@gmail.com
Subject: Re: [PATCH 1/4] phy: qcom: qmp-combo: delay Type-C mux switch while DP PHY is initializing
Date: Mon, 24 Aug 2026 11:44:01 +0000	[thread overview]
Message-ID: <20260824114402.128B01F00A3D@smtp.kernel.org> (raw)
In-Reply-To: <20260824-qcom-dp-typec-reconnect-fixes-v1-1-2825e5bf8a96@oss.qualcomm.com>

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 <saurabh.anand@oss.qualcomm.com>

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

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

  reply	other threads:[~2026-08-24 11:44 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-24 11:28 [PATCH 0/4] drm/msm/dp, phy: qcom: improve DP Type-C reconnect handling Saurabh Anand
2026-08-24 11:28 ` Saurabh Anand
2026-08-24 11:28 ` [PATCH 1/4] phy: qcom: qmp-combo: delay Type-C mux switch while DP PHY is initializing Saurabh Anand
2026-08-24 11:28   ` Saurabh Anand
2026-08-24 11:44   ` sashiko-bot [this message]
2026-08-24 11:44     ` sashiko-bot
2026-09-02  7:48   ` Manivannan Sadhasivam
2026-09-02  7:48     ` Manivannan Sadhasivam
2026-09-07  9:47     ` Saurabh Anand
2026-09-07  9:47       ` Saurabh Anand
2026-09-13 21:36   ` Dmitry Baryshkov
2026-09-13 21:36     ` Dmitry Baryshkov
2026-08-24 11:28 ` [PATCH 2/4] drm/msm/dp: clear force_link_train flag when link training succeeds Saurabh Anand
2026-08-24 11:28   ` Saurabh Anand
2026-08-24 11:40   ` sashiko-bot
2026-08-24 11:40     ` sashiko-bot
2026-08-24 13:17   ` Konrad Dybcio
2026-08-24 13:17     ` Konrad Dybcio
2026-09-07  9:35     ` Saurabh Anand
2026-09-07  9:35       ` Saurabh Anand
2026-10-04 10:04       ` Oleg Keri
2026-10-04 10:04         ` Oleg Keri
2026-08-24 11:28 ` [PATCH 3/4] drm/msm/dp: skip LTTPR init when no LTTPRs are present Saurabh Anand
2026-08-24 11:28   ` Saurabh Anand
2026-08-24 11:35   ` sashiko-bot
2026-08-24 11:35     ` sashiko-bot
2026-08-24 13:15   ` Konrad Dybcio
2026-08-24 13:15     ` Konrad Dybcio
2026-09-07  9:44     ` Saurabh Anand
2026-09-07  9:44       ` Saurabh Anand
2026-09-13 21:39       ` Dmitry Baryshkov
2026-09-13 21:39         ` Dmitry Baryshkov
2026-08-24 11:28 ` [PATCH 4/4] drm/msm/dp: continue link training downgrade when cable is still plugged Saurabh Anand
2026-08-24 11:28   ` Saurabh Anand
2026-08-24 11:40   ` sashiko-bot
2026-08-24 11:40     ` sashiko-bot
2026-09-13 21:42   ` Dmitry Baryshkov
2026-09-13 21:42     ` Dmitry Baryshkov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260824114402.128B01F00A3D@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=linux-phy@lists.infradead.org \
    --cc=neil.armstrong@linaro.org \
    --cc=olteanv@gmail.com \
    --cc=sashiko-reviews@lists.linux.dev \
    --cc=saurabh.anand@oss.qualcomm.com \
    --cc=vkoul@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.