[PATCH 0/3] phy: qcom: carry live SS-link state without growing enum phy_mode

[PATCH 0/3] phy: qcom: carry live SS-link state without growing enum phy_mode

[Tapas Rana]

QMP PHY drivers always enable receiver detection (ARCVR_DTCT_EN) in
autonomous mode, and additionally enable LFPS detection (ALFPS_DTCT_EN)
when a SuperSpeed link is present; otherwise they fall back to
receiver-detect event selection (ARCVR_DTCT_EVENT_SEL). This choice is
made by comparing phy_mode against
PHY_MODE_USB_HOST_SS/PHY_MODE_USB_DEVICE_SS. However, dwc3 core only
calls phy_set_mode()/phy_set_mode_ext() with the base
PHY_MODE_USB_HOST or PHY_MODE_USB_DEVICE, never the _SS/_HS/_FS/_LS
submodes. As a result, the comparison is always false and the PHY
always falls back to ARCVR_DTCT_EVENT_SEL, the legacy USB2
receiver-detect path, regardless of actual link state.

Add a dedicated qcom_usb_ss_state submode and carry it through the
generic submode argument phy_set_mode_ext() already defines for this
purpose, instead of adding more values to the already-overloaded
phy_mode USB host/device submode list.

Patch 1 adds the shared enum qcom_usb_ss_state (UNKNOWN/PRESENT/
ABSENT) as the submode contract. Patch 2 makes
phy-qcom-qmp-combo.c switch on it instead of on phy_mode. Patch 3
makes dwc3-qcom.c sample live SuperSpeed child-device presence at
suspend entry and pass it down.

The contract is intentionally generic so phy-qcom-qmp-usb.c can adopt
it the same way for the gadget-side case in a follow-up series,
without a second mechanism.

Tested on an x1e80100 CRD. On a host port with a SuperSpeed hub
attached, suspended the system and then connected a SuperSpeed USB
pendrive to the hub; confirmed ALFPS_DTCT_EN is set instead of the
legacy ARCVR_DTCT_EVENT_SEL mask. Repeated the same sequence on the
USB-OTG capable port with the same result. As a cross-check, repeated
both suspend scenarios with a High-Speed-only pendrive attached, and
again with nothing attached, and confirmed the PHY falls back to
ARCVR_DTCT_EVENT_SEL in both cases, as expected when no SuperSpeed
device is present.

This series spans both the generic PHY framework tree and the USB/DWC3
tree: patch 1 adds a new header, patch 2 depends on patch 1 and touches
drivers/phy/qualcomm/, patch 3 depends on patches 1-2 and touches
drivers/usb/dwc3/. All three should be merged in order and ideally
through the same tree (or with an immutable branch) to avoid a window
where dwc3-qcom.c references a submode phy-qcom-qmp-combo.c doesn't
yet switch on. ARM/QUALCOMM MAILING LIST (linux-arm-msm@vger.kernel.org)
is Cc'd on all three patches since it covers both directories.

Tapas Rana (3):
  phy: qcom-qmp-usb-ss: add shared SS-link-state submode contract
  phy: qcom: qmp-combo: Select LFPS detect only when SS link is
    confirmed present
  usb: dwc3: qcom: pass live SS-link state into PHY at suspend entry

 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 27 +++++++++++++++--
 drivers/usb/dwc3/dwc3-qcom.c              | 36 ++++++++++++++++++++++-
 include/linux/phy/phy-qcom-qmp-usb-ss.h   | 17 +++++++++++
 3 files changed, 76 insertions(+), 4 deletions(-)
 create mode 100644 include/linux/phy/phy-qcom-qmp-usb-ss.h

-- 
2.43.0

[PATCH 1/3] phy: qcom-qmp-usb-ss: add shared SS-link-state submode contract

[Tapas Rana]

dwc3 core only calls phy_set_mode()/phy_set_mode_ext() with the base
PHY_MODE_USB_HOST or PHY_MODE_USB_DEVICE. It never passes the
PHY_MODE_USB_{HOST,DEVICE}_SS/HS/FS/LS submode values to any PHY
driver. However, QMP PHY drivers need to know whether a SuperSpeed
link is present to select the correct autonomous-mode wake interrupt
mask, and phy_mode alone cannot carry that.

Add enum qcom_usb_ss_state (UNKNOWN/PRESENT/ABSENT) and carry it
through the existing submode argument of phy_set_mode_ext(), instead
of extending enum phy_mode further. UNKNOWN is the default so callers
that do not yet pass a real value keep current behavior.

Signed-off-by: Tapas Rana <tapas.rana@oss.qualcomm.com>
---
 include/linux/phy/phy-qcom-qmp-usb-ss.h | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)
 create mode 100644 include/linux/phy/phy-qcom-qmp-usb-ss.h

diff --git a/include/linux/phy/phy-qcom-qmp-usb-ss.h b/include/linux/phy/phy-qcom-qmp-usb-ss.h
new file mode 100644
index 000000000000..4da4d71e977d
--- /dev/null
+++ b/include/linux/phy/phy-qcom-qmp-usb-ss.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Shared submode contract between dwc3-qcom glue and the Qualcomm QMP
+ * USB3/combo PHY driver for communicating live SuperSpeed link presence
+ * at suspend-entry, via phy_set_mode_ext()'s submode argument.
+ */
+
+#ifndef _PHY_QCOM_QMP_USB_SS_H_
+#define _PHY_QCOM_QMP_USB_SS_H_
+
+enum qcom_usb_ss_state {
+	QCOM_USB_SS_UNKNOWN = 0,
+	QCOM_USB_SS_PRESENT,
+	QCOM_USB_SS_ABSENT,
+};
+
+#endif /* _PHY_QCOM_QMP_USB_SS_H_ */
-- 
2.43.0

Re: [PATCH 1/3] phy: qcom-qmp-usb-ss: add shared SS-link-state submode contract

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] phy: qcom-qmp-usb-ss: redundant submode enum duplicating enum
  phy_mode
--

commit 2f108759619603815653af9584eea10cfa75efab
Author: Tapas Rana <tapas.rana@oss.qualcomm.com>

phy: qcom-qmp-usb-ss: add shared SS-link-state submode contract

This commit adds include/linux/phy/phy-qcom-qmp-usb-ss.h with enum
qcom_usb_ss_state to pass SuperSpeed link presence via the submode
argument of phy_set_mode_ext().

> diff --git a/include/linux/phy/phy-qcom-qmp-usb-ss.h b/include/linux/phy/phy-qcom-qmp-usb-ss.h
> new file mode 100644
> index 0000000000000..4da4d71e977d7
> --- /dev/null
> +++ b/include/linux/phy/phy-qcom-qmp-usb-ss.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +/*
> + * Shared submode contract between dwc3-qcom glue and the Qualcomm QMP
> + * USB3/combo PHY driver for communicating live SuperSpeed link presence
> + * at suspend-entry, via phy_set_mode_ext()'s submode argument.
> + */
> +
> +#ifndef _PHY_QCOM_QMP_USB_SS_H_
> +#define _PHY_QCOM_QMP_USB_SS_H_
> +
> +enum qcom_usb_ss_state {
> +	QCOM_USB_SS_UNKNOWN = 0,
> +	QCOM_USB_SS_PRESENT,
> +	QCOM_USB_SS_ABSENT,
> +};
> +
> +#endif /* _PHY_QCOM_QMP_USB_SS_H_ */

[Severity: Low]
Is a new vendor-specific submode header needed when enum phy_mode in
include/linux/phy/phy.h already defines PHY_MODE_USB_HOST_SS and
PHY_MODE_USB_DEVICE_SS for this purpose?

The Qualcomm QMP PHY drivers (including phy-qcom-qmp-combo.c and
phy-qcom-qmp-usb.c) already check for PHY_MODE_USB_HOST_SS and
PHY_MODE_USB_DEVICE_SS to select the autonomous-mode wake interrupt mask:

drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_enable_autonomous_mode() {
	...
	if (qmp->phy_mode == PHY_MODE_USB_HOST_SS ||
	    qmp->phy_mode == PHY_MODE_USB_DEVICE_SS)
		intr_mask = ARCVR_DTCT_EN | ALFPS_DTCT_EN;
	else
		intr_mask = ARCVR_DTCT_EN | ARCVR_DTCT_EVENT_SEL;
	...
}

Passing PHY_MODE_USB_HOST_SS or PHY_MODE_USB_DEVICE_SS when a SuperSpeed
link is present and the base PHY_MODE_USB_HOST or PHY_MODE_USB_DEVICE
mode when it is absent (or unknown) would avoid introducing a duplicate
Qualcomm-specific submode contract under include/linux/phy/ and keep all
QMP USB PHY drivers consistent.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106082716.128968-1-tapas.rana@oss.qualcomm.com?part=1

[PATCH 2/3] phy: qcom: qmp-combo: Select LFPS detect only when SS link is confirmed present

[Tapas Rana]

qmp_combo_enable_autonomous_mode() selects the autonomous-mode wake
interrupt mask by comparing qmp->phy_mode against
PHY_MODE_USB_HOST_SS/PHY_MODE_USB_DEVICE_SS. However, this driver's
set_mode() only ever stores PHY_MODE_USB_HOST or PHY_MODE_USB_DEVICE,
since dwc3 core and dwc3-qcom never pass the _SS variants. As a
result, the comparison is always false and the PHY always falls back
to the ARCVR_DTCT_EN | ARCVR_DTCT_EVENT_SEL mask instead of also
enabling ALFPS_DTCT_EN, regardless of whether a SuperSpeed device is
attached.

Store the qcom_usb_ss_state submode (added in the previous patch)
alongside phy_mode in qmp_combo_usb_set_mode(), and switch
qmp_combo_enable_autonomous_mode() on it instead of on phy_mode, so
LFPS detection is selected only when a SuperSpeed link is confirmed
present.

Signed-off-by: Tapas Rana <tapas.rana@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 27 ++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index aa9dc9716574..813a0955c591 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -27,6 +27,7 @@
 #include <drm/bridge/aux-bridge.h>
 
 #include <dt-bindings/phy/phy-qcom-qmp.h>
+#include <linux/phy/phy-qcom-qmp-usb-ss.h>
 
 #include "phy-qcom-qmp-common.h"
 
@@ -2630,6 +2631,7 @@ struct qmp_combo {
 
 	struct phy *usb_phy;
 	enum phy_mode phy_mode;
+	enum qcom_usb_ss_state  ss_state;
 	unsigned int usb_init_count;
 
 	struct phy *dp_phy;
@@ -4518,6 +4520,16 @@ static int qmp_combo_usb_set_mode(struct phy *phy, enum phy_mode mode, int submo
 
 	qmp->phy_mode = mode;
 
+	switch (submode) {
+	case QCOM_USB_SS_PRESENT:
+	case QCOM_USB_SS_ABSENT:
+		qmp->ss_state = submode;
+		break;
+	default:
+		qmp->ss_state = QCOM_USB_SS_UNKNOWN;
+		break;
+	}
+
 	return 0;
 }
 
@@ -4672,11 +4684,20 @@ static void qmp_combo_enable_autonomous_mode(struct qmp_combo *qmp)
 	void __iomem *pcs_aon = qmp->pcs_aon;
 	u32 intr_mask;
 
-	if (qmp->phy_mode == PHY_MODE_USB_HOST_SS ||
-	    qmp->phy_mode == PHY_MODE_USB_DEVICE_SS)
+	/*
+	 * Use LFPS detection only when a SuperSpeed device is known to be
+	 * present. For absent or unknown state, use receiver detection.
+	 */
+	switch (qmp->ss_state) {
+	case QCOM_USB_SS_PRESENT:
 		intr_mask = ARCVR_DTCT_EN | ALFPS_DTCT_EN;
-	else
+		break;
+	case QCOM_USB_SS_ABSENT:
+	case QCOM_USB_SS_UNKNOWN:
+	default:
 		intr_mask = ARCVR_DTCT_EN | ARCVR_DTCT_EVENT_SEL;
+		break;
+	}
 
 	/* Clear any pending interrupts status */
 	qphy_setbits(pcs_usb, cfg->regs[QPHY_PCS_LFPS_RXTERM_IRQ_CLEAR], IRQ_CLEAR);
-- 
2.43.0

Re: [PATCH 2/3] phy: qcom: qmp-combo: Select LFPS detect only when SS link is confirmed present

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] phy: qcom: qmp-combo: invalid PCS_MISC clamp register access in
  autonomous mode [1]
--

commit ee7ffdc4089bfb74e59a7747c5a2c1ab2e3f8758
Author: Tapas Rana <tapas.rana@oss.qualcomm.com>

phy: qcom: qmp-combo: Select LFPS detect only when SS link is confirmed present

This commit stores the qcom_usb_ss_state submode in qmp_combo_usb_set_mode()
and uses it in qmp_combo_enable_autonomous_mode() to enable LFPS detection
only when a SuperSpeed link is confirmed present.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 187f0cd11f6c3..05d48ba81719a 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c

[ ... ]

> @@ -4657,11 +4669,20 @@ static void qmp_combo_enable_autonomous_mode(struct qmp_combo *qmp)
>  	void __iomem *pcs_aon = qmp->pcs_aon;
>  	u32 intr_mask;
>  
> -	if (qmp->phy_mode == PHY_MODE_USB_HOST_SS ||
> -	    qmp->phy_mode == PHY_MODE_USB_DEVICE_SS)
> +	/*
> +	 * Use LFPS detection only when a SuperSpeed device is known to be
> +	 * present. For absent or unknown state, use receiver detection.
> +	 */
> +	switch (qmp->ss_state) {
> +	case QCOM_USB_SS_PRESENT:
>  		intr_mask = ARCVR_DTCT_EN | ALFPS_DTCT_EN;
> -	else
> +		break;
> +	case QCOM_USB_SS_ABSENT:
> +	case QCOM_USB_SS_UNKNOWN:
> +	default:
>  		intr_mask = ARCVR_DTCT_EN | ARCVR_DTCT_EVENT_SEL;
> +		break;
> +	}
>  
>  	/* Clear any pending interrupts status */
>  	qphy_setbits(pcs_usb, cfg->regs[QPHY_PCS_LFPS_RXTERM_IRQ_CLEAR], IRQ_CLEAR);

[Severity: High]
This problem wasn't introduced by this patch, but can
qmp_combo_enable_autonomous_mode() and qmp_combo_disable_autonomous_mode()
corrupt the PCS_MISC block [1] on V6 platforms that do not define
usb3_pcs_aon?

drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_enable_autonomous_mode() {
	...
	if (pcs_aon) {
		if (qmp->qmpphy_mode == QMPPHY_MODE_USB4)
			qphy_clrbits(pcs_aon, cfg->regs[QPHY_PCS_USB4_CLAMP_ENABLE], CLAMP_EN);
		else
			qphy_clrbits(pcs_aon, cfg->regs[QPHY_PCS_CLAMP_ENABLE], CLAMP_EN);
	} else if (pcs_misc)
		qphy_clrbits(pcs_misc, cfg->regs[QPHY_PCS_CLAMP_ENABLE], CLAMP_EN);
}

Configs such as sm8550_usb3dpphy_cfg and x1e80100_usb3dpphy_cfg pair
qmp_v6_usb3phy_regs_layout or qmp_v6_n4_usb3phy_regs_layout (where
QPHY_PCS_CLAMP_ENABLE is QPHY_V6_PCS_AON_CLAMP_ENABLE, 0x00) with offset
tables that leave usb3_pcs_aon at 0.  In qmp_combo_parse_dt(), qmp->pcs_aon
remains NULL while qmp->pcs_misc is populated unconditionally.

During runtime suspend and resume, the else if (pcs_misc) fallback writes
CLAMP_EN to offset 0x00 of pcs_misc (QPHY_V4_PCS_MISC_TYPEC_CTRL),
corrupting the Type-C control register instead of toggling the hardware
I/O clamp.

[1] https://sashiko.dev/#/bug/linux-975e0709-1f2c-4dcf-beb9-ae3f0e032aa9

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106082716.128968-1-tapas.rana@oss.qualcomm.com?part=2

[PATCH 3/3] usb: dwc3: qcom: pass live SS-link state into PHY at suspend entry

[Tapas Rana]

dwc3_qcom_suspend() puts each host port into autonomous/wake mode
without ever telling the PHY whether a SuperSpeed device is currently
attached. qmp_combo_enable_autonomous_mode(), made SS-aware in the
previous patch, therefore has no live signal to act on and always
sees QCOM_USB_SS_UNKNOWN, falling back to the legacy receiver-detect
mask regardless of actual link state.

Add dwc3_qcom_read_ss_state() to check each port's USB3 roothub for
an attached child device running at SuperSpeed or above, and pass the
result as the submode argument of phy_set_mode_ext() so the PHY can
select LFPS detection when appropriate.

Signed-off-by: Tapas Rana <tapas.rana@oss.qualcomm.com>
---
 drivers/usb/dwc3/dwc3-qcom.c | 36 +++++++++++++++++++++++++++++++++++-
 1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
index ac68b4218b56..d4ba095e47eb 100644
--- a/drivers/usb/dwc3/dwc3-qcom.c
+++ b/drivers/usb/dwc3/dwc3-qcom.c
@@ -19,6 +19,7 @@
 #include <linux/iopoll.h>
 #include <linux/usb/hcd.h>
 #include <linux/usb.h>
+#include <linux/phy/phy-qcom-qmp-usb-ss.h>
 #include "core.h"
 #include "glue.h"
 
@@ -66,6 +67,7 @@ struct dwc3_qcom_port {
 	int			dm_hs_phy_irq;
 	int			ss_phy_irq;
 	enum usb_device_speed	usb2_speed;
+	enum qcom_usb_ss_state  ss_state;
 };
 
 struct dwc3_qcom {
@@ -251,6 +253,34 @@ static enum usb_device_speed dwc3_qcom_read_usb2_speed(struct dwc3_qcom *qcom, i
 	return udev->speed;
 }
 
+static enum qcom_usb_ss_state dwc3_qcom_read_ss_state(struct dwc3_qcom *qcom, int port_index)
+{
+	struct usb_device *udev;
+	struct usb_hcd __maybe_unused *hcd, *usb3_hcd;
+	struct dwc3 *dwc = &qcom->dwc;
+
+	/*
+	 * FIXME: Fix this layering violation.
+	 */
+	hcd = platform_get_drvdata(dwc->xhci);
+	if (!hcd)
+		return QCOM_USB_SS_ABSENT;
+
+#ifdef CONFIG_USB
+	usb3_hcd = hcd->shared_hcd ? hcd->shared_hcd : hcd;
+	if (!usb3_hcd->self.root_hub)
+		return QCOM_USB_SS_ABSENT;
+
+	udev = usb_hub_find_child(usb3_hcd->self.root_hub, port_index + 1);
+#else
+	udev = NULL;
+#endif
+	if (!udev)
+		return QCOM_USB_SS_ABSENT;
+
+	return udev->speed >= USB_SPEED_SUPER ? QCOM_USB_SS_PRESENT : QCOM_USB_SS_ABSENT;
+}
+
 static void dwc3_qcom_enable_wakeup_irq(int irq, unsigned int polarity)
 {
 	if (!irq)
@@ -359,8 +389,12 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
 	 * freezable workqueue.
 	 */
 	if (dwc3_qcom_is_host(qcom) && wakeup) {
-		for (i = 0; i < qcom->num_ports; i++)
+		for (i = 0; i < qcom->num_ports; i++) {
 			qcom->ports[i].usb2_speed = dwc3_qcom_read_usb2_speed(qcom, i);
+			qcom->ports[i].ss_state = dwc3_qcom_read_ss_state(qcom, i);
+			phy_set_mode_ext(qcom->dwc.usb3_generic_phy[i], PHY_MODE_USB_HOST,
+					 qcom->ports[i].ss_state);
+		}
 		dwc3_qcom_enable_interrupts(qcom);
 	}
 
-- 
2.43.0

Re: [PATCH 3/3] usb: dwc3: qcom: pass live SS-link state into PHY at suspend entry

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] usb: dwc3: qcom: phy_set_mode_ext() called after PHY runtime
  suspend
- [Medium] usb: dwc3: qcom: dwc3_qcom_read_ss_state() broken when
  CONFIG_USB=m
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [High] usb: dwc3: qcom: NULL pointer dereference in
  dwc3_qcom_read_usb2_speed() [1]
--

commit 98eee22a6981ce3ef452cc8f9ec999a2bf823513
Author: Tapas Rana <tapas.rana@oss.qualcomm.com>

usb: dwc3: qcom: pass live SS-link state into PHY at suspend entry

This commit adds dwc3_qcom_read_ss_state() to check whether a SuperSpeed
device is attached on each port's USB3 roothub and passes that state to
phy_set_mode_ext() during dwc3_qcom_suspend().

> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index ac68b4218b56b..d4ba095e47eb0 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
[ ... ]
> @@ -251,6 +253,34 @@ static enum usb_device_speed dwc3_qcom_read_usb2_speed(struct dwc3_qcom *qcom, i
>  	return udev->speed;
>  }
>  
> +static enum qcom_usb_ss_state dwc3_qcom_read_ss_state(struct dwc3_qcom *qcom, int port_index)
> +{
> +	struct usb_device *udev;
> +	struct usb_hcd __maybe_unused *hcd, *usb3_hcd;
> +	struct dwc3 *dwc = &qcom->dwc;
> +
> +	/*
> +	 * FIXME: Fix this layering violation.
> +	 */
> +	hcd = platform_get_drvdata(dwc->xhci);
> +	if (!hcd)
> +		return QCOM_USB_SS_ABSENT;

[Severity: High]
This problem wasn't introduced by this patch, but can
dwc3_qcom_read_usb2_speed() hit a NULL pointer dereference on hcd
[1] if the xHCI driver fails to probe?

dwc3_host_init() assigns dwc->xhci before platform_device_add(), and
platform_device_add() succeeds even if driver binding fails, leaving
dwc->xhci non-NULL with NULL drvdata.  During suspend,
dwc3_qcom_is_host() sees dwc->xhci and calls dwc3_qcom_read_usb2_speed()
right before dwc3_qcom_read_ss_state(), which dereferences
hcd->self.root_hub without the !hcd check added here:

drivers/usb/dwc3/dwc3-qcom.c:dwc3_qcom_read_usb2_speed() {
	...
	hcd = platform_get_drvdata(dwc->xhci);

#ifdef CONFIG_USB
	udev = usb_hub_find_child(hcd->self.root_hub, port_index + 1);
	...
}

> +
> +#ifdef CONFIG_USB
> +	usb3_hcd = hcd->shared_hcd ? hcd->shared_hcd : hcd;
> +	if (!usb3_hcd->self.root_hub)
> +		return QCOM_USB_SS_ABSENT;
> +
> +	udev = usb_hub_find_child(usb3_hcd->self.root_hub, port_index + 1);
> +#else
> +	udev = NULL;
> +#endif
> +	if (!udev)
> +		return QCOM_USB_SS_ABSENT;

[Severity: Medium]
Does #ifdef CONFIG_USB compile out the roothub lookup when CONFIG_USB=m?
Because USB is a tristate Kconfig symbol, modular builds define
CONFIG_USB_MODULE rather than CONFIG_USB, so dwc3_qcom_read_ss_state()
always takes the #else branch and returns QCOM_USB_SS_ABSENT even when a
SuperSpeed device is attached.  Should this use IS_REACHABLE(CONFIG_USB)
or IS_ENABLED(CONFIG_USB)?

[ ... ]
> @@ -359,8 +389,12 @@ static int dwc3_qcom_suspend(struct dwc3_qcom *qcom, bool wakeup)
>  	 * freezable workqueue.
>  	 */
>  	if (dwc3_qcom_is_host(qcom) && wakeup) {
> -		for (i = 0; i < qcom->num_ports; i++)
> +		for (i = 0; i < qcom->num_ports; i++) {
>  			qcom->ports[i].usb2_speed = dwc3_qcom_read_usb2_speed(qcom, i);
> +			qcom->ports[i].ss_state = dwc3_qcom_read_ss_state(qcom, i);
> +			phy_set_mode_ext(qcom->dwc.usb3_generic_phy[i], PHY_MODE_USB_HOST,
> +					 qcom->ports[i].ss_state);
> +		}
>  		dwc3_qcom_enable_interrupts(qcom);
>  	}

[Severity: High]
Isn't phy_set_mode_ext() called here after the USB3 PHY has already
entered runtime suspend?

Both dwc3_qcom_pm_suspend() and dwc3_qcom_runtime_suspend() call
dwc3_pm_suspend() or dwc3_runtime_suspend() before dwc3_qcom_suspend():

drivers/usb/dwc3/dwc3-qcom.c:dwc3_qcom_runtime_suspend() {
	...
	ret = dwc3_runtime_suspend(&qcom->dwc);
	if (ret)
		return ret;

	return dwc3_qcom_suspend(qcom, true);
}

Inside dwc3_suspend_common(), phy_pm_runtime_put_sync() drops the
runtime PM reference on dwc->usb3_generic_phy[i], which schedules
qmp_combo_runtime_suspend() on the parent QMP combo PHY device while
dwc3_qcom_suspend() sleeps in clk_bulk_disable_unprepare() and
dwc3_qcom_interconnect_disable().

Because qmp_combo_usb_set_mode() only records qmp->ss_state in memory and
QPHY_PCS_AUTONOMOUS_MODE_CTRL is programmed in
qmp_combo_runtime_suspend() -> qmp_combo_enable_autonomous_mode() before
the PHY clocks are disabled, updating ss_state here happens after the
PHY has already programmed autonomous mode with QCOM_USB_SS_UNKNOWN (or
the previous cycle's state).

[1] https://sashiko.dev/#/bug/linux-14f73483-dc6c-4250-802a-9e0e3203cc91

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106082716.128968-1-tapas.rana@oss.qualcomm.com?part=3

Re: [PATCH 0/3] phy: qcom: carry live SS-link state without growing enum phy_mode

[Konrad Dybcio]

On 11/6/26 9:27 AM, Tapas Rana wrote:
> QMP PHY drivers always enable receiver detection (ARCVR_DTCT_EN) in
> autonomous mode, and additionally enable LFPS detection (ALFPS_DTCT_EN)
> when a SuperSpeed link is present; otherwise they fall back to
> receiver-detect event selection (ARCVR_DTCT_EVENT_SEL). This choice is
> made by comparing phy_mode against
> PHY_MODE_USB_HOST_SS/PHY_MODE_USB_DEVICE_SS. However, dwc3 core only
> calls phy_set_mode()/phy_set_mode_ext() with the base
> PHY_MODE_USB_HOST or PHY_MODE_USB_DEVICE, never the _SS/_HS/_FS/_LS
> submodes. 

Without much more insight, this sounds like the place to fix it then

Konrad