* [PATCH 0/3] phy: qcom: carry live SS-link state without growing enum phy_mode
@ 2026-11-06 8:27 Tapas Rana
2026-11-06 8:27 ` [PATCH 1/3] phy: qcom-qmp-usb-ss: add shared SS-link-state submode contract Tapas Rana
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Tapas Rana @ 2026-11-06 8:27 UTC (permalink / raw)
To: Vinod Koul, Thinh Nguyen, Greg Kroah-Hartman
Cc: Neil Armstrong, Manivannan Sadhasivam, Dmitry Baryshkov,
Abel Vesa, Konrad Dybcio, Wesley Cheng, Esteban Urrutia,
Loic Poulain, Krishna Kurapati, Ronak Raheja, Faisal Hassan,
Krishna Chaitanya Chundru, prashanth.k, linux-arm-msm, linux-phy,
linux-usb, linux-kernel, 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
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/3] phy: qcom-qmp-usb-ss: add shared SS-link-state submode contract
2026-11-06 8:27 [PATCH 0/3] phy: qcom: carry live SS-link state without growing enum phy_mode Tapas Rana
@ 2026-11-06 8:27 ` Tapas Rana
2026-11-06 8:42 ` sashiko-bot
2026-11-06 8:27 ` [PATCH 2/3] phy: qcom: qmp-combo: Select LFPS detect only when SS link is confirmed present Tapas Rana
2026-11-06 8:27 ` [PATCH 3/3] usb: dwc3: qcom: pass live SS-link state into PHY at suspend entry Tapas Rana
2 siblings, 1 reply; 7+ messages in thread
From: Tapas Rana @ 2026-11-06 8:27 UTC (permalink / raw)
To: Vinod Koul, Thinh Nguyen, Greg Kroah-Hartman
Cc: Neil Armstrong, Manivannan Sadhasivam, Dmitry Baryshkov,
Abel Vesa, Konrad Dybcio, Wesley Cheng, Esteban Urrutia,
Loic Poulain, Krishna Kurapati, Ronak Raheja, Faisal Hassan,
Krishna Chaitanya Chundru, prashanth.k, linux-arm-msm, linux-phy,
linux-usb, linux-kernel, 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 2/3] phy: qcom: qmp-combo: Select LFPS detect only when SS link is confirmed present
2026-11-06 8:27 [PATCH 0/3] phy: qcom: carry live SS-link state without growing enum phy_mode Tapas Rana
2026-11-06 8:27 ` [PATCH 1/3] phy: qcom-qmp-usb-ss: add shared SS-link-state submode contract Tapas Rana
@ 2026-11-06 8:27 ` Tapas Rana
2026-11-06 8:35 ` sashiko-bot
2026-11-06 8:27 ` [PATCH 3/3] usb: dwc3: qcom: pass live SS-link state into PHY at suspend entry Tapas Rana
2 siblings, 1 reply; 7+ messages in thread
From: Tapas Rana @ 2026-11-06 8:27 UTC (permalink / raw)
To: Vinod Koul, Thinh Nguyen, Greg Kroah-Hartman
Cc: Neil Armstrong, Manivannan Sadhasivam, Dmitry Baryshkov,
Abel Vesa, Konrad Dybcio, Wesley Cheng, Esteban Urrutia,
Loic Poulain, Krishna Kurapati, Ronak Raheja, Faisal Hassan,
Krishna Chaitanya Chundru, prashanth.k, linux-arm-msm, linux-phy,
linux-usb, linux-kernel, 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH 3/3] usb: dwc3: qcom: pass live SS-link state into PHY at suspend entry
2026-11-06 8:27 [PATCH 0/3] phy: qcom: carry live SS-link state without growing enum phy_mode Tapas Rana
2026-11-06 8:27 ` [PATCH 1/3] phy: qcom-qmp-usb-ss: add shared SS-link-state submode contract Tapas Rana
2026-11-06 8:27 ` [PATCH 2/3] phy: qcom: qmp-combo: Select LFPS detect only when SS link is confirmed present Tapas Rana
@ 2026-11-06 8:27 ` Tapas Rana
2026-11-06 8:48 ` sashiko-bot
2 siblings, 1 reply; 7+ messages in thread
From: Tapas Rana @ 2026-11-06 8:27 UTC (permalink / raw)
To: Vinod Koul, Thinh Nguyen, Greg Kroah-Hartman
Cc: Neil Armstrong, Manivannan Sadhasivam, Dmitry Baryshkov,
Abel Vesa, Konrad Dybcio, Wesley Cheng, Esteban Urrutia,
Loic Poulain, Krishna Kurapati, Ronak Raheja, Faisal Hassan,
Krishna Chaitanya Chundru, prashanth.k, linux-arm-msm, linux-phy,
linux-usb, linux-kernel, 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/3] phy: qcom: qmp-combo: Select LFPS detect only when SS link is confirmed present
2026-11-06 8:27 ` [PATCH 2/3] phy: qcom: qmp-combo: Select LFPS detect only when SS link is confirmed present Tapas Rana
@ 2026-11-06 8:35 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-11-06 8:35 UTC (permalink / raw)
To: Tapas Rana; +Cc: linux-phy, linux-usb, vkoul, neil.armstrong, olteanv
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/3] phy: qcom-qmp-usb-ss: add shared SS-link-state submode contract
2026-11-06 8:27 ` [PATCH 1/3] phy: qcom-qmp-usb-ss: add shared SS-link-state submode contract Tapas Rana
@ 2026-11-06 8:42 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-11-06 8:42 UTC (permalink / raw)
To: Tapas Rana; +Cc: linux-phy, linux-usb, vkoul, neil.armstrong, olteanv
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
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 3/3] usb: dwc3: qcom: pass live SS-link state into PHY at suspend entry
2026-11-06 8:27 ` [PATCH 3/3] usb: dwc3: qcom: pass live SS-link state into PHY at suspend entry Tapas Rana
@ 2026-11-06 8:48 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-11-06 8:48 UTC (permalink / raw)
To: Tapas Rana; +Cc: linux-phy, linux-usb, vkoul, neil.armstrong, olteanv
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
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-11-06 8:48 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-11-06 8:27 [PATCH 0/3] phy: qcom: carry live SS-link state without growing enum phy_mode Tapas Rana
2026-11-06 8:27 ` [PATCH 1/3] phy: qcom-qmp-usb-ss: add shared SS-link-state submode contract Tapas Rana
2026-11-06 8:42 ` sashiko-bot
2026-11-06 8:27 ` [PATCH 2/3] phy: qcom: qmp-combo: Select LFPS detect only when SS link is confirmed present Tapas Rana
2026-11-06 8:35 ` sashiko-bot
2026-11-06 8:27 ` [PATCH 3/3] usb: dwc3: qcom: pass live SS-link state into PHY at suspend entry Tapas Rana
2026-11-06 8:48 ` sashiko-bot
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox