* [PATCH v3 1/5] dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4
2026-07-30 14:19 [PATCH v3 0/5] USB4 mode programming for QMMPHY on X1E Konrad Dybcio
@ 2026-07-30 14:19 ` Konrad Dybcio
2026-07-30 14:33 ` sashiko-bot
2026-07-30 14:19 ` [PATCH v3 2/5] phy: core: Define TBT phy_mode Konrad Dybcio
` (3 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Konrad Dybcio @ 2026-07-30 14:19 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-kernel, linux-phy, linux-arm-msm, devicetree, usb4-upstream,
Raghavendra Thoorpu, Mika Westerberg, Sven Peter, Konrad Dybcio,
Krzysztof Kozlowski
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Some instances of the QMP combo PHY (called USB43DP) feature a third
functional sub-block, responsible for USB4/Thunderbolt 3 communication.
Compared to the today's state of the binding, one more clock (P2RR2P -
PHY-to-Router, Router-to-PHY) needs to be enabled for the PHY to be
able to switch to USB4 mode. Allow that for X1E.
Also, add a bindings define to let consumers access it.
Assisted-by: Claude:sonnet-5
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@oss.qualcomm.com>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
.../phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml | 55 +++++++++++++++++-----
include/dt-bindings/phy/phy-qcom-qmp.h | 1 +
2 files changed, 45 insertions(+), 11 deletions(-)
diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
index 4eff92343ce4..18930eb96bdf 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
@@ -51,12 +51,7 @@ properties:
clock-names:
minItems: 4
- items:
- - const: aux
- - const: ref
- - const: com_aux
- - const: usb3_pipe
- - const: cfg_ahb
+ maxItems: 5
power-domains:
maxItems: 1
@@ -187,21 +182,59 @@ allOf:
- if:
properties:
compatible:
- enum:
- - qcom,sc7180-qmp-usb3-dp-phy
- - qcom,sdm845-qmp-usb3-dp-phy
+ contains:
+ enum:
+ - qcom,x1e80100-qmp-usb3-dp-phy
then:
properties:
clocks:
maxItems: 5
clock-names:
+ items:
+ - const: aux
+ - const: ref
+ - const: com_aux
+ - const: usb3_pipe
+ - const: p2rr2p_pipe
+
+ - if:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,sc7180-qmp-usb3-dp-phy
+ - qcom,sdm845-qmp-usb3-dp-phy
+ then:
+ properties:
+ clocks:
maxItems: 5
- else:
+ clock-names:
+ items:
+ - const: aux
+ - const: ref
+ - const: com_aux
+ - const: usb3_pipe
+ - const: cfg_ahb
+
+ - if:
+ not:
+ properties:
+ compatible:
+ contains:
+ enum:
+ - qcom,x1e80100-qmp-usb3-dp-phy
+ - qcom,sc7180-qmp-usb3-dp-phy
+ - qcom,sdm845-qmp-usb3-dp-phy
+ then:
properties:
clocks:
maxItems: 4
clock-names:
- maxItems: 4
+ items:
+ - const: aux
+ - const: ref
+ - const: com_aux
+ - const: usb3_pipe
- if:
properties:
diff --git a/include/dt-bindings/phy/phy-qcom-qmp.h b/include/dt-bindings/phy/phy-qcom-qmp.h
index 6b43ea9e0051..1c3ce0c02b0c 100644
--- a/include/dt-bindings/phy/phy-qcom-qmp.h
+++ b/include/dt-bindings/phy/phy-qcom-qmp.h
@@ -16,6 +16,7 @@
/* QMP USB4-USB3-DP PHYs */
#define QMP_USB43DP_USB3_PHY 0
#define QMP_USB43DP_DP_PHY 1
+#define QMP_USB43DP_USB4_PHY 2
/* QMP PCIE PHYs */
#define QMP_PCIE_PIPE_CLK 0
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 1/5] dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4
2026-07-30 14:19 ` [PATCH v3 1/5] dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4 Konrad Dybcio
@ 2026-07-30 14:33 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-30 14:33 UTC (permalink / raw)
To: Konrad Dybcio
Cc: olteanv, vkoul, linux-phy, devicetree, conor+dt, robh,
neil.armstrong
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The `clock-names` definition for `qcom,x1e80100-qmp-usb3-dp-phy` implicitly makes the 5th clock mandatory, breaking validation for existing device trees.
- [Medium] The `p2rr2p_pipe` clock is artificially restricted to the X1E SoC, preventing other USB4-capable SoCs in the same binding from modeling their hardware correctly.
--
--- Patch [1]: [PATCH v3 1/5] dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4 ---
commit 86283e120a292e73e741d8fc0d233382e691770a
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4
This commit extends the X1E USB3/DP PHY bindings to support USB4 mode by
allowing an additional clock (P2RR2P). It adds conditionals to handle the
differences in clock requirements between earlier generations and X1E.
> diff --git a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> index 4eff92343ce4f..18930eb96bdfe 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
[ ... ]
> @@ -187,21 +182,59 @@ allOf:
> - if:
> properties:
> compatible:
> - enum:
> - - qcom,sc7180-qmp-usb3-dp-phy
> - - qcom,sdm845-qmp-usb3-dp-phy
> + contains:
> + enum:
> + - qcom,x1e80100-qmp-usb3-dp-phy
> then:
> properties:
> clocks:
> maxItems: 5
> clock-names:
> + items:
> + - const: aux
> + - const: ref
> + - const: com_aux
> + - const: usb3_pipe
> + - const: p2rr2p_pipe
[Severity: High]
Will this break device tree validation for existing x1e80100 device trees
that only provide 4 clocks? Since minItems is not explicitly set in this
local subschema, dt-schema will automatically set minItems to 5 to match
the length of the items array. Should minItems: 4 be added to this block to
preserve backward compatibility?
> +
> + - if:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,sc7180-qmp-usb3-dp-phy
> + - qcom,sdm845-qmp-usb3-dp-phy
> + then:
> + properties:
> + clocks:
> maxItems: 5
> - else:
> + clock-names:
> + items:
> + - const: aux
> + - const: ref
> + - const: com_aux
> + - const: usb3_pipe
> + - const: cfg_ahb
> +
> + - if:
> + not:
> + properties:
> + compatible:
> + contains:
> + enum:
> + - qcom,x1e80100-qmp-usb3-dp-phy
> + - qcom,sc7180-qmp-usb3-dp-phy
> + - qcom,sdm845-qmp-usb3-dp-phy
[Severity: Medium]
Does this artificial restriction to maxItems: 4 for all other compatibles
prevent other USB4-capable SoCs (like sc8280xp) from correctly modeling
their hardware?
If they share the same USB43DP hardware capabilities and available clock
lines, they will fail validation if a developer attempts to add the
p2rr2p_pipe clock to them in the future.
Could this be structured to allow the 5th clock on other identical hardware
variants?
> + then:
> properties:
> clocks:
> maxItems: 4
> clock-names:
> - maxItems: 4
> + items:
> + - const: aux
> + - const: ref
> + - const: com_aux
> + - const: usb3_pipe
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-topic-usb4phy-v3-0-ccf0f7ea98b5@oss.qualcomm.com?part=1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 2/5] phy: core: Define TBT phy_mode
2026-07-30 14:19 [PATCH v3 0/5] USB4 mode programming for QMMPHY on X1E Konrad Dybcio
2026-07-30 14:19 ` [PATCH v3 1/5] dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4 Konrad Dybcio
@ 2026-07-30 14:19 ` Konrad Dybcio
2026-07-30 14:19 ` [PATCH v3 3/5] phy: qualcomm: qmp-combo: Add preliminary USB4 support Konrad Dybcio
` (2 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Konrad Dybcio @ 2026-07-30 14:19 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-kernel, linux-phy, linux-arm-msm, devicetree, usb4-upstream,
Raghavendra Thoorpu, Mika Westerberg, Sven Peter, Konrad Dybcio,
Dmitry Baryshkov
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
There exist OS-accessible USB4 and Thunderbolt PHYs that need specific
configuration. Define a new phy_mode for them.
Currently, USB4 and TBT3 are defined as submodes, because they're quite
distinct, most visibly in the electrical/analog aspects (slightly
different frequencies, timings, etc.). This results in a need to make
the PHY aware of the actual mode needed (at least in the Qualcomm
implementation, but I believe that'll be a general need).
Newer versions of TBT are basically supersets of USB4 with higher
host-side requirements, so these are not defined. This can always be
changed as necessary.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
include/linux/phy/phy-tbt.h | 14 ++++++++++++++
include/linux/phy/phy.h | 2 ++
2 files changed, 16 insertions(+)
diff --git a/include/linux/phy/phy-tbt.h b/include/linux/phy/phy-tbt.h
new file mode 100644
index 000000000000..5f48059814e1
--- /dev/null
+++ b/include/linux/phy/phy-tbt.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ */
+
+#ifndef __PHY_TBT_H_
+#define __PHY_TBT_H_
+
+enum tbt_phy_submode {
+ PHY_SUBMODE_TBT3,
+ PHY_SUBMODE_USB4,
+};
+
+#endif
diff --git a/include/linux/phy/phy.h b/include/linux/phy/phy.h
index ea47975e288a..26a91c070f8c 100644
--- a/include/linux/phy/phy.h
+++ b/include/linux/phy/phy.h
@@ -20,6 +20,7 @@
#include <linux/phy/phy-hdmi.h>
#include <linux/phy/phy-lvds.h>
#include <linux/phy/phy-mipi-dphy.h>
+#include <linux/phy/phy-tbt.h>
struct phy;
@@ -45,6 +46,7 @@ enum phy_mode {
PHY_MODE_LVDS,
PHY_MODE_DP,
PHY_MODE_HDMI,
+ PHY_MODE_TBT,
};
enum phy_media {
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH v3 3/5] phy: qualcomm: qmp-combo: Add preliminary USB4 support
2026-07-30 14:19 [PATCH v3 0/5] USB4 mode programming for QMMPHY on X1E Konrad Dybcio
2026-07-30 14:19 ` [PATCH v3 1/5] dt-bindings: phy: qcom,qmp-usb3-dp: Extend X1E description for USB4 Konrad Dybcio
2026-07-30 14:19 ` [PATCH v3 2/5] phy: core: Define TBT phy_mode Konrad Dybcio
@ 2026-07-30 14:19 ` Konrad Dybcio
2026-07-30 14:38 ` sashiko-bot
2026-07-30 14:19 ` [PATCH v3 4/5] phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa Konrad Dybcio
2026-07-30 14:19 ` [PATCH v3 5/5] arm64: dts: qcom: hamoa: Extend QMPPHY description for USB4 Konrad Dybcio
4 siblings, 1 reply; 10+ messages in thread
From: Konrad Dybcio @ 2026-07-30 14:19 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-kernel, linux-phy, linux-arm-msm, devicetree, usb4-upstream,
Raghavendra Thoorpu, Mika Westerberg, Sven Peter, Konrad Dybcio
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Some Combo PHYs (so far only on SC8280XP, X1E80100 and Glymur), come in
a flavor called USB43DP, which as the name implies, features USB4, USB3
and DP signal processing capabilities. In that architecture, USB3 and
USB4 PHYs share the same USB_PLL while featuring separate logic spaces.
The DP part is roughly the same as on the instances without USB4.
The USB4 and USB3/DP operation modes of the PHY are mutually exclusive.
Only one USB protocol (and flavor of pipe clock) can be active at a
given moment (not to be confused with USB3 not being able to be
tunneled as USB4 packets - that of course remains possible).
The DP PLL is still used for clocking tunneled DP links. It may be
turned off to save power when no tunnels are active, but that's left as
a TODO item for now.
Due to the nature of USB4, the Type-C handling happens entirely inside
the Host Router, and as such the QMPPHY's mux_set() function is
nullified for the period when USB4 PHY remains active. This is strictly
necessary, as the Host Router driver is going to excercise manual
control over the USB4 PHY's power state, which is needed by the suspend
and resume flows. Failure to control that synchronously with other
parts of the code results in a SoC crash by unlocked access.
Because of that, a new struct phy is spawned to expose the USB4 mode,
along with a .set_mode callback to allow toggling between USB4 and TBT3
submodes.
Thunderbolt 3, having a number of differences vs USB4, requires a
couple specific overrides, pertaining to electrical characteristics,
which are easily accommodated for.
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 392 ++++++++++++++++++++++++------
1 file changed, 322 insertions(+), 70 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index cdcfad2e86b1..b80eac9d7140 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -22,6 +22,7 @@
#include <linux/usb/typec.h>
#include <linux/usb/typec_dp.h>
#include <linux/usb/typec_mux.h>
+#include <linux/usb/typec_tbt.h>
#include <drm/bridge/aux-bridge.h>
@@ -61,10 +62,14 @@
#define SW_USB3PHY_RESET BIT(2)
/* mux to select USB3 PHY reset control, 0:HW control, 1: software reset */
#define SW_USB3PHY_RESET_MUX BIT(3)
+#define SW_USB4PHY_RESET BIT(4)
+#define SW_USB4PHY_RESET_MUX BIT(5)
/* QPHY_V3_DP_COM_PHY_MODE_CTRL register bits */
#define USB3_MODE BIT(0) /* enables USB3 mode */
#define DP_MODE BIT(1) /* enables DP mode */
+#define USB4_MODE BIT(2) /* mutually exclusive with the above */
+#define DP_TUNNELING_CLOCK_GEN_EN BIT(3)
/* QPHY_V3_DP_COM_TYPEC_CTRL register bits */
#define SW_PORTSELECT_VAL BIT(0)
@@ -77,6 +82,8 @@ enum qmpphy_mode {
QMPPHY_MODE_USB3DP = 0,
QMPPHY_MODE_DP_ONLY,
QMPPHY_MODE_USB3_ONLY,
+ /* USB4 QMPPHY mode refers to both USB4 and TBT3 */
+ QMPPHY_MODE_USB4,
};
/* set of registers with offsets different per-PHY */
@@ -89,6 +96,7 @@ enum qphy_reg_layout {
QPHY_PCS_LFPS_RXTERM_IRQ_CLEAR,
QPHY_PCS_POWER_DOWN_CONTROL,
QPHY_PCS_CLAMP_ENABLE,
+ QPHY_PCS_USB4_CLAMP_ENABLE,
QPHY_COM_RESETSM_CNTRL,
QPHY_COM_C_READY_STATUS,
@@ -2198,6 +2206,8 @@ struct qmp_combo_offsets {
u16 dp_txa;
u16 dp_txb;
u16 dp_dp_phy;
+ u16 usb4_serdes;
+ u16 usb4_pcs;
};
struct qmp_phy_cfg {
@@ -2245,6 +2255,18 @@ struct qmp_phy_cfg {
int (*calibrate_dp_phy)(struct qmp_combo *qmp);
void (*dp_aux_init)(struct qmp_combo *qmp);
+ /* USB4 specifics */
+ const struct qmp_phy_init_tbl *usb4_serdes_tbl;
+ int usb4_serdes_tbl_num;
+ const struct qmp_phy_init_tbl *usb4_serdes_tb3_ovrd_tbl;
+ int usb4_serdes_tb3_ovrd_num;
+ const struct qmp_phy_init_tbl *usb4_tx_tbl;
+ int usb4_tx_tbl_num;
+ const struct qmp_phy_init_tbl *usb4_rx_tbl;
+ int usb4_rx_tbl_num;
+ const struct qmp_phy_init_tbl *usb4_pcs_tbl;
+ int usb4_pcs_tbl_num;
+
/* resets to be requested */
const char * const *reset_list;
int num_resets;
@@ -2286,8 +2308,12 @@ struct qmp_combo {
void __iomem *dp_tx2;
void __iomem *dp_dp_phy;
+ void __iomem *usb4_serdes;
+ void __iomem *usb4_pcs;
+
struct clk *pipe_clk;
struct clk_bulk_data *clks;
+ struct clk *p2rr2p_pipe_clk;
int num_clks;
struct reset_control_bulk_data *resets;
struct regulator_bulk_data *vregs;
@@ -2306,6 +2332,10 @@ struct qmp_combo {
unsigned int dp_init_count;
bool dp_powered_on;
+ struct phy *usb4_phy;
+ enum tbt_phy_submode usb4_phy_submode;
+ unsigned int usb4phy_init_count;
+
struct clk_fixed_rate pipe_clk_fixed;
struct clk_hw dp_link_hw;
struct clk_hw dp_pixel_hw;
@@ -3661,14 +3691,26 @@ static int qmp_combo_dp_calibrate(struct phy *phy)
static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
{
const struct qmp_phy_cfg *cfg = qmp->cfg;
+ u32 dp_reset_val = SW_DPPHY_RESET_MUX | SW_DPPHY_RESET;
void __iomem *com = qmp->com;
void __iomem *pcs_aon = qmp->pcs_aon;
+ void __iomem *pcs = qmp->pcs;
int ret;
u32 val;
if (!force && qmp->init_count++)
return 0;
+ if (qmp->qmpphy_mode == QMPPHY_MODE_USB4) {
+ pcs = qmp->usb4_pcs;
+
+ qphy_setbits(pcs_aon, cfg->regs[QPHY_PCS_USB4_CLAMP_ENABLE], CLAMP_EN);
+
+ /* Do not disturb the DP PLL in case there's an active DP tunnel */
+ if (readl(com + QPHY_V3_DP_COM_RESET_OVRD_CTRL) & DP_TUNNELING_CLOCK_GEN_EN)
+ dp_reset_val &= ~SW_DPPHY_RESET_MUX;
+ }
+
ret = regulator_bulk_enable(cfg->num_vregs, qmp->vregs);
if (ret) {
dev_err(qmp->dev, "failed to enable regulators, err=%d\n", ret);
@@ -3702,8 +3744,9 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
/* override hardware control for reset of qmp phy */
qphy_setbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
- SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
- SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
+ dp_reset_val |
+ SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET |
+ SW_USB4PHY_RESET_MUX | SW_USB4PHY_RESET);
/* override hardware control for reset of qmp phy */
if (pcs_aon && cfg->regs[QPHY_AON_TOGGLE_ENABLE]) {
@@ -3719,6 +3762,10 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
if (cfg->invert_cc_polarity)
val |= INVERT_CC_POLARITY;
+ /*
+ * Note that in USB4 mode, the router controls pin assignments instead
+ * and the values of the PORTSELECT registers are ignored.
+ */
writel(val, com + QPHY_V3_DP_COM_TYPEC_CTRL);
switch (qmp->qmpphy_mode) {
@@ -3746,12 +3793,21 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
break;
+ case QMPPHY_MODE_USB4:
+ writel(USB4_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
+
+ /* bring both QMP USB and QMP DP PHYs PCS block out of reset */
+ /* TODO: disable DP PLL if there are no active tunnels after router setup */
+ qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
+ SW_USB4PHY_RESET_MUX | SW_USB4PHY_RESET |
+ SW_DPPHY_RESET_MUX | SW_DPPHY_RESET);
+ break;
}
qphy_clrbits(com, QPHY_V3_DP_COM_SWI_CTRL, 0x03);
qphy_clrbits(com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);
- qphy_setbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
+ qphy_setbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
SW_PWRDN);
return 0;
@@ -3869,6 +3925,10 @@ static int qmp_combo_usb_power_on(struct phy *phy)
struct qmp_combo *qmp = phy_get_drvdata(phy);
const struct qmp_phy_cfg *cfg = qmp->cfg;
void __iomem *serdes = qmp->serdes;
+ const struct qmp_phy_init_tbl *pcs_tbl;
+ const struct qmp_phy_init_tbl *rx_tbl;
+ const struct qmp_phy_init_tbl *serdes_tbl;
+ const struct qmp_phy_init_tbl *tx_tbl;
void __iomem *tx = qmp->tx;
void __iomem *rx = qmp->rx;
void __iomem *tx2 = qmp->tx2;
@@ -3876,23 +3936,54 @@ static int qmp_combo_usb_power_on(struct phy *phy)
void __iomem *pcs = qmp->pcs;
void __iomem *pcs_usb = qmp->pcs_usb;
void __iomem *status;
+ int pcs_tbl_num, rx_tbl_num, serdes_tbl_num, tx_tbl_num;
unsigned int val;
int ret;
- qmp_configure(qmp->dev, serdes, cfg->serdes_tbl, cfg->serdes_tbl_num);
+ if (qmp->qmpphy_mode == QMPPHY_MODE_USB4) {
+ pcs = qmp->usb4_pcs;
+ serdes = qmp->usb4_serdes;
+ pcs_tbl = cfg->usb4_pcs_tbl;
+ pcs_tbl_num = cfg->usb4_pcs_tbl_num;
+
+ serdes_tbl = cfg->usb4_serdes_tbl;
+ serdes_tbl_num = cfg->usb4_serdes_tbl_num;
+
+ rx_tbl = cfg->usb4_rx_tbl;
+ rx_tbl_num = cfg->usb4_rx_tbl_num;
+
+ tx_tbl = cfg->usb4_tx_tbl;
+ tx_tbl_num = cfg->usb4_tx_tbl_num;
+ } else {
+ pcs_tbl = cfg->pcs_tbl;
+ pcs_tbl_num = cfg->pcs_tbl_num;
+
+ serdes_tbl = cfg->serdes_tbl;
+ serdes_tbl_num = cfg->serdes_tbl_num;
+
+ rx_tbl = cfg->rx_tbl;
+ rx_tbl_num = cfg->rx_tbl_num;
+
+ tx_tbl = cfg->tx_tbl;
+ tx_tbl_num = cfg->tx_tbl_num;
+ }
+
+ qmp_configure(qmp->dev, serdes, serdes_tbl, serdes_tbl_num);
/* Tx, Rx, and PCS configurations */
- qmp_configure_lane(qmp->dev, tx, cfg->tx_tbl, cfg->tx_tbl_num, 1);
- qmp_configure_lane(qmp->dev, tx2, cfg->tx_tbl, cfg->tx_tbl_num, 2);
+ qmp_configure_lane(qmp->dev, tx, tx_tbl, tx_tbl_num, 1);
+ qmp_configure_lane(qmp->dev, tx2, tx_tbl, tx_tbl_num, 2);
- qmp_configure_lane(qmp->dev, rx, cfg->rx_tbl, cfg->rx_tbl_num, 1);
- qmp_configure_lane(qmp->dev, rx2, cfg->rx_tbl, cfg->rx_tbl_num, 2);
+ qmp_configure_lane(qmp->dev, rx, rx_tbl, rx_tbl_num, 1);
+ qmp_configure_lane(qmp->dev, rx2, rx_tbl, rx_tbl_num, 2);
- qmp_configure(qmp->dev, pcs, cfg->pcs_tbl, cfg->pcs_tbl_num);
+ qmp_configure(qmp->dev, pcs, pcs_tbl, pcs_tbl_num);
qmp_configure(qmp->dev, qmp->pcs_misc, cfg->pcs_misc_tbl, cfg->pcs_misc_tbl_num);
-
- if (pcs_usb)
+ if (qmp->qmpphy_mode == QMPPHY_MODE_USB4 && qmp->usb4_phy_submode == PHY_SUBMODE_TBT3)
+ qmp_configure(qmp->dev, serdes, cfg->usb4_serdes_tb3_ovrd_tbl,
+ cfg->usb4_serdes_tb3_ovrd_num);
+ else if (qmp->qmpphy_mode != QMPPHY_MODE_USB4 && pcs_usb)
qmp_configure(qmp->dev, pcs_usb, cfg->pcs_usb_tbl,
cfg->pcs_usb_tbl_num);
@@ -3925,16 +4016,20 @@ static int qmp_combo_usb_power_off(struct phy *phy)
{
struct qmp_combo *qmp = phy_get_drvdata(phy);
const struct qmp_phy_cfg *cfg = qmp->cfg;
+ void __iomem *pcs = qmp->pcs;
+
+ if (qmp->usb4phy_init_count)
+ pcs = qmp->usb4_pcs;
/* PHY reset */
- qphy_setbits(qmp->pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);
+ qphy_setbits(pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);
/* stop SerDes and Phy-Coding-Sublayer */
- qphy_clrbits(qmp->pcs, cfg->regs[QPHY_START_CTRL],
+ qphy_clrbits(pcs, cfg->regs[QPHY_START_CTRL],
SERDES_START | PCS_START);
/* Put PHY into POWER DOWN state: active low */
- qphy_clrbits(qmp->pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
+ qphy_clrbits(pcs, cfg->regs[QPHY_PCS_POWER_DOWN_CONTROL],
SW_PWRDN);
return 0;
@@ -3945,21 +4040,27 @@ static int qmp_combo_usb_init(struct phy *phy)
struct qmp_combo *qmp = phy_get_drvdata(phy);
int ret;
- mutex_lock(&qmp->phy_mutex);
+ guard(mutex)(&qmp->phy_mutex);
+
+ /* USB4 mode takes precedence, do not reprogram the PHY in that case */
+ if (qmp->usb4phy_init_count) {
+ qmp->init_count++;
+ qmp->usb_init_count++;
+ return 0;
+ }
+
ret = qmp_combo_com_init(qmp, false);
if (ret)
- goto out_unlock;
+ return ret;
ret = qmp_combo_usb_power_on(phy);
if (ret) {
qmp_combo_com_exit(qmp, false);
- goto out_unlock;
+ return ret;
}
qmp->usb_init_count++;
-out_unlock:
- mutex_unlock(&qmp->phy_mutex);
return ret;
}
@@ -3968,20 +4069,26 @@ static int qmp_combo_usb_exit(struct phy *phy)
struct qmp_combo *qmp = phy_get_drvdata(phy);
int ret;
- mutex_lock(&qmp->phy_mutex);
+ guard(mutex)(&qmp->phy_mutex);
+
+ /* USB4 mode takes precedence, do not reprogram the PHY in that case */
+ if (qmp->usb4phy_init_count) {
+ qmp->init_count--;
+ qmp->usb_init_count--;
+ return 0;
+ }
+
ret = qmp_combo_usb_power_off(phy);
if (ret)
- goto out_unlock;
+ return ret;
ret = qmp_combo_com_exit(qmp, false);
if (ret)
- goto out_unlock;
+ return ret;
qmp->usb_init_count--;
-out_unlock:
- mutex_unlock(&qmp->phy_mutex);
- return ret;
+ return 0;
}
static int qmp_combo_usb_set_mode(struct phy *phy, enum phy_mode mode, int submode)
@@ -3993,6 +4100,119 @@ static int qmp_combo_usb_set_mode(struct phy *phy, enum phy_mode mode, int submo
return 0;
}
+static int qmp_combo_reconfigure_phy(struct qmp_combo *qmp, enum qmpphy_mode new_mode)
+{
+ dev_dbg(qmp->dev, "qmp_combo_reconfigure_phy: switching from qmpphy mode %d to %d\n",
+ qmp->qmpphy_mode, new_mode);
+
+ if (qmp->usb_init_count || qmp->usb4phy_init_count)
+ qmp_combo_usb_power_off(qmp->usb_phy);
+
+ if (qmp->dp_init_count)
+ writel(DP_PHY_PD_CTL_PSR_PWRDN, qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);
+
+ qmp_combo_com_exit(qmp, true);
+
+ qmp->qmpphy_mode = new_mode;
+
+ /* Now everything's powered down, power up the right PHYs */
+ qmp_combo_com_init(qmp, true);
+
+ if ((qmp->usb_init_count || qmp->usb4phy_init_count) &&
+ new_mode != QMPPHY_MODE_DP_ONLY)
+ qmp_combo_usb_power_on(qmp->usb_phy);
+
+ if ((new_mode == QMPPHY_MODE_USB3DP || new_mode == QMPPHY_MODE_DP_ONLY) &&
+ qmp->dp_init_count)
+ qmp->cfg->dp_aux_init(qmp);
+
+ return 0;
+}
+
+static int qmp_combo_usb4_init(struct phy *phy)
+{
+ struct qmp_combo *qmp = phy_get_drvdata(phy);
+ int ret;
+
+ guard(mutex)(&qmp->phy_mutex);
+
+ if (!qmp->p2rr2p_pipe_clk) {
+ dev_err(qmp->dev, "missing p2rr2p_pipe clock handle. Update your Device Tree.\n");
+ return -EINVAL;
+ }
+
+ ret = clk_prepare_enable(qmp->p2rr2p_pipe_clk);
+ if (ret) {
+ dev_err(qmp->dev, "p2rr2p_pipe enable failed: %d\n", ret);
+ return ret;
+ }
+
+ ret = qmp_combo_com_init(qmp, false);
+ if (ret)
+ return ret;
+
+ /* USB4 mode takes precedence to USB3(+DP), force reconfigure the PHY */
+ ret = qmp_combo_reconfigure_phy(qmp, QMPPHY_MODE_USB4);
+ if (ret)
+ return ret;
+
+ ret = qmp_combo_usb_power_on(phy);
+ if (ret) {
+ qmp_combo_com_exit(qmp, false);
+ return ret;
+ }
+
+ /*
+ * Due to the SoC design, the PHY only has a single valid consumer and
+ * preventing it from having sole ownership of the PHY's power state
+ * makes suspending/resuming the router impossible.
+ */
+ WARN_ON(qmp->usb4phy_init_count++);
+
+ return 0;
+}
+
+static int qmp_combo_usb4_exit(struct phy *phy)
+{
+ struct qmp_combo *qmp = phy_get_drvdata(phy);
+ int ret;
+
+ guard(mutex)(&qmp->phy_mutex);
+
+ ret = qmp_combo_usb_power_off(qmp->usb_phy);
+ if (ret)
+ return ret;
+
+ ret = qmp_combo_com_exit(qmp, false);
+ if (ret)
+ return ret;
+
+ /*
+ * Mark the USB4 PHY uninitialized and wait for a mux_set event to determine the correct
+ * setting. This will always be possible because USB4 requires Type-C.
+ */
+ qmp->usb4phy_init_count--;
+
+ clk_disable_unprepare(qmp->p2rr2p_pipe_clk);
+
+ return 0;
+}
+
+static int qmp_combo_usb4_set_mode(struct phy *phy, enum phy_mode mode, int submode)
+{
+ struct qmp_combo *qmp = phy_get_drvdata(phy);
+
+ if (mode != PHY_MODE_TBT)
+ return -EINVAL;
+
+ if (submode == PHY_SUBMODE_USB4 || submode == PHY_SUBMODE_TBT3) {
+ qmp->usb4_phy_submode = submode;
+ return 0;
+ }
+
+ return -EINVAL;
+}
+
static const struct phy_ops qmp_combo_usb_phy_ops = {
.init = qmp_combo_usb_init,
.exit = qmp_combo_usb_exit,
@@ -4000,6 +4220,13 @@ static const struct phy_ops qmp_combo_usb_phy_ops = {
.owner = THIS_MODULE,
};
+static const struct phy_ops qmp_combo_usb4_phy_ops = {
+ .init = qmp_combo_usb4_init,
+ .exit = qmp_combo_usb4_exit,
+ .set_mode = qmp_combo_usb4_set_mode,
+ .owner = THIS_MODULE,
+};
+
static const struct phy_ops qmp_combo_dp_phy_ops = {
.init = qmp_combo_dp_init,
.configure = qmp_combo_dp_configure,
@@ -4039,9 +4266,12 @@ static void qmp_combo_enable_autonomous_mode(struct qmp_combo *qmp)
* Enable i/o clamp_n for autonomous mode
* V6 and later versions use pcs aon clamp register
*/
- if (pcs_aon)
- qphy_clrbits(pcs_aon, cfg->regs[QPHY_PCS_CLAMP_ENABLE], CLAMP_EN);
- else if (pcs_misc)
+ 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);
}
@@ -4053,9 +4283,12 @@ static void qmp_combo_disable_autonomous_mode(struct qmp_combo *qmp)
void __iomem *pcs_aon = qmp->pcs_aon;
/* Disable i/o clamp_n on resume for normal mode */
- if (pcs_aon)
- qphy_setbits(pcs_aon, cfg->regs[QPHY_PCS_CLAMP_ENABLE], CLAMP_EN);
- else if (pcs_misc)
+ if (pcs_aon) {
+ if (qmp->qmpphy_mode == QMPPHY_MODE_USB4)
+ qphy_setbits(pcs_aon, cfg->regs[QPHY_PCS_USB4_CLAMP_ENABLE], CLAMP_EN);
+ else
+ qphy_setbits(pcs_aon, cfg->regs[QPHY_PCS_CLAMP_ENABLE], CLAMP_EN);
+ } else if (pcs_misc)
qphy_setbits(pcs_misc, cfg->regs[QPHY_PCS_CLAMP_ENABLE], CLAMP_EN);
qphy_clrbits(pcs_usb, cfg->regs[QPHY_PCS_AUTONOMOUS_MODE_CTRL],
@@ -4429,24 +4662,29 @@ static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw,
struct qmp_combo *qmp = typec_switch_get_drvdata(sw);
const struct qmp_phy_cfg *cfg = qmp->cfg;
+ if (qmp->qmpphy_mode == QMPPHY_MODE_USB4) {
+ /* QMPPHY has no orientation handling in USB4 mode, don't cache the setting */
+ qmp->orientation = TYPEC_ORIENTATION_NONE;
+ return 0;
+ }
+
if (orientation == qmp->orientation || orientation == TYPEC_ORIENTATION_NONE)
return 0;
- mutex_lock(&qmp->phy_mutex);
+ guard(mutex)(&qmp->phy_mutex);
qmp->orientation = orientation;
if (qmp->init_count) {
- if (qmp->usb_init_count)
+ if (qmp->usb_init_count || qmp->usb4phy_init_count)
qmp_combo_usb_power_off(qmp->usb_phy);
qmp_combo_com_exit(qmp, true);
qmp_combo_com_init(qmp, true);
- if (qmp->usb_init_count)
+ if (qmp->usb_init_count || qmp->usb4phy_init_count)
qmp_combo_usb_power_on(qmp->usb_phy);
if (qmp->dp_init_count)
cfg->dp_aux_init(qmp);
}
- mutex_unlock(&qmp->phy_mutex);
return 0;
}
@@ -4454,7 +4692,6 @@ static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw,
static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_state *state)
{
struct qmp_combo *qmp = typec_mux_get_drvdata(mux);
- const struct qmp_phy_cfg *cfg = qmp->cfg;
enum qmpphy_mode new_mode;
unsigned int svid;
@@ -4465,6 +4702,29 @@ static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s
else
svid = 0;
+ /*
+ * The USB4 router driver must excercise fine-grained control over the timing of
+ * USB4 QMPPHY mode entry/exit, which is difficult to otherwise ensure within Linux's
+ * Type-C framework if the PHY acts as a self-decisive mode mux in parallel.
+ *
+ * Keep the hardware in QMPPHY_MODE_USB4 at all times between .init and .exit of the
+ * (Linux) USB4 PHY and wait for the router driver to turn it off at its discretion.
+ * Once that happens, fall back to the usual USB3/DP/Combo mode logic.
+ *
+ * After the QMPPHY has been turned off through phy_exit(usb4_phy), the next mux_set
+ * will initialize it in the right mode.
+ */
+ if (qmp->usb4phy_init_count)
+ return 0;
+
+ /*
+ * Explicitly ignore TBT/USB4 mode requests that may come if the USB4 PHY hasn't been
+ * initialized, either due to the USB4 drivers being disabled or due to this PHY instance
+ * lacking USB4 support.
+ */
+ if (svid == USB_TYPEC_TBT_SID || (!state->alt && state->mode == TYPEC_MODE_USB4))
+ return 0;
+
if (svid == USB_TYPEC_DP_SID) {
switch (state->mode) {
/* DP Only */
@@ -4497,41 +4757,11 @@ static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s
return 0;
}
- dev_dbg(qmp->dev, "typec_mux_set: switching from qmpphy mode %d to %d\n",
- qmp->qmpphy_mode, new_mode);
+ /* The mux still receives Type-C events, even if all PHYs are uninitialized */
+ if (!qmp->init_count)
+ return 0;
- qmp->qmpphy_mode = new_mode;
-
- if (qmp->init_count) {
- if (qmp->usb_init_count)
- qmp_combo_usb_power_off(qmp->usb_phy);
-
- if (qmp->dp_init_count)
- writel(DP_PHY_PD_CTL_PSR_PWRDN, qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);
-
- qmp_combo_com_exit(qmp, true);
-
- /* Now everything's powered down, power up the right PHYs */
- qmp_combo_com_init(qmp, true);
-
- if (new_mode == QMPPHY_MODE_DP_ONLY) {
- if (qmp->usb_init_count)
- qmp->usb_init_count--;
- }
-
- if (new_mode == QMPPHY_MODE_USB3DP || new_mode == QMPPHY_MODE_USB3_ONLY) {
- qmp_combo_usb_power_on(qmp->usb_phy);
- if (!qmp->usb_init_count)
- qmp->usb_init_count++;
- }
-
- if (new_mode == QMPPHY_MODE_DP_ONLY || new_mode == QMPPHY_MODE_USB3DP) {
- if (qmp->dp_init_count)
- cfg->dp_aux_init(qmp);
- }
- }
-
- return 0;
+ return qmp_combo_reconfigure_phy(qmp, new_mode);
}
static void qmp_combo_typec_switch_unregister(void *data)
@@ -4735,6 +4965,9 @@ static int qmp_combo_parse_dt(struct qmp_combo *qmp)
}
qmp->dp_dp_phy = base + offs->dp_dp_phy;
+ qmp->usb4_serdes = base + offs->usb4_serdes;
+ qmp->usb4_pcs = base + offs->usb4_pcs;
+
ret = qmp_combo_clk_init(qmp);
if (ret)
return ret;
@@ -4745,6 +4978,12 @@ static int qmp_combo_parse_dt(struct qmp_combo *qmp)
"failed to get usb3_pipe clock\n");
}
+ qmp->p2rr2p_pipe_clk = devm_clk_get_optional(dev, "p2rr2p_pipe");
+ if (IS_ERR(qmp->p2rr2p_pipe_clk)) {
+ return dev_err_probe(dev, PTR_ERR(qmp->p2rr2p_pipe_clk),
+ "failed to get p2rr2p_pipe clock\n");
+ }
+
return 0;
}
@@ -4760,6 +4999,8 @@ static struct phy *qmp_combo_phy_xlate(struct device *dev, const struct of_phand
return qmp->usb_phy;
case QMP_USB43DP_DP_PHY:
return qmp->dp_phy;
+ case QMP_USB43DP_USB4_PHY:
+ return qmp->usb4_phy ?: ERR_PTR(-EINVAL);
}
return ERR_PTR(-EINVAL);
@@ -4967,6 +5208,17 @@ static int qmp_combo_probe(struct platform_device *pdev)
phy_set_drvdata(qmp->dp_phy, qmp);
+ if (qmp->cfg->usb4_serdes_tbl) {
+ qmp->usb4_phy = devm_phy_create(dev, dev->of_node, &qmp_combo_usb4_phy_ops);
+ if (IS_ERR(qmp->usb4_phy)) {
+ ret = PTR_ERR(qmp->usb4_phy);
+ dev_err(dev, "failed to create USB4 PHY: %d\n", ret);
+ goto err_node_put;
+ }
+
+ phy_set_drvdata(qmp->usb4_phy, qmp);
+ }
+
if (usb_np == dev->of_node)
phy_provider = devm_of_phy_provider_register(dev, qmp_combo_phy_xlate);
else
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 3/5] phy: qualcomm: qmp-combo: Add preliminary USB4 support
2026-07-30 14:19 ` [PATCH v3 3/5] phy: qualcomm: qmp-combo: Add preliminary USB4 support Konrad Dybcio
@ 2026-07-30 14:38 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-30 14:38 UTC (permalink / raw)
To: Konrad Dybcio
Cc: vkoul, olteanv, neil.armstrong, conor+dt, devicetree, robh,
linux-phy
Thank you for your contribution! Sashiko AI review found 8 potential issue(s) to consider:
New issues:
- [High] Unconditional NULL pointer dereference of pcs_aon in qmp_combo_com_init() when configuring USB4 mode.
- [High] Double initialization of the USB PHY hardware registers in qmp_combo_usb4_init().
- [High] Type-C mux events are completely lost if the PHY is uninitialized, leading to incorrect hardware configuration.
- [Medium] Resource leak of p2rr2p_pipe_clk on error paths in qmp_combo_usb4_init().
- [Medium] Data race due to unlocked access to qmpphy_mode and orientation in qmp_combo_typec_switch_set().
Pre-existing issues:
- [High] Missing lock in qmp_combo_typec_mux_set() causes race conditions with concurrent PHY initializations.
- [High] Error paths in qmp_combo_com_init() unconditionally decrement init_count, leading to underflow when force=true.
- [High] The return value of qmp_combo_com_init() is ignored during PHY reconfiguration.
--
--- Patch [3]: [PATCH v3 3/5] phy: qualcomm: qmp-combo: Add preliminary USB4 support ---
commit 77ccc077f3b20bfda2985ec877acdf62ac837b2f
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
phy: qualcomm: qmp-combo: Add preliminary USB4 support
Some Combo PHYs (so far only on SC8280XP, X1E80100 and Glymur), come in
a flavor called USB43DP, which as the name implies, features USB4, USB3
and DP signal processing capabilities. This commit adds a new struct phy
to expose the USB4 mode, along with a .set_mode callback to allow
toggling between USB4 and TBT3 submodes.
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index cdcfad2e86b1d..b80eac9d71401 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
[ ... ]
> @@ -3661,14 +3691,26 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
> {
> const struct qmp_phy_cfg *cfg = qmp->cfg;
> + u32 dp_reset_val = SW_DPPHY_RESET_MUX | SW_DPPHY_RESET;
> void __iomem *com = qmp->com;
> void __iomem *pcs_aon = qmp->pcs_aon;
> + void __iomem *pcs = qmp->pcs;
> int ret;
> u32 val;
>
> if (!force && qmp->init_count++)
> return 0;
[Severity: High]
This is a pre-existing issue, but does calling qmp_combo_com_init(qmp, true)
cause an underflow on error paths?
When force=true, the function skips qmp->init_count++, but if regulators or
clocks fail to enable, it jumps to err_decrement_count and unconditionally
executes qmp->init_count--.
>
> + if (qmp->qmpphy_mode == QMPPHY_MODE_USB4) {
> + pcs = qmp->usb4_pcs;
> +
> + qphy_setbits(pcs_aon, cfg->regs[QPHY_PCS_USB4_CLAMP_ENABLE], CLAMP_EN);
[Severity: High]
Does this unconditionally dereference pcs_aon?
Other parts of the driver explicitly check if (pcs_aon) before accessing USB4
clamp registers, suggesting this register block is optional and could be NULL.
[ ... ]
> @@ -3993,6 +4100,119 @@ static int qmp_combo_usb_set_mode(struct phy *phy, enum phy_mode mode, int submo
> return 0;
> }
>
> +static int qmp_combo_reconfigure_phy(struct qmp_combo *qmp, enum qmpphy_mode new_mode)
> +{
> + dev_dbg(qmp->dev, "qmp_combo_reconfigure_phy: switching from qmpphy mode %d to %d\n",
> + qmp->qmpphy_mode, new_mode);
> +
> + if (qmp->usb_init_count || qmp->usb4phy_init_count)
> + qmp_combo_usb_power_off(qmp->usb_phy);
> +
> + if (qmp->dp_init_count)
> + writel(DP_PHY_PD_CTL_PSR_PWRDN, qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);
> +
> + qmp_combo_com_exit(qmp, true);
> +
> + qmp->qmpphy_mode = new_mode;
> +
> + /* Now everything's powered down, power up the right PHYs */
> + qmp_combo_com_init(qmp, true);
[Severity: High]
This is a pre-existing issue, but is it safe to ignore the return value of
qmp_combo_com_init() here?
If it fails (e.g., failed clock enable), the subsequent call to
qmp_combo_usb_power_on() executes hardware register writes which might cause a
synchronous external abort.
> +
> + if ((qmp->usb_init_count || qmp->usb4phy_init_count) &&
> + new_mode != QMPPHY_MODE_DP_ONLY)
> + qmp_combo_usb_power_on(qmp->usb_phy);
> +
> + if ((new_mode == QMPPHY_MODE_USB3DP || new_mode == QMPPHY_MODE_DP_ONLY) &&
> + qmp->dp_init_count)
> + qmp->cfg->dp_aux_init(qmp);
> +
> + return 0;
> +}
> +
> +static int qmp_combo_usb4_init(struct phy *phy)
> +{
> + struct qmp_combo *qmp = phy_get_drvdata(phy);
> + int ret;
> +
> + guard(mutex)(&qmp->phy_mutex);
> +
> + if (!qmp->p2rr2p_pipe_clk) {
> + dev_err(qmp->dev, "missing p2rr2p_pipe clock handle. Update your Device Tree.\n");
> + return -EINVAL;
> + }
> +
> + ret = clk_prepare_enable(qmp->p2rr2p_pipe_clk);
> + if (ret) {
> + dev_err(qmp->dev, "p2rr2p_pipe enable failed: %d\n", ret);
> + return ret;
> + }
> +
> + ret = qmp_combo_com_init(qmp, false);
> + if (ret)
> + return ret;
[Severity: Medium]
Does this leak the p2rr2p_pipe_clk reference?
If qmp_combo_com_init() or subsequent initialization steps fail, this returns
directly without calling clk_disable_unprepare().
> +
> + /* USB4 mode takes precedence to USB3(+DP), force reconfigure the PHY */
> + ret = qmp_combo_reconfigure_phy(qmp, QMPPHY_MODE_USB4);
> + if (ret)
> + return ret;
> +
> + ret = qmp_combo_usb_power_on(phy);
[Severity: High]
Can this cause a double initialization of the USB PHY hardware registers?
If the USB PHY was previously initialized, qmp_combo_reconfigure_phy()
implicitly calls qmp_combo_usb_power_on(qmp->usb_phy). Then this code
immediately calls qmp_combo_usb_power_on(phy) a second time, which might lead
to hardware state machine lockups or polling timeouts.
[ ... ]
> @@ -4429,24 +4662,29 @@ static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw,
> struct qmp_combo *qmp = typec_switch_get_drvdata(sw);
> const struct qmp_phy_cfg *cfg = qmp->cfg;
>
> + if (qmp->qmpphy_mode == QMPPHY_MODE_USB4) {
> + /* QMPPHY has no orientation handling in USB4 mode, don't cache the setting */
> + qmp->orientation = TYPEC_ORIENTATION_NONE;
> + return 0;
> + }
[Severity: Medium]
Can this result in a data race on qmp->qmpphy_mode and qmp->orientation?
This reads qmp->qmpphy_mode and modifies qmp->orientation before acquiring the
mutex guard, while other routines like qmp_combo_reconfigure_phy() modify these
fields concurrently under the lock.
> +
> if (orientation == qmp->orientation || orientation == TYPEC_ORIENTATION_NONE)
> return 0;
>
> - mutex_lock(&qmp->phy_mutex);
> + guard(mutex)(&qmp->phy_mutex);
[ ... ]
> @@ -4497,41 +4757,11 @@ static int qmp_combo_typec_mux_set(struct typec_mux_dev *mux, struct typec_mux_s
> return 0;
> }
>
> - dev_dbg(qmp->dev, "typec_mux_set: switching from qmpphy mode %d to %d\n",
> - qmp->qmpphy_mode, new_mode);
> -
> - qmp->qmpphy_mode = new_mode;
> -
> - if (qmp->init_count) {
> - if (qmp->usb_init_count)
> - qmp_combo_usb_power_off(qmp->usb_phy);
[ ... ]
> + /* The mux still receives Type-C events, even if all PHYs are uninitialized */
> + if (!qmp->init_count)
> + return 0;
[Severity: High]
Will Type-C mux events be completely lost if the PHY is uninitialized?
Previously, the incoming new_mode was unconditionally cached in qmp->qmpphy_mode
regardless of init_count. Now that the state save is buried inside
qmp_combo_reconfigure_phy(), returning early here means the requested mode
is lost if the PHY hasn't initialized yet.
>
> - return 0;
> + return qmp_combo_reconfigure_phy(qmp, new_mode);
[Severity: High]
This is a pre-existing issue, but is qmp_combo_typec_mux_set() missing a lock
to prevent race conditions?
This executes without holding qmp->phy_mutex and calls
qmp_combo_reconfigure_phy() which manipulates hardware registers. Concurrently,
qmp_combo_usb_init() or qmp_combo_usb4_init() can execute from other paths that
do hold the lock, potentially clobbering the same registers.
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-topic-usb4phy-v3-0-ccf0f7ea98b5@oss.qualcomm.com?part=3
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 4/5] phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa
2026-07-30 14:19 [PATCH v3 0/5] USB4 mode programming for QMMPHY on X1E Konrad Dybcio
` (2 preceding siblings ...)
2026-07-30 14:19 ` [PATCH v3 3/5] phy: qualcomm: qmp-combo: Add preliminary USB4 support Konrad Dybcio
@ 2026-07-30 14:19 ` Konrad Dybcio
2026-07-30 14:44 ` sashiko-bot
2026-07-30 14:19 ` [PATCH v3 5/5] arm64: dts: qcom: hamoa: Extend QMPPHY description for USB4 Konrad Dybcio
4 siblings, 1 reply; 10+ messages in thread
From: Konrad Dybcio @ 2026-07-30 14:19 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-kernel, linux-phy, linux-arm-msm, devicetree, usb4-upstream,
Raghavendra Thoorpu, Mika Westerberg, Sven Peter, Konrad Dybcio,
Dmitry Baryshkov, Abel Vesa
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Add the offsets and configuration tables to support USB4 and
Thunderbolt 3 operation on the USB4-capable PHYs found on Hamoa chips.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 200 ++++++++++++++++++++-
drivers/phy/qualcomm/phy-qcom-qmp-pcs-aon-v6.h | 2 +
drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h | 15 ++
.../phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h | 45 +++++
4 files changed, 256 insertions(+), 6 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index b80eac9d7140..419668e01490 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -235,6 +235,7 @@ static const unsigned int qmp_v6_n4_usb3phy_regs_layout[QPHY_LAYOUT_SIZE] = {
[QPHY_PCS_LFPS_RXTERM_IRQ_CLEAR] = QPHY_V6_PCS_USB3_LFPS_RXTERM_IRQ_CLEAR,
[QPHY_PCS_CLAMP_ENABLE] = QPHY_V6_PCS_AON_CLAMP_ENABLE,
+ [QPHY_PCS_USB4_CLAMP_ENABLE] = QPHY_V6_PCS_AON_USB4_CLAMP_ENABLE,
[QPHY_COM_RESETSM_CNTRL] = QSERDES_V6_COM_RESETSM_CNTRL,
[QPHY_COM_C_READY_STATUS] = QSERDES_V6_COM_C_READY_STATUS,
@@ -1812,7 +1813,85 @@ static const struct qmp_phy_init_tbl sc8280xp_usb43dp_pcs_tbl[] = {
QMP_PHY_INIT_CFG(QPHY_V5_PCS_USB3_RXEQTRAINING_DFE_TIME_S2, 0x07),
};
-static const struct qmp_phy_init_tbl x1e80100_usb43dp_serdes_tbl[] = {
+static const struct qmp_phy_init_tbl x1e80100_usb4_serdes_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_EN_CENTER, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_PER1, 0x62),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_PER2, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE1_MODE0, 0xd9),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE2_MODE0, 0x03),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE1_MODE1, 0xb2),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_STEP_SIZE2_MODE1, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SYSCLK_BUF_ENABLE, 0x0a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CP_CTRL_MODE0, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CP_CTRL_MODE1, 0x02),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_RCTRL_MODE0, 0x18),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_RCTRL_MODE1, 0x18),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_CCTRL_MODE0, 0x36),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_CCTRL_MODE1, 0x36),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SYSCLK_EN_SEL, 0x1a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP_EN, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP_CFG, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP1_MODE0, 0x1a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP2_MODE0, 0x41),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP1_MODE1, 0x24),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_LOCK_CMP2_MODE1, 0x43),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DEC_START_MODE0, 0x82),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DEC_START_MSB_MODE0, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DEC_START_MODE1, 0x86),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DEC_START_MSB_MODE1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START1_MODE0, 0x55),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START2_MODE0, 0x55),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START3_MODE0, 0x03),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START1_MODE1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START2_MODE1, 0xa7),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_DIV_FRAC_START3_MODE1, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE_MAP, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE1_MODE0, 0xba),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE2_MODE0, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE1_MODE1, 0x3c),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE2_MODE1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_HSCLK_SEL_1, 0x11),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_HSCLK_HS_SWITCH_SEL_1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_CORE_CLK_DIV_MODE0, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CORECLK_DIV_MODE1, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_CONFIG_1, 0x76),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CORE_CLK_EN, 0xb0),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_IVCO, 0x7f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_IVCO_MODE1, 0x7f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_INTEGLOOP_GAIN0_MODE0, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_INTEGLOOP_GAIN0_MODE1, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_IPTRIM, 0x17),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SVS_MODE_CLK_SEL, 0x0a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_CMN_MODE, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_VCO_DC_LEVEL_CTRL, 0x07),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_ADDITIONAL_MISC_2, 0x0f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE_INITVAL2, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE_MAXVAL2, 0x01),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_PLL_POST_DIV_MUX, 0x60),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_BG_TIMER, 0x0a),
+};
+
+static const struct qmp_phy_init_tbl x1e80100_usb4_tb3_serdes_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_PER1, 0x15),
+ QMP_PHY_INIT_CFG(QSERDES_V6_COM_VCO_TUNE_MAP, 0x54),
+};
+
+static const struct qmp_phy_init_tbl x1e80100_usb4_tx_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_LANE_MODE_1, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_LANE_MODE_2, 0x50),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_LANE_MODE_3, 0x40),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RES_CODE_LANE_OFFSET_TX, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RES_CODE_LANE_OFFSET_RX, 0x0a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH1_RATE210, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH1_RATE3, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH2_RATE210, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH2_RATE3, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH3_RATE210, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH3_RATE3, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_TX_LDO_TIMER_CTRL, 0x03),
+};
+
+static const struct qmp_phy_init_tbl x1e80100_usb3dp_serdes_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_EN_CENTER, 0x01),
QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_PER1, 0x62),
QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_PER2, 0x02),
@@ -1873,6 +1952,68 @@ static const struct qmp_phy_init_tbl x1e80100_usb43dp_tx_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_RES_CODE_LANE_OFFSET_RX, 0x0a),
};
+static const struct qmp_phy_init_tbl x1e80100_usb4_rx_tbl[] = {
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_SIGDET_CNTRL, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_SIGDET_DEGLITCH_CNTRL, 0x0e),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_SIGDET_ENABLES, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE2_B0, 0xd3),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE2_B1, 0xee),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE2_B2, 0x58),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE2_B4, 0x24),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE3_B0, 0xbf),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE3_B1, 0x7f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE3_B2, 0xa2),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE3_B3, 0xdd),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE3_B4, 0x6a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE3_B5, 0xdb),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_MODE_RATE3_B6, 0xe3),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_LOW_RATE2, 0x30),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_HIGH_RATE2, 0x14),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_LOW_RATE3, 0xff),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_HIGH_RATE3, 0x3f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_PI_CTRL1, 0xd0),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_PI_CTRL2, 0x48),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_SB2_THRESH1_RATE3, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_SB2_GAIN2_RATE2, 0x0a),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_SB2_GAIN1_RATE3, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_SB2_GAIN2_RATE3, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_IVCM_CAL_CODE_OVERRIDE, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_RX_IVCM_CAL_CTRL2, 0x80),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_RX_SUMMER_CAL_SPD_MODE, 0x2f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_CTLE_POST_CAL_OFFSET, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DCC_CMUX_POSTCAL_OFFSET, 0x1d),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_FO_GAIN_RATE2, 0x09),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_FO_GAIN_RATE3, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_SO_GAIN_RATE2, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_SO_GAIN_RATE3, 0x05),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_PI_CONTROLS, 0x15),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_FASTLOCK_FO_GAIN_RATE3, 0x1f),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_FASTLOCK_SO_GAIN_RATE3, 0x08),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_UCDR_SO_SATURATION, 0x36),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_RX_IVCM_POSTCAL_OFFSET, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_VGA_CAL_CNTRL1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_VGA_CAL_MAN_VAL, 0x08),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_DAC_ENABLE1, 0x88),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_DAC_ENABLE2, 0x08),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_1, 0x00),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_2, 0x04),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_3, 0x45),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_TAP3_CTRL, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_TAP4_CTRL, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_TAP5_CTRL, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_GM_CAL, 0x0d),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1, 0x24),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_RX_OFFSET_ADAPTOR_CNTRL3, 0x08),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_EOM_CTRL1, 0x10),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_EOM_CTRL2, 0x21),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_TAP3_MANVAL_KTAP, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_TAP4_MANVAL_KTAP, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_DFE_TAP5_MANVAL_KTAP, 0x06),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_VTHRESH_CAL_MAN_VAL_RATE3, 0x78),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_Q_PI_INTRINSIC_BIAS_RATE32, 0x27),
+ QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_RX_BKUP_CTRL1, 0x14),
+};
+
static const struct qmp_phy_init_tbl x1e80100_usb43dp_rx_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_SIGDET_CNTRL, 0x04),
QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_SIGDET_DEGLITCH_CNTRL, 0x0e),
@@ -1911,6 +2052,23 @@ static const struct qmp_phy_init_tbl x1e80100_usb43dp_rx_tbl[] = {
QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_RX_BKUP_CTRL1, 0x14),
};
+static const struct qmp_phy_init_tbl x1e80100_usb4_pcs_tbl[] = {
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_POWER_STATE_CONFIG3, 0x4f),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_LOCK_DETECT_CONFIG1, 0x03),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_LOCK_DETECT_CONFIG2, 0xc0),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_RX_SIGDET_LVL, 0x55),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_PCS_PCS_TX_RX_CONFIG1, 0x51),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_EQ_CONFIG1, 0x2e),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_G3_EQ_CONFIG1, 0x4b),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_G3_EQ_CONFIG5, 0x1e),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_G2_FOM_EQ_CONFIG1, 0x83),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_G2_FOM_EQ_CONFIG3, 0x2e),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_G2_FOM_EQ_CONFIG5, 0x10),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_G3_FOM_EQ_CONFIG3, 0x25),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_G3_FOM_EQ_CONFIG5, 0x22),
+ QMP_PHY_INIT_CFG(QPHY_V6_PCS_USB4_LPFS_TX_ECSTART, 0x0f),
+};
+
static const struct qmp_phy_init_tbl x1e80100_usb43dp_pcs_tbl[] = {
QMP_PHY_INIT_CFG(QPHY_V6_N4_PCS_RCVR_DTCT_DLY_P1U2_L, 0xe7),
QMP_PHY_INIT_CFG(QPHY_V6_N4_PCS_RCVR_DTCT_DLY_P1U2_H, 0x03),
@@ -2429,6 +2587,23 @@ static const struct qmp_combo_offsets qmp_combo_offsets_v5 = {
.dp_dp_phy = 0x2200,
};
+static const struct qmp_combo_offsets qmp_combo_offsets_v6_n4 = {
+ .com = 0x0000,
+ .usb3_pcs_aon = 0x0100,
+ .txa = 0x0400,
+ .rxa = 0x0600,
+ .txb = 0x0a00,
+ .rxb = 0x0c00,
+ .usb3_serdes = 0x1000,
+ .usb3_pcs_misc = 0x1200,
+ .usb3_pcs = 0x1400,
+ .usb3_pcs_usb = 0x1700,
+ .dp_serdes = 0x2000,
+ .dp_dp_phy = 0x2200,
+ .usb4_serdes = 0x3000,
+ .usb4_pcs = 0x3400,
+};
+
static const struct qmp_combo_offsets qmp_combo_offsets_v8 = {
.com = 0x0000,
.txa = 0x1400,
@@ -2689,11 +2864,11 @@ static const struct qmp_phy_cfg sc8280xp_usb43dpphy_cfg = {
.regs = qmp_v5_5nm_usb3phy_regs_layout,
};
-static const struct qmp_phy_cfg x1e80100_usb3dpphy_cfg = {
- .offsets = &qmp_combo_offsets_v5,
+static const struct qmp_phy_cfg x1e80100_usb43dpphy_cfg = {
+ .offsets = &qmp_combo_offsets_v6_n4,
- .serdes_tbl = x1e80100_usb43dp_serdes_tbl,
- .serdes_tbl_num = ARRAY_SIZE(x1e80100_usb43dp_serdes_tbl),
+ .serdes_tbl = x1e80100_usb3dp_serdes_tbl,
+ .serdes_tbl_num = ARRAY_SIZE(x1e80100_usb3dp_serdes_tbl),
.tx_tbl = x1e80100_usb43dp_tx_tbl,
.tx_tbl_num = ARRAY_SIZE(x1e80100_usb43dp_tx_tbl),
.rx_tbl = x1e80100_usb43dp_rx_tbl,
@@ -2728,6 +2903,19 @@ static const struct qmp_phy_cfg x1e80100_usb3dpphy_cfg = {
.configure_dp_phy = qmp_v4_configure_dp_phy,
.calibrate_dp_phy = qmp_v4_calibrate_dp_phy,
+ .usb4_serdes_tbl = x1e80100_usb4_serdes_tbl,
+ .usb4_serdes_tbl_num = ARRAY_SIZE(x1e80100_usb4_serdes_tbl),
+ .usb4_serdes_tb3_ovrd_tbl = x1e80100_usb4_tb3_serdes_tbl,
+ .usb4_serdes_tb3_ovrd_num = ARRAY_SIZE(x1e80100_usb4_tb3_serdes_tbl),
+
+ .usb4_tx_tbl = x1e80100_usb4_tx_tbl,
+ .usb4_tx_tbl_num = ARRAY_SIZE(x1e80100_usb4_tx_tbl),
+ .usb4_rx_tbl = x1e80100_usb4_rx_tbl,
+ .usb4_rx_tbl_num = ARRAY_SIZE(x1e80100_usb4_rx_tbl),
+
+ .usb4_pcs_tbl = x1e80100_usb4_pcs_tbl,
+ .usb4_pcs_tbl_num = ARRAY_SIZE(x1e80100_usb4_pcs_tbl),
+
.reset_list = msm8996_usb3phy_reset_l,
.num_resets = ARRAY_SIZE(msm8996_usb3phy_reset_l),
.vreg_list = qmp_phy_vreg_l,
@@ -5298,7 +5486,7 @@ static const struct of_device_id qmp_combo_of_match_table[] = {
},
{
.compatible = "qcom,x1e80100-qmp-usb3-dp-phy",
- .data = &x1e80100_usb3dpphy_cfg,
+ .data = &x1e80100_usb43dpphy_cfg,
},
{ }
};
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-aon-v6.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-aon-v6.h
index 52db31a7cf22..f19f461297b6 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-aon-v6.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-aon-v6.h
@@ -8,5 +8,7 @@
/* Only for QMP V6 PHY - PCS_AON registers */
#define QPHY_V6_PCS_AON_CLAMP_ENABLE 0x00
+/* Valid only for USB43DP variants */
+#define QPHY_V6_PCS_AON_USB4_CLAMP_ENABLE 0x04
#endif
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h
index df670143feb1..30fdb437146c 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v6.h
@@ -14,4 +14,19 @@
#define QPHY_V6_PCS_USB3_RCVR_DTCT_DLY_U3_L 0x40
#define QPHY_V6_PCS_USB3_RCVR_DTCT_DLY_U3_H 0x44
+#define QPHY_V6_PCS_USB4_POWER_STATE_CONFIG3 0x98
+#define QPHY_V6_PCS_USB4_LOCK_DETECT_CONFIG1 0xd0
+#define QPHY_V6_PCS_USB4_LOCK_DETECT_CONFIG2 0xd4
+#define QPHY_V6_PCS_USB4_RX_SIGDET_LVL 0x17c
+#define QPHY_V6_PCS_USB4_PCS_PCS_TX_RX_CONFIG1 0x1a8
+#define QPHY_V6_PCS_USB4_EQ_CONFIG1 0x1b8
+#define QPHY_V6_PCS_USB4_G3_EQ_CONFIG1 0x1d8
+#define QPHY_V6_PCS_USB4_G3_EQ_CONFIG5 0x1e8
+#define QPHY_V6_PCS_USB4_G2_FOM_EQ_CONFIG1 0x1f4
+#define QPHY_V6_PCS_USB4_G2_FOM_EQ_CONFIG3 0x1fc
+#define QPHY_V6_PCS_USB4_G2_FOM_EQ_CONFIG5 0x204
+#define QPHY_V6_PCS_USB4_G3_FOM_EQ_CONFIG3 0x210
+#define QPHY_V6_PCS_USB4_G3_FOM_EQ_CONFIG5 0x218
+#define QPHY_V6_PCS_USB4_LPFS_TX_ECSTART 0x220
+
#endif
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h
index d37cc0d4fd36..e72ae2cb460a 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h
@@ -23,27 +23,63 @@
#define QSERDES_V6_N4_TX_TRAN_DRVR_EMP_EN 0xac
#define QSERDES_V6_N4_TX_TX_BAND 0xd8
#define QSERDES_V6_N4_TX_INTERFACE_SELECT 0xe4
+#define QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH1_RATE210 0xe8
+#define QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH1_RATE3 0xec
+#define QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH2_RATE210 0xf0
+#define QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH2_RATE3 0xf4
+#define QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH3_RATE210 0xf8
+#define QSERDES_V6_N4_TX_RX_MARG_COARSE_THRESH3_RATE3 0xfc
#define QSERDES_V6_N4_TX_VMODE_CTRL1 0xb0
+#define QSERDES_V6_TX_LDO_TIMER_CTRL 0x120
#define QSERDES_V6_N4_RX_UCDR_FO_GAIN_RATE2 0x8
+#define QSERDES_V6_N4_RX_UCDR_FO_GAIN_RATE3 0xc
+#define QSERDES_V6_N4_RX_UCDR_SO_GAIN_RATE3 0x1c
#define QSERDES_V6_N4_RX_UCDR_SO_GAIN_RATE2 0x18
#define QSERDES_V6_N4_RX_UCDR_PI_CONTROLS 0x20
+#define QSERDES_V6_N4_RX_EOM_CTRL1 0x44
+#define QSERDES_V6_N4_RX_EOM_CTRL2 0x48
#define QSERDES_V6_N4_RX_IVCM_CAL_CODE_OVERRIDE 0x94
#define QSERDES_V6_N4_RX_RX_IVCM_CAL_CTRL2 0x9c
#define QSERDES_V6_N4_RX_RX_IVCM_POSTCAL_OFFSET 0xa0
+#define QSERDES_V6_N4_RX_DFE_1 0xac
+#define QSERDES_V6_N4_RX_DFE_2 0xb0
#define QSERDES_V6_N4_RX_DFE_3 0xb4
+#define QSERDES_V6_N4_RX_DFE_TAP3_CTRL 0xbc
+#define QSERDES_V6_N4_RX_DFE_TAP3_MANVAL_KTAP 0xc0
+#define QSERDES_V6_N4_RX_DFE_TAP4_CTRL 0xc4
+#define QSERDES_V6_N4_RX_DFE_TAP4_MANVAL_KTAP 0xc8
+#define QSERDES_V6_N4_RX_DFE_TAP5_CTRL 0xcc
+#define QSERDES_V6_N4_RX_DFE_TAP5_MANVAL_KTAP 0xd0
#define QSERDES_V6_N4_RX_VGA_CAL_CNTRL1 0xe0
#define QSERDES_V6_N4_RX_VGA_CAL_MAN_VAL 0xe8
+#define QSERDES_V6_N4_RX_VTHRESH_CAL_MAN_VAL_RATE3 0x108
#define QSERDES_V6_N4_RX_GM_CAL 0x10c
+#define QSERDES_V6_N4_RX_RX_EQ_OFFSET_ADAPTOR_CNTRL1 0x13c
+#define QSERDES_V6_N4_RX_RX_OFFSET_ADAPTOR_CNTRL3 0x144
#define QSERDES_V6_N4_RX_SIGDET_ENABLES 0x148
#define QSERDES_V6_N4_RX_SIGDET_CNTRL 0x14c
#define QSERDES_V6_N4_RX_SIGDET_DEGLITCH_CNTRL 0x154
#define QSERDES_V6_N4_RX_DFE_CTLE_POST_CAL_OFFSET 0x194
+#define QSERDES_V6_N4_RX_DCC_CMUX_POSTCAL_OFFSET 0x1a8
#define QSERDES_V6_N4_RX_Q_PI_INTRINSIC_BIAS_RATE32 0x1dc
+#define QSERDES_V6_N4_RX_UCDR_FASTLOCK_FO_GAIN_RATE3 0x200
+#define QSERDES_V6_N4_RX_UCDR_FASTLOCK_SO_GAIN_RATE3 0x210
+#define QSERDES_V6_N4_RX_UCDR_SO_SATURATION 0x214
+#define QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_LOW_RATE1 0x224
+#define QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_HIGH_RATE1 0x228
+#define QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_LOW_RATE2 0x22c
+#define QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_HIGH_RATE2 0x230
+#define QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_LOW_RATE3 0x234
+#define QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_HIGH_RATE3 0x238
#define QSERDES_V6_N4_RX_UCDR_PI_CTRL1 0x23c
#define QSERDES_V6_N4_RX_UCDR_PI_CTRL2 0x240
+#define QSERDES_V6_N4_RX_UCDR_SB2_THRESH1_RATE3 0x250
+#define QSERDES_V6_N4_RX_UCDR_SB2_GAIN1_RATE3 0x270
#define QSERDES_V6_N4_RX_UCDR_SB2_GAIN2_RATE2 0x27c
+#define QSERDES_V6_N4_RX_UCDR_SB2_GAIN2_RATE3 0x280
#define QSERDES_V6_N4_RX_DFE_DAC_ENABLE1 0x298
+#define QSERDES_V6_N4_RX_DFE_DAC_ENABLE2 0x29c
#define QSERDES_V6_N4_RX_MODE_RATE_0_1_B0 0x2b8
#define QSERDES_V6_N4_RX_MODE_RATE_0_1_B1 0x2bc
#define QSERDES_V6_N4_RX_MODE_RATE_0_1_B2 0x2c0
@@ -58,6 +94,15 @@
#define QSERDES_V6_N4_RX_MODE_RATE2_B4 0x2e4
#define QSERDES_V6_N4_RX_MODE_RATE2_B5 0x2e8
#define QSERDES_V6_N4_RX_MODE_RATE2_B6 0x2ec
+
+#define QSERDES_V6_N4_RX_MODE_RATE3_B0 0x2f0
+#define QSERDES_V6_N4_RX_MODE_RATE3_B1 0x2f4
+#define QSERDES_V6_N4_RX_MODE_RATE3_B2 0x2f8
+#define QSERDES_V6_N4_RX_MODE_RATE3_B3 0x2fc
+#define QSERDES_V6_N4_RX_MODE_RATE3_B4 0x300
+#define QSERDES_V6_N4_RX_MODE_RATE3_B5 0x304
+#define QSERDES_V6_N4_RX_MODE_RATE3_B6 0x308
+
#define QSERDES_V6_N4_RX_RX_SUMMER_CAL_SPD_MODE 0x30c
#define QSERDES_V6_N4_RX_RX_BKUP_CTRL1 0x310
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 4/5] phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa
2026-07-30 14:19 ` [PATCH v3 4/5] phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa Konrad Dybcio
@ 2026-07-30 14:44 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-30 14:44 UTC (permalink / raw)
To: Konrad Dybcio
Cc: robh, vkoul, devicetree, olteanv, linux-phy, neil.armstrong,
conor+dt
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Missing clk_disable_unprepare() for p2rr2p_pipe_clk on error paths in qmp_combo_usb4_init().
- [High] Double initialization of the PHY in qmp_combo_usb4_init() when switching from USB3 mode.
- [Medium] Silent replacement of USB3 configuration values.
- [Medium] Unchecked usage of `pcs_aon` during USB4 initialization.
- [Low] UCDR registers are incorrectly prefixed with TX instead of RX.
--
--- Patch [4]: [PATCH v3 4/5] phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa ---
commit 49d296c106dd364ca64df2721ab78171f96275ce
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Subject: phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa
Add the offsets and configuration tables to support USB4 and
Thunderbolt 3 operation on the USB4-capable PHYs found on Hamoa chips.
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index b80eac9d71401..419668e014906 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
[ ... ]
> +static const struct qmp_phy_init_tbl x1e80100_usb3dp_serdes_tbl[] = {
> QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_EN_CENTER, 0x01),
> QMP_PHY_INIT_CFG(QSERDES_V6_COM_SSC_PER1, 0x62),
[Severity: Medium]
Does this introduce a silent functional change to the USB3 SerDes
initialization sequence?
The patch renames the original x1e80100_usb43dp_serdes_tbl to
x1e80100_usb4_serdes_tbl for USB4, and introduces this new table for USB3
operations.
However, this new table contains distinctly different tuning parameters
(e.g., QSERDES_V6_COM_SSC_STEP_SIZE1_MODE0 changes from 0xd9 to 0xc2, and
LOCK_CMP1_MODE0 changes from 0x1a to 0x08).
Since this change is not documented in the commit message, is there a risk of
regression for USB3 operations?
[ ... ]
> +static const struct qmp_phy_init_tbl x1e80100_usb4_rx_tbl[] = {
> + QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_SIGDET_CNTRL, 0x04),
> + QMP_PHY_INIT_CFG(QSERDES_V6_N4_RX_SIGDET_DEGLITCH_CNTRL, 0x0e),
[ ... ]
> + QMP_PHY_INIT_CFG(QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_LOW_RATE2, 0x30),
[Severity: Low]
This isn't a bug, but should these UCDR macros use an RX prefix instead of TX?
UCDR (Universal Clock and Data Recovery) is strictly an RX feature, and these
offsets belong to the RX block. They are populated correctly in this RX table,
but the misleading _TX_ prefix could pose a maintainability risk.
[ ... ]
> @@ -2689,11 +2864,11 @@ static const struct qmp_phy_cfg sc8280xp_usb43dpphy_cfg = {
> .regs = qmp_v5_5nm_usb3phy_regs_layout,
> };
>
> -static const struct qmp_phy_cfg x1e80100_usb3dpphy_cfg = {
> - .offsets = &qmp_combo_offsets_v5,
> +static const struct qmp_phy_cfg x1e80100_usb43dpphy_cfg = {
> + .offsets = &qmp_combo_offsets_v6_n4,
[Severity: Medium]
Does supporting USB4 introduce a latent bug in qmp_combo_com_init()?
In qmp_combo_com_init(), if qmp->qmpphy_mode == QMPPHY_MODE_USB4, the driver
directly executes:
qmp_combo_com_init() {
...
if (qmp->qmpphy_mode == QMPPHY_MODE_USB4)
qphy_setbits(pcs_aon, cfg->regs[QPHY_PCS_USB4_CLAMP_ENABLE], CLAMP_EN);
...
}
Unlike other instances where pcs_aon is strictly checked (if (pcs_aon)) before
dereferencing, this assumes pcs_aon is populated. While this works on
x1e80100 because its layout defines .usb3_pcs_aon (in qmp_combo_offsets_v6_n4),
any future configuration supporting USB4 but omitting usb3_pcs_aon (e.g.,
sc8280xp which uses qmp_combo_offsets_v5) would trigger a NULL pointer
dereference.
[Severity: High]
Does enabling USB4 expose resource leaks in qmp_combo_usb4_init()?
If a failure occurs during initialization, qmp_combo_usb4_init() immediately
returns the error code:
qmp_combo_usb4_init() {
...
ret = qmp_combo_com_init(qmp, false);
if (ret)
return ret;
...
}
However, it fails to call clk_disable_unprepare(qmp->p2rr2p_pipe_clk), which
was successfully enabled earlier in the function. Does this result in a
resource leak on the error paths for qmp_combo_com_init(),
qmp_combo_reconfigure_phy(), and qmp_combo_usb_power_on()?
[Severity: High]
Can switching from USB3 mode cause a double initialization of the PHY?
When qmp_combo_usb4_init() executes and USB3 is already active
(qmp->usb_init_count > 0), it calls:
qmp_combo_usb4_init() {
...
ret = qmp_combo_reconfigure_phy(qmp, QMPPHY_MODE_USB4);
if (ret)
return ret;
ret = qmp_combo_usb_power_on(phy);
...
}
Inside qmp_combo_reconfigure_phy(), if qmp->usb_init_count is non-zero, it
explicitly calls qmp_combo_usb_power_on(qmp->usb_phy).
After returning successfully, qmp_combo_usb4_init() proceeds to call
qmp_combo_usb_power_on(phy) a second time. Could writing the entire
configuration sequence (tuning parameters, clearing resets, setting
SERDES_START | PCS_START) to an actively running PHY cause hardware lockups
or polling timeouts?
[ ... ]
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h
> index d37cc0d4fd365..e72ae2cb460ac 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-qserdes-txrx-v6_n4.h
> @@ -23,27 +23,63 @@
[ ... ]
> +#define QSERDES_V6_N4_TX_UCDR_FASTLOCK_COUNTER_LOW_RATE1 0x224
[Severity: Low]
This isn't a bug, but as mentioned above, these UCDR macros are prefixed with
TX instead of RX despite UCDR being an RX feature.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-topic-usb4phy-v3-0-ccf0f7ea98b5@oss.qualcomm.com?part=4
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 5/5] arm64: dts: qcom: hamoa: Extend QMPPHY description for USB4
2026-07-30 14:19 [PATCH v3 0/5] USB4 mode programming for QMMPHY on X1E Konrad Dybcio
` (3 preceding siblings ...)
2026-07-30 14:19 ` [PATCH v3 4/5] phy: qualcomm: qmp-combo: Add USB4/TBT3 configuration data for Hamoa Konrad Dybcio
@ 2026-07-30 14:19 ` Konrad Dybcio
2026-07-30 14:31 ` sashiko-bot
4 siblings, 1 reply; 10+ messages in thread
From: Konrad Dybcio @ 2026-07-30 14:19 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Bjorn Andersson, Konrad Dybcio
Cc: linux-kernel, linux-phy, linux-arm-msm, devicetree, usb4-upstream,
Raghavendra Thoorpu, Mika Westerberg, Sven Peter, Konrad Dybcio,
Dmitry Baryshkov, Abel Vesa
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
The USB4 part of the QMPPHY requires that one more GCC clock (P2RR2P -
PHY-to-Router, Router-to-PHY) is enabled for the PHY to initialize
successfully. Describe that.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
arch/arm64/boot/dts/qcom/hamoa.dtsi | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
index 09527dcf9576..2effc0319a01 100644
--- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
+++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
@@ -2825,11 +2825,13 @@ usb_1_ss0_qmpphy: phy@fd5000 {
clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
<&rpmhcc RPMH_CXO_CLK>,
<&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
- <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
+ <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>,
+ <&gcc GCC_USB4_0_PHY_P2RR2P_PIPE_CLK>;
clock-names = "aux",
"ref",
"com_aux",
- "usb3_pipe";
+ "usb3_pipe",
+ "p2rr2p_pipe";
power-domains = <&gcc GCC_USB_0_PHY_GDSC>;
@@ -2896,11 +2898,13 @@ usb_1_ss1_qmpphy: phy@fda000 {
clocks = <&gcc GCC_USB3_SEC_PHY_AUX_CLK>,
<&tcsr TCSR_USB4_1_CLKREF_EN>,
<&gcc GCC_USB3_SEC_PHY_COM_AUX_CLK>,
- <&gcc GCC_USB3_SEC_PHY_PIPE_CLK>;
+ <&gcc GCC_USB3_SEC_PHY_PIPE_CLK>,
+ <&gcc GCC_USB4_1_PHY_P2RR2P_PIPE_CLK>;
clock-names = "aux",
"ref",
"com_aux",
- "usb3_pipe";
+ "usb3_pipe",
+ "p2rr2p_pipe";
power-domains = <&gcc GCC_USB_1_PHY_GDSC>;
@@ -2967,11 +2971,13 @@ usb_1_ss2_qmpphy: phy@fdf000 {
clocks = <&gcc GCC_USB3_TERT_PHY_AUX_CLK>,
<&tcsr TCSR_USB4_2_CLKREF_EN>,
<&gcc GCC_USB3_TERT_PHY_COM_AUX_CLK>,
- <&gcc GCC_USB3_TERT_PHY_PIPE_CLK>;
+ <&gcc GCC_USB3_TERT_PHY_PIPE_CLK>,
+ <&gcc GCC_USB4_2_PHY_P2RR2P_PIPE_CLK>;
clock-names = "aux",
"ref",
"com_aux",
- "usb3_pipe";
+ "usb3_pipe",
+ "p2rr2p_pipe";
power-domains = <&gcc GCC_USB_2_PHY_GDSC>;
--
2.55.0
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH v3 5/5] arm64: dts: qcom: hamoa: Extend QMPPHY description for USB4
2026-07-30 14:19 ` [PATCH v3 5/5] arm64: dts: qcom: hamoa: Extend QMPPHY description for USB4 Konrad Dybcio
@ 2026-07-30 14:31 ` sashiko-bot
0 siblings, 0 replies; 10+ messages in thread
From: sashiko-bot @ 2026-07-30 14:31 UTC (permalink / raw)
To: Konrad Dybcio
Cc: neil.armstrong, vkoul, olteanv, robh, linux-phy, conor+dt,
devicetree
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] A resource leak is present in `qmp_combo_usb4_init()` (introduced by a previous patch in this series) and is exposed when this DT patch provides the missing `p2rr2p_pipe` clock. If `clk_prepare_enable(qmp->p2rr2p_pipe_clk)` succeeds, the function may later fail during `qmp_combo_com_init()`, `qmp_combo_reconfigure_phy()`, or `qmp_combo_usb_power_on()`. On these error paths, the function returns without calling `clk_disable_unprepare(qmp->p2rr2p_pipe_clk)`, leaking the clock reference.
--
--- Patch [5]: [PATCH v3 5/5] arm64: dts: qcom: hamoa: Extend QMPPHY description for USB4 ---
commit f94fbc66a74d14588b2084c9550548446b3f5825
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
arm64: dts: qcom: hamoa: Extend QMPPHY description for USB4
This commit extends the QMPPHY description for USB4 by adding the missing
GCC_USB4_X_PHY_P2RR2P_PIPE_CLK clock. This additional clock is required
for the PHY to initialize successfully.
> diff --git a/arch/arm64/boot/dts/qcom/hamoa.dtsi b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> index 09527dcf95767..2effc0319a01a 100644
> --- a/arch/arm64/boot/dts/qcom/hamoa.dtsi
> +++ b/arch/arm64/boot/dts/qcom/hamoa.dtsi
> @@ -2825,11 +2825,13 @@ usb_1_ss0_qmpphy: phy@fd5000 {
> clocks = <&gcc GCC_USB3_PRIM_PHY_AUX_CLK>,
> <&rpmhcc RPMH_CXO_CLK>,
> <&gcc GCC_USB3_PRIM_PHY_COM_AUX_CLK>,
> - <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>;
> + <&gcc GCC_USB3_PRIM_PHY_PIPE_CLK>,
> + <&gcc GCC_USB4_0_PHY_P2RR2P_PIPE_CLK>;
> clock-names = "aux",
> "ref",
> "com_aux",
> - "usb3_pipe";
> + "usb3_pipe",
> + "p2rr2p_pipe";
[Severity: Medium]
This isn't a bug introduced by this device tree patch itself, but providing
this missing p2rr2p_pipe clock exposes a resource leak in the driver's
error path.
If the call to clk_prepare_enable(qmp->p2rr2p_pipe_clk) succeeds in
qmp_combo_usb4_init():
drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_usb4_init() {
...
ret = clk_prepare_enable(qmp->p2rr2p_pipe_clk);
if (ret) {
dev_err(qmp->dev, "p2rr2p_pipe enable failed: %d\n", ret);
return ret;
}
ret = qmp_combo_com_init(qmp, false);
if (ret)
return ret;
...
ret = qmp_combo_usb_power_on(phy);
if (ret) {
qmp_combo_com_exit(qmp, false);
return ret;
}
...
}
Does this code leak the p2rr2p_pipe_clk reference if qmp_combo_com_init(),
qmp_combo_reconfigure_phy(), or qmp_combo_usb_power_on() fails?
Should it call clk_disable_unprepare(qmp->p2rr2p_pipe_clk) on these error
paths before returning?
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260730-topic-usb4phy-v3-0-ccf0f7ea98b5@oss.qualcomm.com?part=5
^ permalink raw reply [flat|nested] 10+ messages in thread