linux-kernel - [PATCH] phy: qcom: qmp-combo: Move pipe

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20250927093915.45124-2-val@packett.cool>
Date: Sat, 27 Sep 2025 06:17:32 -0300
From: Val Packett <val@...kett.cool>
To: Vinod Koul <vkoul@...nel.org>,
	Kishon Vijay Abraham I <kishon@...nel.org>
Cc: Val Packett <val@...kett.cool>,
	Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
	Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>,
	Neil Armstrong <neil.armstrong@...aro.org>,
	linux-arm-msm@...r.kernel.org,
	linux-phy@...ts.infradead.org,
	linux-kernel@...r.kernel.org
Subject: [PATCH] phy: qcom: qmp-combo: Move pipe_clk on/off to common

Keep the USB pipe clock working when the phy is in DP-only mode, because
the dwc controller still needs it for USB 2.0 over the same Type-C port.

Tested with the BenQ RD280UA monitor which has a downstream-facing port
for data passthrough that's manually switchable between USB 2 and 3,
corresponding to 4-lane and 2-lane DP respectively.

Note: the suspend/resume callbacks were already gating the enable/disable
of this clock only on init_count and not usb_init_count!

Signed-off-by: Val Packett <val@...kett.cool>
---
o/

Just got my hands on a perfect test device for DP alt mode: a monitor with an
on-demand toggle between 2 and 4 lanes. (Started digging because I thought
I needed 4 lanes to use its full resolution and refresh rate, even though
it turned out to be the dpu adjusted mode clock check rejecting the modes,
patches for which are already posted.)

In [1] Konrad mentioned that "the hardware disagrees" with keeping the USB
PLL always on. I'm not sure what exactly was meant by disagreement there,
and I didn't find any specific code that touches that PLL in the driver,
so I decided to just try it anyway.

Before the changes, 4-lane mode would actually kill the USB 2.0 functionality
on the port, no recovery until reboot.

With this patch, I can switch the monitor between 4-lane and 2-lane modes
(with an unplug-replug cycle..) and the USB 2.0 devices attached through
the monitor keep working! (I verified the number of lanes used via dp_debug).

I'm sure it might not be that simple but from my limited and uninformed
understanding without any internal knowledge, the "sneaky workaround"
might actually be the intended way to do things?

Thanks,
~val

P.S. if I'm actually wrong and this is not acceptable for $reasons, the suspend
and resume callbacks would need to be changed to match the logic of having the
clk on depending on usb_init_count, not just the overall init_count.

[1]: https://lore.kernel.org/all/f21b7d52-4c3f-4e5b-bee7-f8b2945b5b02@oss.qualcomm.com/

---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 7b5af30f1d02..c4bbd738eba1 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -3035,6 +3035,13 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
 	if (ret)
 		goto err_assert_reset;

+	/* In DP-only mode, the pipe clk is still required for USB2 */
+	ret = clk_prepare_enable(qmp->pipe_clk);
+	if (ret) {
+		dev_err(qmp->dev, "pipe_clk enable failed err=%d\n", ret);
+		return ret;
+	}
+
 	qphy_setbits(com, QPHY_V3_DP_COM_POWER_DOWN_CTRL, SW_PWRDN);

 	/* override hardware control for reset of qmp phy */
@@ -3103,6 +3110,7 @@ static int qmp_combo_com_exit(struct qmp_combo *qmp, bool force)
 	reset_control_bulk_assert(cfg->num_resets, qmp->resets);

 	clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
+	clk_disable_unprepare(qmp->pipe_clk);

 	regulator_bulk_disable(cfg->num_vregs, qmp->vregs);

@@ -3205,12 +3213,6 @@ static int qmp_combo_usb_power_on(struct phy *phy)

 	qmp_configure(qmp->dev, serdes, cfg->serdes_tbl, cfg->serdes_tbl_num);

-	ret = clk_prepare_enable(qmp->pipe_clk);
-	if (ret) {
-		dev_err(qmp->dev, "pipe_clk enable failed err=%d\n", ret);
-		return ret;
-	}
-
 	/* 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);
@@ -3254,8 +3256,6 @@ 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;

-	clk_disable_unprepare(qmp->pipe_clk);
-
 	/* PHY reset */
 	qphy_setbits(qmp->pcs, cfg->regs[QPHY_SW_RESET], SW_RESET);

-- 
2.51.0