[PATCH v2 0/7] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode

[PATCH v2 0/7] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode

[Neil Armstrong]

Register a typec mux in order to change the PHY mode on the Type-C
mux events depending on the mode and the svid when in Altmode setup.

The DisplayPort phy should be left enabled if is still powered on
by the DRM DisplayPort controller, so bail out until the DisplayPort
PHY is not powered off.

The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
will be set in between of USB-Only, Combo and DisplayPort Only so
this will leave enough time to the DRM DisplayPort controller to
turn of the DisplayPort PHY.

The patchset also includes bindings changes and DT changes.

This has been successfully tested on an SM8550 board, but the
Thinkpad X13s deserved testing between non-PD USB, non-PD DisplayPort,
PD USB Hubs and PD Altmode Dongles to make sure the switch works
as expected.

The DisplayPort 4 lanes setup can be check with:
$ cat /sys/kernel/debug/dri/ae01000.display-controller/DP-1/dp_debug
	name = msm_dp
	drm_dp_link
		rate = 540000
		num_lanes = 4
...

This patchset depends on [1] to allow broadcasting the type-c mode
to the PHY, otherwise the PHY will keep the combo state while the
retimer would setup the 4 lanes in DP mode.

[1] https://lore.kernel.org/all/20240527-topic-sm8x50-upstream-retimer-broadcast-mode-v1-0-79ec91381aba@linaro.org/

To: Bjorn Andersson <andersson@kernel.org>
To: Konrad Dybcio <konrad.dybcio@linaro.org>
To: Vinod Koul <vkoul@kernel.org>
To: Kishon Vijay Abraham I <kishon@kernel.org>
To: Rob Herring <robh@kernel.org>
To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
To: Conor Dooley <conor+dt@kernel.org>
To: Krzysztof Kozlowski <krzk+dt@kernel.org>
Cc: linux-arm-msm@vger.kernel.org
Cc: linux-phy@lists.infradead.org
Cc: devicetree@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>

Changes in v2:
- Reference usb-switch.yaml in bindings patch
- Fix switch/case indenting
- Check svid for USB_TYPEC_DP_SID
- Fix X13s patch subject
- Update SM8650 patch to enable 4 lanes on HDK aswell
- Link to v1: https://lore.kernel.org/r/20240229-topic-sm8x50-upstream-phy-combo-typec-mux-v1-0-07e24a231840@linaro.org

---
Neil Armstrong (7):
      dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch
      phy: qcom: qmp-combo: store DP phy power state
      phy: qcom: qmp-combo: introduce QPHY_MODE
      phy: qcom: qmp-combo: register a typec mux to change the QPHY_MODE
      arm64: dts: qcom-sm8550: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch
      arm64: dts: qcom-sm8650: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch
      arm64: dts: qcom: sc8280xp-lenovo-thinkpad-x13: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch

 .../phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml         |   7 +-
 .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts     |   6 +-
 arch/arm64/boot/dts/qcom/sm8550-hdk.dts            |   3 +-
 arch/arm64/boot/dts/qcom/sm8550-qrd.dts            |   3 +-
 arch/arm64/boot/dts/qcom/sm8650-hdk.dts            |   3 +-
 arch/arm64/boot/dts/qcom/sm8650-qrd.dts            |   3 +-
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c          | 169 +++++++++++++++++++--
 7 files changed, 174 insertions(+), 20 deletions(-)
---
base-commit: d4eef8b2e18d3e4d2343fb3bb975f8ac4522129a
change-id: 20240229-topic-sm8x50-upstream-phy-combo-typec-mux-31b5252513c9

Best regards,
-- 
Neil Armstrong <neil.armstrong@linaro.org>

[PATCH v2 1/7] dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch

[Neil Armstrong]

The QMP USB3/DP Combo PHY can work in 3 modes:
- DisplayPort Only
- USB3 Only
- USB3 + DisplayPort Combo mode

In order to switch between those modes, the PHY needs to receive
Type-C events, allow marking to the phy with the mode-switch
property in order to allow the PHY to Type-C events.

Reference usb-switch.yaml as a simpler way to allow the mode-switch
property instead of duplicating the property definition.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 .../devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml     | 7 +++----
 1 file changed, 3 insertions(+), 4 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 2d0d7e9e6431..5358d3a6fde3 100644
--- a/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml
@@ -71,10 +71,8 @@ properties:
     description:
       See include/dt-bindings/phy/phy-qcom-qmp.h
 
-  orientation-switch:
-    description:
-      Flag the PHY as possible handler of USB Type-C orientation switching
-    type: boolean
+  mode-switch: true
+  orientation-switch: true
 
   ports:
     $ref: /schemas/graph.yaml#/properties/ports
@@ -104,6 +102,7 @@ required:
   - "#phy-cells"
 
 allOf:
+  - $ref: /schemas/usb/usb-switch.yaml#
   - if:
       properties:
         compatible:

-- 
2.34.1

[PATCH v2 2/7] phy: qcom: qmp-combo: store DP phy power state

[Neil Armstrong]

Switching the PHY Mode requires the DisplayPort PHY to be powered off,
keep track of the DisplayPort phy power state.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 7f999e8a433d..183cd9cd1884 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -1512,6 +1512,7 @@ struct qmp_combo {
 	unsigned int dp_aux_cfg;
 	struct phy_configure_opts_dp dp_opts;
 	unsigned int dp_init_count;
+	bool dp_powered_on;
 
 	struct clk_fixed_rate pipe_clk_fixed;
 	struct clk_hw dp_link_hw;
@@ -2685,6 +2686,8 @@ static int qmp_combo_dp_power_on(struct phy *phy)
 	/* Configure link rate, swing, etc. */
 	cfg->configure_dp_phy(qmp);
 
+	qmp->dp_powered_on = true;
+
 	mutex_unlock(&qmp->phy_mutex);
 
 	return 0;
@@ -2699,6 +2702,8 @@ static int qmp_combo_dp_power_off(struct phy *phy)
 	/* Assert DP PHY power down */
 	writel(DP_PHY_PD_CTL_PSR_PWRDN, qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);
 
+	qmp->dp_powered_on = false;
+
 	mutex_unlock(&qmp->phy_mutex);
 
 	return 0;

-- 
2.34.1

[PATCH v2 3/7] phy: qcom: qmp-combo: introduce QPHY_MODE

[Neil Armstrong]

Introduce an enum for the QMP Combo PHY modes, use it in the
QMP commmon phy init function and default to COMBO mode.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 41 +++++++++++++++++++++++++++----
 1 file changed, 36 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 183cd9cd1884..788e4c05eaf2 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -61,6 +61,12 @@
 
 #define PHY_INIT_COMPLETE_TIMEOUT		10000
 
+enum qphy_mode {
+	QPHY_MODE_COMBO = 0,
+	QPHY_MODE_DP_ONLY,
+	QPHY_MODE_USB_ONLY,
+};
+
 /* set of registers with offsets different per-PHY */
 enum qphy_reg_layout {
 	/* PCS registers */
@@ -1503,6 +1509,7 @@ struct qmp_combo {
 
 	struct mutex phy_mutex;
 	int init_count;
+	enum qphy_mode init_mode;
 
 	struct phy *usb_phy;
 	enum phy_mode mode;
@@ -2589,12 +2596,33 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
 	if (qmp->orientation == TYPEC_ORIENTATION_REVERSE)
 		val |= SW_PORTSELECT_VAL;
 	writel(val, com + QPHY_V3_DP_COM_TYPEC_CTRL);
-	writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
 
-	/* bring both QMP USB and QMP DP PHYs PCS block out of reset */
-	qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
-			SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
-			SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
+	switch (qmp->init_mode) {
+	case QPHY_MODE_COMBO:
+		writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
+
+		/* bring both QMP USB and QMP DP PHYs PCS block out of reset */
+		qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
+				SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
+				SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
+		break;
+
+	case QPHY_MODE_DP_ONLY:
+		writel(DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
+
+		/* bring QMP DP PHY PCS block out of reset */
+		qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
+				SW_DPPHY_RESET_MUX | SW_DPPHY_RESET);
+		break;
+
+	case QPHY_MODE_USB_ONLY:
+		writel(USB3_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
+
+		/* bring QMP USB PHY PCS block out of reset */
+		qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
+				SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
+		break;
+	}
 
 	qphy_clrbits(com, QPHY_V3_DP_COM_SWI_CTRL, 0x03);
 	qphy_clrbits(com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);
@@ -3603,6 +3631,9 @@ static int qmp_combo_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_node_put;
 
+	/* Set PHY_MODE as combo by default */
+	qmp->init_mode = QPHY_MODE_COMBO;
+
 	qmp->usb_phy = devm_phy_create(dev, usb_np, &qmp_combo_usb_phy_ops);
 	if (IS_ERR(qmp->usb_phy)) {
 		ret = PTR_ERR(qmp->usb_phy);

-- 
2.34.1

[PATCH v2 4/7] phy: qcom: qmp-combo: register a typec mux to change the QPHY_MODE

[Neil Armstrong]

Register a typec mux in order to change the PHY mode on the Type-C
mux events depending on the mode and the svid when in Altmode setup.

The DisplayPort phy should be left enabled if is still powered on
by the DRM DisplayPort controller, so bail out until the DisplayPort
PHY is not powered off.

The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
will be set in between of USB-Only, Combo and DisplayPort Only so
this will leave enough time to the DRM DisplayPort controller to
turn of the DisplayPort PHY.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 123 ++++++++++++++++++++++++++++--
 1 file changed, 118 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 788e4c05eaf2..b55ab08d44c2 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -19,6 +19,7 @@
 #include <linux/reset.h>
 #include <linux/slab.h>
 #include <linux/usb/typec.h>
+#include <linux/usb/typec_dp.h>
 #include <linux/usb/typec_mux.h>
 
 #include <drm/bridge/aux-bridge.h>
@@ -1527,6 +1528,10 @@ struct qmp_combo {
 
 	struct typec_switch_dev *sw;
 	enum typec_orientation orientation;
+
+	struct typec_mux_dev *mux;
+	unsigned long mux_mode;
+	unsigned int svid;
 };
 
 static void qmp_v3_dp_aux_init(struct qmp_combo *qmp);
@@ -3353,17 +3358,112 @@ static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw,
 	return 0;
 }
 
-static void qmp_combo_typec_unregister(void *data)
+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 qphy_mode new_mode;
+	unsigned int svid;
+
+	if (state->mode == qmp->mode)
+		return 0;
+
+	mutex_lock(&qmp->phy_mutex);
+
+	if (state->alt)
+		svid = state->alt->svid;
+	else
+		svid = 0; // No SVID
+
+	if (svid == USB_TYPEC_DP_SID) {
+		switch (state->mode) {
+		/* DP Only */
+		case TYPEC_DP_STATE_C:
+		case TYPEC_DP_STATE_E:
+			new_mode = QPHY_MODE_DP_ONLY;
+			break;
+
+		/* DP + USB */
+		case TYPEC_DP_STATE_D:
+		case TYPEC_DP_STATE_F:
+
+		/* Safe fallback...*/
+		default:
+			new_mode = QPHY_MODE_COMBO;
+			break;
+		}
+	} else {
+		/* Only switch to USB_ONLY when we know we only have USB3 */
+		if (qmp->mux_mode == TYPEC_MODE_USB3)
+			new_mode = QPHY_MODE_USB_ONLY;
+		else
+			new_mode = QPHY_MODE_COMBO;
+	}
+
+	if (new_mode == qmp->init_mode) {
+		dev_dbg(qmp->dev, "typec_mux_set: same phy mode, bail out\n");
+		qmp->mode = state->mode;
+		goto out;
+	}
+
+	if (qmp->init_mode != QPHY_MODE_USB_ONLY && qmp->dp_powered_on) {
+		dev_dbg(qmp->dev, "typec_mux_set: DP is still powered on, delaying switch\n");
+		goto out;
+	}
+
+	dev_dbg(qmp->dev, "typec_mux_set: switching from phy mode %d to %d\n",
+		qmp->init_mode, new_mode);
+
+	qmp->mux_mode = state->mode;
+	qmp->init_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 (qmp->init_mode == QPHY_MODE_DP_ONLY && qmp->usb_init_count) {
+			qmp->usb_init_count--;
+		} else if (qmp->init_mode != QPHY_MODE_DP_ONLY) {
+			qmp_combo_usb_power_on(qmp->usb_phy);
+			if (!qmp->usb_init_count)
+				qmp->usb_init_count++;
+		}
+		if (qmp->init_mode != QPHY_MODE_USB_ONLY && qmp->dp_init_count)
+			cfg->dp_aux_init(qmp);
+	}
+
+out:
+	mutex_unlock(&qmp->phy_mutex);
+
+	return 0;
+}
+
+static void qmp_combo_typec_switch_unregister(void *data)
 {
 	struct qmp_combo *qmp = data;
 
 	typec_switch_unregister(qmp->sw);
 }
 
-static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
+static void qmp_combo_typec_mux_unregister(void *data)
+{
+	struct qmp_combo *qmp = data;
+
+	typec_mux_unregister(qmp->mux);
+}
+
+static int qmp_combo_typec_register(struct qmp_combo *qmp)
 {
 	struct typec_switch_desc sw_desc = {};
+	struct typec_mux_desc mux_desc = { };
 	struct device *dev = qmp->dev;
+	int ret;
 
 	sw_desc.drvdata = qmp;
 	sw_desc.fwnode = dev->fwnode;
@@ -3374,10 +3474,23 @@ static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
 		return PTR_ERR(qmp->sw);
 	}
 
-	return devm_add_action_or_reset(dev, qmp_combo_typec_unregister, qmp);
+	ret = devm_add_action_or_reset(dev, qmp_combo_typec_switch_unregister, qmp);
+	if (ret)
+		return ret;
+
+	mux_desc.drvdata = qmp;
+	mux_desc.fwnode = dev->fwnode;
+	mux_desc.set = qmp_combo_typec_mux_set;
+	qmp->mux = typec_mux_register(dev, &mux_desc);
+	if (IS_ERR(qmp->mux)) {
+		dev_err(dev, "Unable to register typec mux: %pe\n", qmp->mux);
+		return PTR_ERR(qmp->mux);
+	}
+
+	return devm_add_action_or_reset(dev, qmp_combo_typec_mux_unregister, qmp);
 }
 #else
-static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
+static int qmp_combo_typec_register(struct qmp_combo *qmp)
 {
 	return 0;
 }
@@ -3609,7 +3722,7 @@ static int qmp_combo_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_node_put;
 
-	ret = qmp_combo_typec_switch_register(qmp);
+	ret = qmp_combo_typec_register(qmp);
 	if (ret)
 		goto err_node_put;
 

-- 
2.34.1

[PATCH v2 5/7] arm64: dts: qcom-sm8550: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch

[Neil Armstrong]

Allow up to 4 lanes for the DisplayPort link from the PHY to the Controller
and allow mode-switch events to the QMP Combo PHY.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8550-hdk.dts | 3 ++-
 arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
index ccff744dcd14..a95949c01f25 100644
--- a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
+++ b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
@@ -941,7 +941,7 @@ &mdss_dp0 {
 
 &mdss_dp0_out {
 	remote-endpoint = <&usb_dp_qmpphy_dp_in>;
-	data-lanes = <0 1>;
+	data-lanes = <0 1 2 3>;
 };
 
 &pcie0 {
@@ -1280,6 +1280,7 @@ &usb_dp_qmpphy {
 	vdda-phy-supply = <&vreg_l3e_1p2>;
 	vdda-pll-supply = <&vreg_l3f_0p88>;
 
+	mode-switch;
 	orientation-switch;
 
 	status = "okay";
diff --git a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
index 39ba3e9969b7..fbac5270b4d7 100644
--- a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
+++ b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
@@ -795,7 +795,7 @@ &mdss_dp0 {
 };
 
 &mdss_dp0_out {
-	data-lanes = <0 1>;
+	data-lanes = <0 1 2 3>;
 	remote-endpoint = <&usb_dp_qmpphy_dp_in>;
 };
 
@@ -1142,6 +1142,7 @@ &usb_dp_qmpphy {
 	vdda-phy-supply = <&vreg_l3e_1p2>;
 	vdda-pll-supply = <&vreg_l3f_0p88>;
 
+	mode-switch;
 	orientation-switch;
 
 	status = "okay";

-- 
2.34.1

[PATCH v2 6/7] arm64: dts: qcom-sm8650: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch

[Neil Armstrong]

Allow up to 4 lanes for the DisplayPort link from the PHY to the Controller
and allow mode-switch events to the QMP Combo PHY.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 arch/arm64/boot/dts/qcom/sm8650-hdk.dts | 3 ++-
 arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sm8650-hdk.dts b/arch/arm64/boot/dts/qcom/sm8650-hdk.dts
index 3791c36579be..9ab07e265321 100644
--- a/arch/arm64/boot/dts/qcom/sm8650-hdk.dts
+++ b/arch/arm64/boot/dts/qcom/sm8650-hdk.dts
@@ -872,7 +872,7 @@ &mdss_dp0 {
 };
 
 &mdss_dp0_out {
-	data-lanes = <0 1>;
+	data-lanes = <0 1 2 3>;
 	remote-endpoint = <&usb_dp_qmpphy_dp_in>;
 };
 
@@ -1229,6 +1229,7 @@ &usb_dp_qmpphy {
 	vdda-phy-supply = <&vreg_l3i_1p2>;
 	vdda-pll-supply = <&vreg_l3g_0p91>;
 
+	mode-switch;
 	orientation-switch;
 
 	status = "okay";
diff --git a/arch/arm64/boot/dts/qcom/sm8650-qrd.dts b/arch/arm64/boot/dts/qcom/sm8650-qrd.dts
index bd87aa3aa548..884e846842f5 100644
--- a/arch/arm64/boot/dts/qcom/sm8650-qrd.dts
+++ b/arch/arm64/boot/dts/qcom/sm8650-qrd.dts
@@ -831,7 +831,7 @@ &mdss_dp0 {
 };
 
 &mdss_dp0_out {
-	data-lanes = <0 1>;
+	data-lanes = <0 1 2 3>;
 	remote-endpoint = <&usb_dp_qmpphy_dp_in>;
 };
 
@@ -1224,6 +1224,7 @@ &usb_dp_qmpphy {
 	vdda-phy-supply = <&vreg_l3i_1p2>;
 	vdda-pll-supply = <&vreg_l3g_0p91>;
 
+	mode-switch;
 	orientation-switch;
 
 	status = "okay";

-- 
2.34.1

[PATCH v2 7/7] arm64: dts: qcom: sc8280xp-lenovo-thinkpad-x13: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch

[Neil Armstrong]

Allow up to 4 lanes for the DisplayPort link from the PHYs to the Controllers
and allow mode-switch events to the QMP Combo PHYs.

Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
---
 arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
index e937732abede..bcd38831f9d3 100644
--- a/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
+++ b/arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts
@@ -593,7 +593,7 @@ &mdss0_dp0 {
 };
 
 &mdss0_dp0_out {
-	data-lanes = <0 1>;
+	data-lanes = <0 1 2 3>;
 	remote-endpoint = <&usb_0_qmpphy_dp_in>;
 };
 
@@ -602,7 +602,7 @@ &mdss0_dp1 {
 };
 
 &mdss0_dp1_out {
-	data-lanes = <0 1>;
+	data-lanes = <0 1 2 3>;
 	remote-endpoint = <&usb_1_qmpphy_dp_in>;
 };
 
@@ -1143,6 +1143,7 @@ &usb_0_qmpphy {
 	vdda-phy-supply = <&vreg_l9d>;
 	vdda-pll-supply = <&vreg_l4d>;
 
+	mode-switch;
 	orientation-switch;
 
 	status = "okay";
@@ -1180,6 +1181,7 @@ &usb_1_qmpphy {
 	vdda-phy-supply = <&vreg_l4b>;
 	vdda-pll-supply = <&vreg_l3b>;
 
+	mode-switch;
 	orientation-switch;
 
 	status = "okay";

-- 
2.34.1

Re: [PATCH v2 3/7] phy: qcom: qmp-combo: introduce QPHY_MODE

[Dmitry Baryshkov]

On Mon, May 27, 2024 at 10:42:35AM +0200, Neil Armstrong wrote:
> Introduce an enum for the QMP Combo PHY modes, use it in the
> QMP commmon phy init function and default to COMBO mode.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 41 +++++++++++++++++++++++++++----
>  1 file changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 183cd9cd1884..788e4c05eaf2 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -61,6 +61,12 @@
>  
>  #define PHY_INIT_COMPLETE_TIMEOUT		10000
>  
> +enum qphy_mode {
> +	QPHY_MODE_COMBO = 0,
> +	QPHY_MODE_DP_ONLY,
> +	QPHY_MODE_USB_ONLY,
> +};
> +
>  /* set of registers with offsets different per-PHY */
>  enum qphy_reg_layout {
>  	/* PCS registers */
> @@ -1503,6 +1509,7 @@ struct qmp_combo {
>  
>  	struct mutex phy_mutex;
>  	int init_count;
> +	enum qphy_mode init_mode;
>  
>  	struct phy *usb_phy;
>  	enum phy_mode mode;
> @@ -2589,12 +2596,33 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
>  	if (qmp->orientation == TYPEC_ORIENTATION_REVERSE)
>  		val |= SW_PORTSELECT_VAL;
>  	writel(val, com + QPHY_V3_DP_COM_TYPEC_CTRL);
> -	writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
>  
> -	/* bring both QMP USB and QMP DP PHYs PCS block out of reset */
> -	qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
> -			SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
> -			SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
> +	switch (qmp->init_mode) {
> +	case QPHY_MODE_COMBO:
> +		writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
> +
> +		/* bring both QMP USB and QMP DP PHYs PCS block out of reset */
> +		qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
> +				SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
> +				SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
> +		break;
> +
> +	case QPHY_MODE_DP_ONLY:
> +		writel(DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
> +
> +		/* bring QMP DP PHY PCS block out of reset */
> +		qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
> +				SW_DPPHY_RESET_MUX | SW_DPPHY_RESET);
> +		break;
> +
> +	case QPHY_MODE_USB_ONLY:
> +		writel(USB3_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
> +
> +		/* bring QMP USB PHY PCS block out of reset */
> +		qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
> +				SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
> +		break;
> +	}
>  
>  	qphy_clrbits(com, QPHY_V3_DP_COM_SWI_CTRL, 0x03);
>  	qphy_clrbits(com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);
> @@ -3603,6 +3631,9 @@ static int qmp_combo_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_node_put;
>  
> +	/* Set PHY_MODE as combo by default */
> +	qmp->init_mode = QPHY_MODE_COMBO;
> +

I see that COMBO mode is backwards compatible with existing code. But
shouldn't the USB-only be a default mode?

>  	qmp->usb_phy = devm_phy_create(dev, usb_np, &qmp_combo_usb_phy_ops);
>  	if (IS_ERR(qmp->usb_phy)) {
>  		ret = PTR_ERR(qmp->usb_phy);
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 3/7] phy: qcom: qmp-combo: introduce QPHY_MODE

[Neil Armstrong]

On 27/05/2024 10:46, Dmitry Baryshkov wrote:
> On Mon, May 27, 2024 at 10:42:35AM +0200, Neil Armstrong wrote:
>> Introduce an enum for the QMP Combo PHY modes, use it in the
>> QMP commmon phy init function and default to COMBO mode.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 41 +++++++++++++++++++++++++++----
>>   1 file changed, 36 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> index 183cd9cd1884..788e4c05eaf2 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> @@ -61,6 +61,12 @@
>>   
>>   #define PHY_INIT_COMPLETE_TIMEOUT		10000
>>   
>> +enum qphy_mode {
>> +	QPHY_MODE_COMBO = 0,
>> +	QPHY_MODE_DP_ONLY,
>> +	QPHY_MODE_USB_ONLY,
>> +};
>> +
>>   /* set of registers with offsets different per-PHY */
>>   enum qphy_reg_layout {
>>   	/* PCS registers */
>> @@ -1503,6 +1509,7 @@ struct qmp_combo {
>>   
>>   	struct mutex phy_mutex;
>>   	int init_count;
>> +	enum qphy_mode init_mode;
>>   
>>   	struct phy *usb_phy;
>>   	enum phy_mode mode;
>> @@ -2589,12 +2596,33 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
>>   	if (qmp->orientation == TYPEC_ORIENTATION_REVERSE)
>>   		val |= SW_PORTSELECT_VAL;
>>   	writel(val, com + QPHY_V3_DP_COM_TYPEC_CTRL);
>> -	writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
>>   
>> -	/* bring both QMP USB and QMP DP PHYs PCS block out of reset */
>> -	qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
>> -			SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
>> -			SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
>> +	switch (qmp->init_mode) {
>> +	case QPHY_MODE_COMBO:
>> +		writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
>> +
>> +		/* bring both QMP USB and QMP DP PHYs PCS block out of reset */
>> +		qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
>> +				SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
>> +				SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
>> +		break;
>> +
>> +	case QPHY_MODE_DP_ONLY:
>> +		writel(DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
>> +
>> +		/* bring QMP DP PHY PCS block out of reset */
>> +		qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
>> +				SW_DPPHY_RESET_MUX | SW_DPPHY_RESET);
>> +		break;
>> +
>> +	case QPHY_MODE_USB_ONLY:
>> +		writel(USB3_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
>> +
>> +		/* bring QMP USB PHY PCS block out of reset */
>> +		qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
>> +				SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
>> +		break;
>> +	}
>>   
>>   	qphy_clrbits(com, QPHY_V3_DP_COM_SWI_CTRL, 0x03);
>>   	qphy_clrbits(com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);
>> @@ -3603,6 +3631,9 @@ static int qmp_combo_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		goto err_node_put;
>>   
>> +	/* Set PHY_MODE as combo by default */
>> +	qmp->init_mode = QPHY_MODE_COMBO;
>> +
> 
> I see that COMBO mode is backwards compatible with existing code. But
> shouldn't the USB-only be a default mode?

No because it would break existing platforms without "mode-switch" in DT.

Neil

> 
>>   	qmp->usb_phy = devm_phy_create(dev, usb_np, &qmp_combo_usb_phy_ops);
>>   	if (IS_ERR(qmp->usb_phy)) {
>>   		ret = PTR_ERR(qmp->usb_phy);
>>
>> -- 
>> 2.34.1
>>
> 

Re: [PATCH v2 4/7] phy: qcom: qmp-combo: register a typec mux to change the QPHY_MODE

[Dmitry Baryshkov]

On Mon, May 27, 2024 at 10:42:36AM +0200, Neil Armstrong wrote:
> Register a typec mux in order to change the PHY mode on the Type-C
> mux events depending on the mode and the svid when in Altmode setup.
> 
> The DisplayPort phy should be left enabled if is still powered on
> by the DRM DisplayPort controller, so bail out until the DisplayPort
> PHY is not powered off.
> 
> The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
> will be set in between of USB-Only, Combo and DisplayPort Only so
> this will leave enough time to the DRM DisplayPort controller to
> turn of the DisplayPort PHY.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 123 ++++++++++++++++++++++++++++--
>  1 file changed, 118 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 788e4c05eaf2..b55ab08d44c2 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -19,6 +19,7 @@
>  #include <linux/reset.h>
>  #include <linux/slab.h>
>  #include <linux/usb/typec.h>
> +#include <linux/usb/typec_dp.h>
>  #include <linux/usb/typec_mux.h>
>  
>  #include <drm/bridge/aux-bridge.h>
> @@ -1527,6 +1528,10 @@ struct qmp_combo {
>  
>  	struct typec_switch_dev *sw;
>  	enum typec_orientation orientation;
> +
> +	struct typec_mux_dev *mux;
> +	unsigned long mux_mode;
> +	unsigned int svid;
>  };
>  
>  static void qmp_v3_dp_aux_init(struct qmp_combo *qmp);
> @@ -3353,17 +3358,112 @@ static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw,
>  	return 0;
>  }
>  
> -static void qmp_combo_typec_unregister(void *data)
> +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 qphy_mode new_mode;
> +	unsigned int svid;
> +
> +	if (state->mode == qmp->mode)
> +		return 0;
> +
> +	mutex_lock(&qmp->phy_mutex);
> +
> +	if (state->alt)
> +		svid = state->alt->svid;
> +	else
> +		svid = 0; // No SVID
> +
> +	if (svid == USB_TYPEC_DP_SID) {
> +		switch (state->mode) {
> +		/* DP Only */
> +		case TYPEC_DP_STATE_C:
> +		case TYPEC_DP_STATE_E:
> +			new_mode = QPHY_MODE_DP_ONLY;
> +			break;
> +
> +		/* DP + USB */
> +		case TYPEC_DP_STATE_D:
> +		case TYPEC_DP_STATE_F:
> +
> +		/* Safe fallback...*/
> +		default:
> +			new_mode = QPHY_MODE_COMBO;
> +			break;
> +		}
> +	} else {
> +		/* Only switch to USB_ONLY when we know we only have USB3 */
> +		if (qmp->mux_mode == TYPEC_MODE_USB3)
> +			new_mode = QPHY_MODE_USB_ONLY;
> +		else
> +			new_mode = QPHY_MODE_COMBO;
> +	}
> +
> +	if (new_mode == qmp->init_mode) {
> +		dev_dbg(qmp->dev, "typec_mux_set: same phy mode, bail out\n");
> +		qmp->mode = state->mode;
> +		goto out;
> +	}
> +
> +	if (qmp->init_mode != QPHY_MODE_USB_ONLY && qmp->dp_powered_on) {
> +		dev_dbg(qmp->dev, "typec_mux_set: DP is still powered on, delaying switch\n");
> +		goto out;
> +	}
> +
> +	dev_dbg(qmp->dev, "typec_mux_set: switching from phy mode %d to %d\n",
> +		qmp->init_mode, new_mode);
> +
> +	qmp->mux_mode = state->mode;
> +	qmp->init_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 (qmp->init_mode == QPHY_MODE_DP_ONLY && qmp->usb_init_count) {
> +			qmp->usb_init_count--;

Can we move this clause next to actually powering USB part off?

> +		} else if (qmp->init_mode != QPHY_MODE_DP_ONLY) {
> +			qmp_combo_usb_power_on(qmp->usb_phy);
> +			if (!qmp->usb_init_count)
> +				qmp->usb_init_count++;
> +		}
> +		if (qmp->init_mode != QPHY_MODE_USB_ONLY && qmp->dp_init_count)
> +			cfg->dp_aux_init(qmp);

Does dp_init_count reflect the actual necessity to bring up the DP part
up? Maybe we can unify the code between this function and
qmp_combo_typec_switch_set()? I don't like that it is unobvious whether
these two functions will results in the same state or not depending on
the order in which they are being called.

> +	}
> +
> +out:
> +	mutex_unlock(&qmp->phy_mutex);
> +
> +	return 0;
> +}
> +
> +static void qmp_combo_typec_switch_unregister(void *data)
>  {
>  	struct qmp_combo *qmp = data;
>  
>  	typec_switch_unregister(qmp->sw);
>  }
>  
> -static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
> +static void qmp_combo_typec_mux_unregister(void *data)
> +{
> +	struct qmp_combo *qmp = data;
> +
> +	typec_mux_unregister(qmp->mux);
> +}
> +
> +static int qmp_combo_typec_register(struct qmp_combo *qmp)
>  {
>  	struct typec_switch_desc sw_desc = {};
> +	struct typec_mux_desc mux_desc = { };
>  	struct device *dev = qmp->dev;
> +	int ret;
>  
>  	sw_desc.drvdata = qmp;
>  	sw_desc.fwnode = dev->fwnode;
> @@ -3374,10 +3474,23 @@ static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
>  		return PTR_ERR(qmp->sw);
>  	}
>  
> -	return devm_add_action_or_reset(dev, qmp_combo_typec_unregister, qmp);
> +	ret = devm_add_action_or_reset(dev, qmp_combo_typec_switch_unregister, qmp);
> +	if (ret)
> +		return ret;
> +
> +	mux_desc.drvdata = qmp;
> +	mux_desc.fwnode = dev->fwnode;
> +	mux_desc.set = qmp_combo_typec_mux_set;
> +	qmp->mux = typec_mux_register(dev, &mux_desc);
> +	if (IS_ERR(qmp->mux)) {
> +		dev_err(dev, "Unable to register typec mux: %pe\n", qmp->mux);
> +		return PTR_ERR(qmp->mux);
> +	}
> +
> +	return devm_add_action_or_reset(dev, qmp_combo_typec_mux_unregister, qmp);
>  }
>  #else
> -static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
> +static int qmp_combo_typec_register(struct qmp_combo *qmp)
>  {
>  	return 0;
>  }
> @@ -3609,7 +3722,7 @@ static int qmp_combo_probe(struct platform_device *pdev)
>  	if (ret)
>  		goto err_node_put;
>  
> -	ret = qmp_combo_typec_switch_register(qmp);
> +	ret = qmp_combo_typec_register(qmp);
>  	if (ret)
>  		goto err_node_put;
>  
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 2/7] phy: qcom: qmp-combo: store DP phy power state

[Dmitry Baryshkov]

On Mon, May 27, 2024 at 10:42:34AM +0200, Neil Armstrong wrote:
> Switching the PHY Mode requires the DisplayPort PHY to be powered off,
> keep track of the DisplayPort phy power state.

How is this different from dp_init_count?

> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 7f999e8a433d..183cd9cd1884 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -1512,6 +1512,7 @@ struct qmp_combo {
>  	unsigned int dp_aux_cfg;
>  	struct phy_configure_opts_dp dp_opts;
>  	unsigned int dp_init_count;
> +	bool dp_powered_on;
>  
>  	struct clk_fixed_rate pipe_clk_fixed;
>  	struct clk_hw dp_link_hw;
> @@ -2685,6 +2686,8 @@ static int qmp_combo_dp_power_on(struct phy *phy)
>  	/* Configure link rate, swing, etc. */
>  	cfg->configure_dp_phy(qmp);
>  
> +	qmp->dp_powered_on = true;
> +
>  	mutex_unlock(&qmp->phy_mutex);
>  
>  	return 0;
> @@ -2699,6 +2702,8 @@ static int qmp_combo_dp_power_off(struct phy *phy)
>  	/* Assert DP PHY power down */
>  	writel(DP_PHY_PD_CTL_PSR_PWRDN, qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);
>  
> +	qmp->dp_powered_on = false;
> +
>  	mutex_unlock(&qmp->phy_mutex);
>  
>  	return 0;
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 3/7] phy: qcom: qmp-combo: introduce QPHY_MODE

[Dmitry Baryshkov]

On Mon, May 27, 2024 at 10:48:12AM +0200, Neil Armstrong wrote:
> On 27/05/2024 10:46, Dmitry Baryshkov wrote:
> > On Mon, May 27, 2024 at 10:42:35AM +0200, Neil Armstrong wrote:
> > > Introduce an enum for the QMP Combo PHY modes, use it in the
> > > QMP commmon phy init function and default to COMBO mode.
> > > 
> > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> > > ---
> > >   drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 41 +++++++++++++++++++++++++++----
> > >   1 file changed, 36 insertions(+), 5 deletions(-)
> > > 

[trimmed]

> > > @@ -3603,6 +3631,9 @@ static int qmp_combo_probe(struct platform_device *pdev)
> > >   	if (ret)
> > >   		goto err_node_put;
> > > +	/* Set PHY_MODE as combo by default */
> > > +	qmp->init_mode = QPHY_MODE_COMBO;
> > > +
> > 
> > I see that COMBO mode is backwards compatible with existing code. But
> > shouldn't the USB-only be a default mode?
> 
> No because it would break existing platforms without "mode-switch" in DT.


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


> 
> Neil
> 
> > 
> > >   	qmp->usb_phy = devm_phy_create(dev, usb_np, &qmp_combo_usb_phy_ops);
> > >   	if (IS_ERR(qmp->usb_phy)) {
> > >   		ret = PTR_ERR(qmp->usb_phy);
> > > 
> > > -- 
> > > 2.34.1
> > > 
> > 
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 5/7] arm64: dts: qcom-sm8550: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch

[Dmitry Baryshkov]

On Mon, May 27, 2024 at 10:42:37AM +0200, Neil Armstrong wrote:
> Allow up to 4 lanes for the DisplayPort link from the PHY to the Controller
> and allow mode-switch events to the QMP Combo PHY.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8550-hdk.dts | 3 ++-
>  arch/arm64/boot/dts/qcom/sm8550-qrd.dts | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> index ccff744dcd14..a95949c01f25 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8550-hdk.dts
> @@ -941,7 +941,7 @@ &mdss_dp0 {
>  
>  &mdss_dp0_out {
>  	remote-endpoint = <&usb_dp_qmpphy_dp_in>;
> -	data-lanes = <0 1>;
> +	data-lanes = <0 1 2 3>;
>  };
>  
>  &pcie0 {
> @@ -1280,6 +1280,7 @@ &usb_dp_qmpphy {
>  	vdda-phy-supply = <&vreg_l3e_1p2>;
>  	vdda-pll-supply = <&vreg_l3f_0p88>;
>  
> +	mode-switch;
>  	orientation-switch;

Please rebase on top of https://lore.kernel.org/linux-arm-msm/20240429-usb-link-dtsi-v1-0-87c341b55cdf@linaro.org/

After that:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


>  
>  	status = "okay";
> diff --git a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
> index 39ba3e9969b7..fbac5270b4d7 100644
> --- a/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
> +++ b/arch/arm64/boot/dts/qcom/sm8550-qrd.dts
> @@ -795,7 +795,7 @@ &mdss_dp0 {
>  };
>  
>  &mdss_dp0_out {
> -	data-lanes = <0 1>;
> +	data-lanes = <0 1 2 3>;
>  	remote-endpoint = <&usb_dp_qmpphy_dp_in>;
>  };
>  
> @@ -1142,6 +1142,7 @@ &usb_dp_qmpphy {
>  	vdda-phy-supply = <&vreg_l3e_1p2>;
>  	vdda-pll-supply = <&vreg_l3f_0p88>;
>  
> +	mode-switch;
>  	orientation-switch;
>  
>  	status = "okay";
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 6/7] arm64: dts: qcom-sm8650: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch

[Dmitry Baryshkov]

On Mon, May 27, 2024 at 10:42:38AM +0200, Neil Armstrong wrote:
> Allow up to 4 lanes for the DisplayPort link from the PHY to the Controller
> and allow mode-switch events to the QMP Combo PHY.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sm8650-hdk.dts | 3 ++-
>  arch/arm64/boot/dts/qcom/sm8650-qrd.dts | 3 ++-
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 

After rebase,


Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


-- 
With best wishes
Dmitry

Re: [PATCH v2 7/7] arm64: dts: qcom: sc8280xp-lenovo-thinkpad-x13: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch

[Dmitry Baryshkov]

On Mon, May 27, 2024 at 10:42:39AM +0200, Neil Armstrong wrote:
> Allow up to 4 lanes for the DisplayPort link from the PHYs to the Controllers
> and allow mode-switch events to the QMP Combo PHYs.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


-- 
With best wishes
Dmitry

Re: [PATCH v2 1/7] dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch

[Krzysztof Kozlowski]

On 27/05/2024 10:42, Neil Armstrong wrote:
> The QMP USB3/DP Combo PHY can work in 3 modes:
> - DisplayPort Only
> - USB3 Only
> - USB3 + DisplayPort Combo mode
> 
> In order to switch between those modes, the PHY needs to receive
> Type-C events, allow marking to the phy with the mode-switch
> property in order to allow the PHY to Type-C events.
> 
> Reference usb-switch.yaml as a simpler way to allow the mode-switch
> property instead of duplicating the property definition.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---

Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>

Best regards,
Krzysztof

Re: [PATCH v2 0/7] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode

[Bjorn Andersson]

On Mon, May 27, 2024 at 10:42:32AM GMT, Neil Armstrong wrote:
> Register a typec mux in order to change the PHY mode on the Type-C
> mux events depending on the mode and the svid when in Altmode setup.
> 
> The DisplayPort phy should be left enabled if is still powered on
> by the DRM DisplayPort controller, so bail out until the DisplayPort
> PHY is not powered off.
> 
> The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
> will be set in between of USB-Only, Combo and DisplayPort Only so
> this will leave enough time to the DRM DisplayPort controller to
> turn of the DisplayPort PHY.
> 
> The patchset also includes bindings changes and DT changes.
> 
> This has been successfully tested on an SM8550 board, but the
> Thinkpad X13s deserved testing between non-PD USB, non-PD DisplayPort,
> PD USB Hubs and PD Altmode Dongles to make sure the switch works
> as expected.
> 
> The DisplayPort 4 lanes setup can be check with:
> $ cat /sys/kernel/debug/dri/ae01000.display-controller/DP-1/dp_debug
> 	name = msm_dp
> 	drm_dp_link
> 		rate = 540000
> 		num_lanes = 4

Has the issue with the USB controller dying on us been resolved?

Regards,
Bjorn

Re: [PATCH v2 0/7] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode

[Neil Armstrong]

On 28/05/2024 15:48, Bjorn Andersson wrote:
> On Mon, May 27, 2024 at 10:42:32AM GMT, Neil Armstrong wrote:
>> Register a typec mux in order to change the PHY mode on the Type-C
>> mux events depending on the mode and the svid when in Altmode setup.
>>
>> The DisplayPort phy should be left enabled if is still powered on
>> by the DRM DisplayPort controller, so bail out until the DisplayPort
>> PHY is not powered off.
>>
>> The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
>> will be set in between of USB-Only, Combo and DisplayPort Only so
>> this will leave enough time to the DRM DisplayPort controller to
>> turn of the DisplayPort PHY.
>>
>> The patchset also includes bindings changes and DT changes.
>>
>> This has been successfully tested on an SM8550 board, but the
>> Thinkpad X13s deserved testing between non-PD USB, non-PD DisplayPort,
>> PD USB Hubs and PD Altmode Dongles to make sure the switch works
>> as expected.
>>
>> The DisplayPort 4 lanes setup can be check with:
>> $ cat /sys/kernel/debug/dri/ae01000.display-controller/DP-1/dp_debug
>> 	name = msm_dp
>> 	drm_dp_link
>> 		rate = 540000
>> 		num_lanes = 4
> 
> Has the issue with the USB controller dying on us been resolved?

No, this would require some changes in dwc3 & ucsi to support the USB_ROLE_NONE

I haven't looked into this yet.

Neil

> 
> Regards,
> Bjorn

Re: [PATCH v2 2/7] phy: qcom: qmp-combo: store DP phy power state

[Neil Armstrong]

On 27/05/2024 10:59, Dmitry Baryshkov wrote:
> On Mon, May 27, 2024 at 10:42:34AM +0200, Neil Armstrong wrote:
>> Switching the PHY Mode requires the DisplayPort PHY to be powered off,
>> keep track of the DisplayPort phy power state.
> 
> How is this different from dp_init_count?

dp_init_count tracks the DP PHY init, while dp_powered_on tracks
the DP PHY beeing powered on by the DRM DP driver, those are
not the same state at all.

While testing, I figured that de-initializing the DP PHY while
is was powered-on by the DRM DP, caused the system to freeze and crash.

SO I've added this to track this state and try to de-init the DP phy
if still in use.

Neil

> 
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> index 7f999e8a433d..183cd9cd1884 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> @@ -1512,6 +1512,7 @@ struct qmp_combo {
>>   	unsigned int dp_aux_cfg;
>>   	struct phy_configure_opts_dp dp_opts;
>>   	unsigned int dp_init_count;
>> +	bool dp_powered_on;
>>   
>>   	struct clk_fixed_rate pipe_clk_fixed;
>>   	struct clk_hw dp_link_hw;
>> @@ -2685,6 +2686,8 @@ static int qmp_combo_dp_power_on(struct phy *phy)
>>   	/* Configure link rate, swing, etc. */
>>   	cfg->configure_dp_phy(qmp);
>>   
>> +	qmp->dp_powered_on = true;
>> +
>>   	mutex_unlock(&qmp->phy_mutex);
>>   
>>   	return 0;
>> @@ -2699,6 +2702,8 @@ static int qmp_combo_dp_power_off(struct phy *phy)
>>   	/* Assert DP PHY power down */
>>   	writel(DP_PHY_PD_CTL_PSR_PWRDN, qmp->dp_dp_phy + QSERDES_DP_PHY_PD_CTL);
>>   
>> +	qmp->dp_powered_on = false;
>> +
>>   	mutex_unlock(&qmp->phy_mutex);
>>   
>>   	return 0;
>>
>> -- 
>> 2.34.1
>>
> 

Re: [PATCH v2 4/7] phy: qcom: qmp-combo: register a typec mux to change the QPHY_MODE

[Neil Armstrong]

On 27/05/2024 10:57, Dmitry Baryshkov wrote:
> On Mon, May 27, 2024 at 10:42:36AM +0200, Neil Armstrong wrote:
>> Register a typec mux in order to change the PHY mode on the Type-C
>> mux events depending on the mode and the svid when in Altmode setup.
>>
>> The DisplayPort phy should be left enabled if is still powered on
>> by the DRM DisplayPort controller, so bail out until the DisplayPort
>> PHY is not powered off.
>>
>> The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
>> will be set in between of USB-Only, Combo and DisplayPort Only so
>> this will leave enough time to the DRM DisplayPort controller to
>> turn of the DisplayPort PHY.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 123 ++++++++++++++++++++++++++++--
>>   1 file changed, 118 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> index 788e4c05eaf2..b55ab08d44c2 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> @@ -19,6 +19,7 @@
>>   #include <linux/reset.h>
>>   #include <linux/slab.h>
>>   #include <linux/usb/typec.h>
>> +#include <linux/usb/typec_dp.h>
>>   #include <linux/usb/typec_mux.h>
>>   
>>   #include <drm/bridge/aux-bridge.h>
>> @@ -1527,6 +1528,10 @@ struct qmp_combo {
>>   
>>   	struct typec_switch_dev *sw;
>>   	enum typec_orientation orientation;
>> +
>> +	struct typec_mux_dev *mux;
>> +	unsigned long mux_mode;
>> +	unsigned int svid;
>>   };
>>   
>>   static void qmp_v3_dp_aux_init(struct qmp_combo *qmp);
>> @@ -3353,17 +3358,112 @@ static int qmp_combo_typec_switch_set(struct typec_switch_dev *sw,
>>   	return 0;
>>   }
>>   
>> -static void qmp_combo_typec_unregister(void *data)
>> +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 qphy_mode new_mode;
>> +	unsigned int svid;
>> +
>> +	if (state->mode == qmp->mode)
>> +		return 0;
>> +
>> +	mutex_lock(&qmp->phy_mutex);
>> +
>> +	if (state->alt)
>> +		svid = state->alt->svid;
>> +	else
>> +		svid = 0; // No SVID
>> +
>> +	if (svid == USB_TYPEC_DP_SID) {
>> +		switch (state->mode) {
>> +		/* DP Only */
>> +		case TYPEC_DP_STATE_C:
>> +		case TYPEC_DP_STATE_E:
>> +			new_mode = QPHY_MODE_DP_ONLY;
>> +			break;
>> +
>> +		/* DP + USB */
>> +		case TYPEC_DP_STATE_D:
>> +		case TYPEC_DP_STATE_F:
>> +
>> +		/* Safe fallback...*/
>> +		default:
>> +			new_mode = QPHY_MODE_COMBO;
>> +			break;
>> +		}
>> +	} else {
>> +		/* Only switch to USB_ONLY when we know we only have USB3 */
>> +		if (qmp->mux_mode == TYPEC_MODE_USB3)
>> +			new_mode = QPHY_MODE_USB_ONLY;
>> +		else
>> +			new_mode = QPHY_MODE_COMBO;
>> +	}
>> +
>> +	if (new_mode == qmp->init_mode) {
>> +		dev_dbg(qmp->dev, "typec_mux_set: same phy mode, bail out\n");
>> +		qmp->mode = state->mode;
>> +		goto out;
>> +	}
>> +
>> +	if (qmp->init_mode != QPHY_MODE_USB_ONLY && qmp->dp_powered_on) {
>> +		dev_dbg(qmp->dev, "typec_mux_set: DP is still powered on, delaying switch\n");
>> +		goto out;
>> +	}
>> +
>> +	dev_dbg(qmp->dev, "typec_mux_set: switching from phy mode %d to %d\n",
>> +		qmp->init_mode, new_mode);
>> +
>> +	qmp->mux_mode = state->mode;
>> +	qmp->init_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 (qmp->init_mode == QPHY_MODE_DP_ONLY && qmp->usb_init_count) {
>> +			qmp->usb_init_count--;
> 
> Can we move this clause next to actually powering USB part off?
> 
>> +		} else if (qmp->init_mode != QPHY_MODE_DP_ONLY) {
>> +			qmp_combo_usb_power_on(qmp->usb_phy);
>> +			if (!qmp->usb_init_count)
>> +				qmp->usb_init_count++;
>> +		}
>> +		if (qmp->init_mode != QPHY_MODE_USB_ONLY && qmp->dp_init_count)
>> +			cfg->dp_aux_init(qmp);
> 
> Does dp_init_count reflect the actual necessity to bring up the DP part
> up? Maybe we can unify the code between this function and
> qmp_combo_typec_switch_set()? I don't like that it is unobvious whether
> these two functions will results in the same state or not depending on
> the order in which they are being called.

Yep, I'll try to use a common function, so any switch/mux call would use
the same process, and I can probably simplify it.

Thanks,
Neil

> 
>> +	}
>> +
>> +out:
>> +	mutex_unlock(&qmp->phy_mutex);
>> +
>> +	return 0;
>> +}
>> +
>> +static void qmp_combo_typec_switch_unregister(void *data)
>>   {
>>   	struct qmp_combo *qmp = data;
>>   
>>   	typec_switch_unregister(qmp->sw);
>>   }
>>   
>> -static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
>> +static void qmp_combo_typec_mux_unregister(void *data)
>> +{
>> +	struct qmp_combo *qmp = data;
>> +
>> +	typec_mux_unregister(qmp->mux);
>> +}
>> +
>> +static int qmp_combo_typec_register(struct qmp_combo *qmp)
>>   {
>>   	struct typec_switch_desc sw_desc = {};
>> +	struct typec_mux_desc mux_desc = { };
>>   	struct device *dev = qmp->dev;
>> +	int ret;
>>   
>>   	sw_desc.drvdata = qmp;
>>   	sw_desc.fwnode = dev->fwnode;
>> @@ -3374,10 +3474,23 @@ static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
>>   		return PTR_ERR(qmp->sw);
>>   	}
>>   
>> -	return devm_add_action_or_reset(dev, qmp_combo_typec_unregister, qmp);
>> +	ret = devm_add_action_or_reset(dev, qmp_combo_typec_switch_unregister, qmp);
>> +	if (ret)
>> +		return ret;
>> +
>> +	mux_desc.drvdata = qmp;
>> +	mux_desc.fwnode = dev->fwnode;
>> +	mux_desc.set = qmp_combo_typec_mux_set;
>> +	qmp->mux = typec_mux_register(dev, &mux_desc);
>> +	if (IS_ERR(qmp->mux)) {
>> +		dev_err(dev, "Unable to register typec mux: %pe\n", qmp->mux);
>> +		return PTR_ERR(qmp->mux);
>> +	}
>> +
>> +	return devm_add_action_or_reset(dev, qmp_combo_typec_mux_unregister, qmp);
>>   }
>>   #else
>> -static int qmp_combo_typec_switch_register(struct qmp_combo *qmp)
>> +static int qmp_combo_typec_register(struct qmp_combo *qmp)
>>   {
>>   	return 0;
>>   }
>> @@ -3609,7 +3722,7 @@ static int qmp_combo_probe(struct platform_device *pdev)
>>   	if (ret)
>>   		goto err_node_put;
>>   
>> -	ret = qmp_combo_typec_switch_register(qmp);
>> +	ret = qmp_combo_typec_register(qmp);
>>   	if (ret)
>>   		goto err_node_put;
>>   
>>
>> -- 
>> 2.34.1
>>
> 

Re: [PATCH v2 2/7] phy: qcom: qmp-combo: store DP phy power state

[Dmitry Baryshkov]

On Thu, Jun 06, 2024 at 03:29:52PM +0200, Neil Armstrong wrote:
> On 27/05/2024 10:59, Dmitry Baryshkov wrote:
> > On Mon, May 27, 2024 at 10:42:34AM +0200, Neil Armstrong wrote:
> > > Switching the PHY Mode requires the DisplayPort PHY to be powered off,
> > > keep track of the DisplayPort phy power state.
> > 
> > How is this different from dp_init_count?
> 
> dp_init_count tracks the DP PHY init, while dp_powered_on tracks
> the DP PHY beeing powered on by the DRM DP driver, those are
> not the same state at all.
> 
> While testing, I figured that de-initializing the DP PHY while
> is was powered-on by the DRM DP, caused the system to freeze and crash.
> 
> SO I've added this to track this state and try to de-init the DP phy
> if still in use.

If you are to send next iteration, please add these bits to the commit
message.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


-- 
With best wishes
Dmitry

Re: [PATCH v2 0/7] arm64: qcom: allow up to 4 lanes for the Type-C DisplayPort Altmode

[Luca Weiss]

On Mon May 27, 2024 at 10:42 AM CEST, Neil Armstrong wrote:
> Register a typec mux in order to change the PHY mode on the Type-C
> mux events depending on the mode and the svid when in Altmode setup.
>
> The DisplayPort phy should be left enabled if is still powered on
> by the DRM DisplayPort controller, so bail out until the DisplayPort
> PHY is not powered off.
>
> The Type-C Mode/SVID only changes on plug/unplug, and USB SAFE states
> will be set in between of USB-Only, Combo and DisplayPort Only so
> this will leave enough time to the DRM DisplayPort controller to
> turn of the DisplayPort PHY.
>
> The patchset also includes bindings changes and DT changes.
>
> This has been successfully tested on an SM8550 board, but the
> Thinkpad X13s deserved testing between non-PD USB, non-PD DisplayPort,
> PD USB Hubs and PD Altmode Dongles to make sure the switch works
> as expected.
>
> The DisplayPort 4 lanes setup can be check with:
> $ cat /sys/kernel/debug/dri/ae01000.display-controller/DP-1/dp_debug
> 	name = msm_dp
> 	drm_dp_link
> 		rate = 540000
> 		num_lanes = 4
> ...
>
> This patchset depends on [1] to allow broadcasting the type-c mode
> to the PHY, otherwise the PHY will keep the combo state while the
> retimer would setup the 4 lanes in DP mode.
>
> [1] https://lore.kernel.org/all/20240527-topic-sm8x50-upstream-retimer-broadcast-mode-v1-0-79ec91381aba@linaro.org/

Hi Neil,

Is there anything happening on this patchset? From what I can see there
were a few comments on the patches, would be nice if we could get this
in at some point.

Regards
Luca

>
> To: Bjorn Andersson <andersson@kernel.org>
> To: Konrad Dybcio <konrad.dybcio@linaro.org>
> To: Vinod Koul <vkoul@kernel.org>
> To: Kishon Vijay Abraham I <kishon@kernel.org>
> To: Rob Herring <robh@kernel.org>
> To: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> To: Conor Dooley <conor+dt@kernel.org>
> To: Krzysztof Kozlowski <krzk+dt@kernel.org>
> Cc: linux-arm-msm@vger.kernel.org
> Cc: linux-phy@lists.infradead.org
> Cc: devicetree@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>
> Changes in v2:
> - Reference usb-switch.yaml in bindings patch
> - Fix switch/case indenting
> - Check svid for USB_TYPEC_DP_SID
> - Fix X13s patch subject
> - Update SM8650 patch to enable 4 lanes on HDK aswell
> - Link to v1: https://lore.kernel.org/r/20240229-topic-sm8x50-upstream-phy-combo-typec-mux-v1-0-07e24a231840@linaro.org
>
> ---
> Neil Armstrong (7):
>       dt-bindings: phy: qcom,sc8280xp-qmp-usb43dp: Reference usb-switch.yaml to allow mode-switch
>       phy: qcom: qmp-combo: store DP phy power state
>       phy: qcom: qmp-combo: introduce QPHY_MODE
>       phy: qcom: qmp-combo: register a typec mux to change the QPHY_MODE
>       arm64: dts: qcom-sm8550: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch
>       arm64: dts: qcom-sm8650: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch
>       arm64: dts: qcom: sc8280xp-lenovo-thinkpad-x13: allow 4 lanes for DisplayPort and enable QMP PHY mode-switch
>
>  .../phy/qcom,sc8280xp-qmp-usb43dp-phy.yaml         |   7 +-
>  .../dts/qcom/sc8280xp-lenovo-thinkpad-x13s.dts     |   6 +-
>  arch/arm64/boot/dts/qcom/sm8550-hdk.dts            |   3 +-
>  arch/arm64/boot/dts/qcom/sm8550-qrd.dts            |   3 +-
>  arch/arm64/boot/dts/qcom/sm8650-hdk.dts            |   3 +-
>  arch/arm64/boot/dts/qcom/sm8650-qrd.dts            |   3 +-
>  drivers/phy/qualcomm/phy-qcom-qmp-combo.c          | 169 +++++++++++++++++++--
>  7 files changed, 174 insertions(+), 20 deletions(-)
> ---
> base-commit: d4eef8b2e18d3e4d2343fb3bb975f8ac4522129a
> change-id: 20240229-topic-sm8x50-upstream-phy-combo-typec-mux-31b5252513c9
>
> Best regards,

Re: [PATCH v2 3/7] phy: qcom: qmp-combo: introduce QPHY_MODE

[Udipto Goswami]

On 5/27/2024 2:12 PM, Neil Armstrong wrote:
> Introduce an enum for the QMP Combo PHY modes, use it in the
> QMP commmon phy init function and default to COMBO mode.
> 
> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
> ---
>   drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 41 +++++++++++++++++++++++++++----
>   1 file changed, 36 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 183cd9cd1884..788e4c05eaf2 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -61,6 +61,12 @@
>   
>   #define PHY_INIT_COMPLETE_TIMEOUT		10000
>   
> +enum qphy_mode {
> +	QPHY_MODE_COMBO = 0,

Hi Neil,

I have a doubt here, shouldn't this be aligned with what typec_altmode has ?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/usb/typec_altmode.h?h=v6.15#n113

This patch marks COMBO mode as 0
when the mux_set when called from pmic_glink_altmode.c
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/soc/qcom/pmic_glink_altmode.c?h=v6.15#n160

the state will start from 2 if i'm not wrong ?

For the similar implmentation I referring to fsa4480.c which seems to be 
using the enums from typec_altmode.c therefore asking.


Thanks,
-Udipto

> +	QPHY_MODE_DP_ONLY,
> +	QPHY_MODE_USB_ONLY,
> +};
> +
>   /* set of registers with offsets different per-PHY */
>   enum qphy_reg_layout {
>   	/* PCS registers */
> @@ -1503,6 +1509,7 @@ struct qmp_combo {
>   
>   	struct mutex phy_mutex;
>   	int init_count;
> +	enum qphy_mode init_mode;
>   
>   	struct phy *usb_phy;
>   	enum phy_mode mode;
> @@ -2589,12 +2596,33 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
>   	if (qmp->orientation == TYPEC_ORIENTATION_REVERSE)
>   		val |= SW_PORTSELECT_VAL;
>   	writel(val, com + QPHY_V3_DP_COM_TYPEC_CTRL);
> -	writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
>   
> -	/* bring both QMP USB and QMP DP PHYs PCS block out of reset */
> -	qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
> -			SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
> -			SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
> +	switch (qmp->init_mode) {
> +	case QPHY_MODE_COMBO:
> +		writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
> +
> +		/* bring both QMP USB and QMP DP PHYs PCS block out of reset */
> +		qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
> +				SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
> +				SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
> +		break;
> +
> +	case QPHY_MODE_DP_ONLY:
> +		writel(DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
> +
> +		/* bring QMP DP PHY PCS block out of reset */
> +		qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
> +				SW_DPPHY_RESET_MUX | SW_DPPHY_RESET);
> +		break;
> +
> +	case QPHY_MODE_USB_ONLY:
> +		writel(USB3_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
> +
> +		/* bring QMP USB PHY PCS block out of reset */
> +		qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
> +				SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
> +		break;
> +	}
>   
>   	qphy_clrbits(com, QPHY_V3_DP_COM_SWI_CTRL, 0x03);
>   	qphy_clrbits(com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);
> @@ -3603,6 +3631,9 @@ static int qmp_combo_probe(struct platform_device *pdev)
>   	if (ret)
>   		goto err_node_put;
>   
> +	/* Set PHY_MODE as combo by default */
> +	qmp->init_mode = QPHY_MODE_COMBO;
> +
>   	qmp->usb_phy = devm_phy_create(dev, usb_np, &qmp_combo_usb_phy_ops);
>   	if (IS_ERR(qmp->usb_phy)) {
>   		ret = PTR_ERR(qmp->usb_phy);
> 

Re: [PATCH v2 3/7] phy: qcom: qmp-combo: introduce QPHY_MODE

[Neil Armstrong]

On 04/06/2025 13:25, Udipto Goswami wrote:
> 
> 
> On 5/27/2024 2:12 PM, Neil Armstrong wrote:
>> Introduce an enum for the QMP Combo PHY modes, use it in the
>> QMP commmon phy init function and default to COMBO mode.
>>
>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>> ---
>>   drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 41 +++++++++++++++++++++++++++----
>>   1 file changed, 36 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> index 183cd9cd1884..788e4c05eaf2 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
>> @@ -61,6 +61,12 @@
>>   #define PHY_INIT_COMPLETE_TIMEOUT        10000
>> +enum qphy_mode {
>> +    QPHY_MODE_COMBO = 0,
> 
> Hi Neil,
> 
> I have a doubt here, shouldn't this be aligned with what typec_altmode has ?
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/linux/usb/typec_altmode.h?h=v6.15#n113
> 
> This patch marks COMBO mode as 0
> when the mux_set when called from pmic_glink_altmode.c
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/soc/qcom/pmic_glink_altmode.c?h=v6.15#n160
> 
> the state will start from 2 if i'm not wrong ?
> 
> For the similar implmentation I referring to fsa4480.c which seems to be using the enums from typec_altmode.c therefore asking.

Those enums are local to the driver, not related to altmode bits at all,
they represent the 3 possible states of the combo phy.

Neil

> 
> 
> Thanks,
> -Udipto
> 
>> +    QPHY_MODE_DP_ONLY,
>> +    QPHY_MODE_USB_ONLY,
>> +};
>> +
>>   /* set of registers with offsets different per-PHY */
>>   enum qphy_reg_layout {
>>       /* PCS registers */
>> @@ -1503,6 +1509,7 @@ struct qmp_combo {
>>       struct mutex phy_mutex;
>>       int init_count;
>> +    enum qphy_mode init_mode;
>>       struct phy *usb_phy;
>>       enum phy_mode mode;
>> @@ -2589,12 +2596,33 @@ static int qmp_combo_com_init(struct qmp_combo *qmp, bool force)
>>       if (qmp->orientation == TYPEC_ORIENTATION_REVERSE)
>>           val |= SW_PORTSELECT_VAL;
>>       writel(val, com + QPHY_V3_DP_COM_TYPEC_CTRL);
>> -    writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
>> -    /* bring both QMP USB and QMP DP PHYs PCS block out of reset */
>> -    qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
>> -            SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
>> -            SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
>> +    switch (qmp->init_mode) {
>> +    case QPHY_MODE_COMBO:
>> +        writel(USB3_MODE | DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
>> +
>> +        /* bring both QMP USB and QMP DP PHYs PCS block out of reset */
>> +        qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
>> +                SW_DPPHY_RESET_MUX | SW_DPPHY_RESET |
>> +                SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
>> +        break;
>> +
>> +    case QPHY_MODE_DP_ONLY:
>> +        writel(DP_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
>> +
>> +        /* bring QMP DP PHY PCS block out of reset */
>> +        qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
>> +                SW_DPPHY_RESET_MUX | SW_DPPHY_RESET);
>> +        break;
>> +
>> +    case QPHY_MODE_USB_ONLY:
>> +        writel(USB3_MODE, com + QPHY_V3_DP_COM_PHY_MODE_CTRL);
>> +
>> +        /* bring QMP USB PHY PCS block out of reset */
>> +        qphy_clrbits(com, QPHY_V3_DP_COM_RESET_OVRD_CTRL,
>> +                SW_USB3PHY_RESET_MUX | SW_USB3PHY_RESET);
>> +        break;
>> +    }
>>       qphy_clrbits(com, QPHY_V3_DP_COM_SWI_CTRL, 0x03);
>>       qphy_clrbits(com, QPHY_V3_DP_COM_SW_RESET, SW_RESET);
>> @@ -3603,6 +3631,9 @@ static int qmp_combo_probe(struct platform_device *pdev)
>>       if (ret)
>>           goto err_node_put;
>> +    /* Set PHY_MODE as combo by default */
>> +    qmp->init_mode = QPHY_MODE_COMBO;
>> +
>>       qmp->usb_phy = devm_phy_create(dev, usb_np, &qmp_combo_usb_phy_ops);
>>       if (IS_ERR(qmp->usb_phy)) {
>>           ret = PTR_ERR(qmp->usb_phy);
>>
>