Complete USB/DP switchable PHY integration by adding DP clock
registration, aux bridge setup, and DT parsing. Implement clock
provider logic for USB and DP branches, and extend PHY translation
to support both USB and DP instances.

Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 331 ++++++++++++++++++++++++++++---
 1 file changed, 299 insertions(+), 32 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
index 821398653bef23e1915d9d3a3a2950b0bfbefb9a..74b9f75c8864efe270f394bfbfd748793dada1f5 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
@@ -995,6 +995,11 @@ static int qmp_usbc_usb_power_on(struct phy *phy)
 	qmp_configure(qmp->dev, qmp->serdes, cfg->serdes_tbl,
 		      cfg->serdes_tbl_num);
 
+	if (IS_ERR(qmp->pipe_clk)) {
+		return dev_err_probe(qmp->dev, PTR_ERR(qmp->pipe_clk),
+				     "pipe clock not defined\n");
+	}
+
 	ret = clk_prepare_enable(qmp->pipe_clk);
 	if (ret) {
 		dev_err(qmp->dev, "pipe_clk enable failed err=%d\n", ret);
@@ -1365,11 +1370,13 @@ static int __maybe_unused qmp_usbc_runtime_resume(struct device *dev)
 	if (ret)
 		return ret;
 
-	ret = clk_prepare_enable(qmp->pipe_clk);
-	if (ret) {
-		dev_err(dev, "pipe_clk enable failed, err=%d\n", ret);
-		clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
-		return ret;
+	if (!IS_ERR(qmp->pipe_clk)) {
+		ret = clk_prepare_enable(qmp->pipe_clk);
+		if (ret) {
+			dev_err(dev, "pipe_clk enable failed, err=%d\n", ret);
+			clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
+			return ret;
+		}
 	}
 
 	qmp_usbc_disable_autonomous_mode(qmp);
@@ -1422,9 +1429,23 @@ static int qmp_usbc_clk_init(struct qmp_usbc *qmp)
 	return devm_clk_bulk_get_optional(dev, num, qmp->clks);
 }
 
-static void phy_clk_release_provider(void *res)
+static struct clk_hw *qmp_usbc_clks_hw_get(struct of_phandle_args *clkspec, void *data)
 {
-	of_clk_del_provider(res);
+	struct qmp_usbc *qmp = data;
+
+	if (clkspec->args_count == 0)
+		return &qmp->pipe_clk_fixed.hw;
+
+	switch (clkspec->args[0]) {
+	case QMP_USB43DP_USB3_PIPE_CLK:
+		return &qmp->pipe_clk_fixed.hw;
+	case QMP_USB43DP_DP_LINK_CLK:
+		return &qmp->dp_link_hw;
+	case QMP_USB43DP_DP_VCO_DIV_CLK:
+		return &qmp->dp_pixel_hw;
+	}
+
+	return ERR_PTR(-EINVAL);
 }
 
 /*
@@ -1453,8 +1474,11 @@ static int phy_pipe_clk_register(struct qmp_usbc *qmp, struct device_node *np)
 
 	ret = of_property_read_string(np, "clock-output-names", &init.name);
 	if (ret) {
-		dev_err(qmp->dev, "%pOFn: No clock-output-names\n", np);
-		return ret;
+		char name[64];
+
+		/* Clock name is not mandatory. */
+		snprintf(name, sizeof(name), "%s::pipe_clk", dev_name(qmp->dev));
+		init.name = name;
 	}
 
 	init.ops = &clk_fixed_rate_ops;
@@ -1463,19 +1487,7 @@ static int phy_pipe_clk_register(struct qmp_usbc *qmp, struct device_node *np)
 	fixed->fixed_rate = 125000000;
 	fixed->hw.init = &init;
 
-	ret = devm_clk_hw_register(qmp->dev, &fixed->hw);
-	if (ret)
-		return ret;
-
-	ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &fixed->hw);
-	if (ret)
-		return ret;
-
-	/*
-	 * Roll a devm action because the clock provider is the child node, but
-	 * the child node is not actually a device.
-	 */
-	return devm_add_action_or_reset(qmp->dev, phy_clk_release_provider, np);
+	return devm_clk_hw_register(qmp->dev, &fixed->hw);
 }
 
 #if IS_ENABLED(CONFIG_TYPEC)
@@ -1660,6 +1672,235 @@ static int qmp_usbc_parse_tcsr(struct qmp_usbc *qmp)
 	return 0;
 }
 
+static int qmp_usbc_parse_usb3dp_dt(struct qmp_usbc *qmp)
+{
+	struct platform_device *pdev = to_platform_device(qmp->dev);
+	const struct qmp_phy_cfg *cfg = qmp->cfg;
+	const struct qmp_usbc_offsets *offs = cfg->offsets;
+	struct device *dev = qmp->dev;
+	void __iomem *base;
+	int ret;
+
+	base = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(base))
+		return PTR_ERR(base);
+
+	qmp->dp_serdes = base + offs->dp_serdes;
+	qmp->dp_tx = base + offs->dp_txa;
+	qmp->dp_tx2 = base + offs->dp_txb;
+	qmp->dp_dp_phy = base + offs->dp_dp_phy;
+	qmp->serdes = base + offs->serdes;
+	qmp->pcs = base + offs->pcs;
+	if (offs->pcs_misc)
+		qmp->pcs_misc = base + offs->pcs_misc;
+	qmp->tx = base + offs->tx;
+	qmp->rx = base + offs->rx;
+
+	qmp->tx2 = base + offs->tx2;
+	qmp->rx2 = base + offs->rx2;
+
+	ret = qmp_usbc_clk_init(qmp);
+	if (ret)
+		return ret;
+
+	qmp->pipe_clk = devm_clk_get(dev, "pipe");
+	if (IS_ERR(qmp->pipe_clk)) {
+		/* usb3dp allow no pipe clk define */
+		if (cfg->type == QMP_PHY_USBC_USB3_ONLY)
+			return dev_err_probe(dev, PTR_ERR(qmp->pipe_clk),
+						"failed to get pipe clock\n");
+	}
+
+	return 0;
+}
+
+/*
+ * Display Port PLL driver block diagram for branch clocks
+ *
+ *              +------------------------------+
+ *              |         DP_VCO_CLK           |
+ *              |                              |
+ *              |    +-------------------+     |
+ *              |    |   (DP PLL/VCO)    |     |
+ *              |    +---------+---------+     |
+ *              |              v               |
+ *              |   +----------+-----------+   |
+ *              |   | hsclk_divsel_clk_src |   |
+ *              |   +----------+-----------+   |
+ *              +------------------------------+
+ *                              |
+ *          +---------<---------v------------>----------+
+ *          |                                           |
+ * +--------v----------------+                          |
+ * |    dp_phy_pll_link_clk  |                          |
+ * |     link_clk            |                          |
+ * +--------+----------------+                          |
+ *          |                                           |
+ *          |                                           |
+ *          v                                           v
+ * Input to DISPCC block                                |
+ * for link clk, crypto clk                             |
+ * and interface clock                                  |
+ *                                                      |
+ *                                                      |
+ *      +--------<------------+-----------------+---<---+
+ *      |                     |                 |
+ * +----v---------+  +--------v-----+  +--------v------+
+ * | vco_divided  |  | vco_divided  |  | vco_divided   |
+ * |    _clk_src  |  |    _clk_src  |  |    _clk_src   |
+ * |              |  |              |  |               |
+ * |divsel_six    |  |  divsel_two  |  |  divsel_four  |
+ * +-------+------+  +-----+--------+  +--------+------+
+ *         |                 |                  |
+ *         v---->----------v-------------<------v
+ *                         |
+ *              +----------+-----------------+
+ *              |   dp_phy_pll_vco_div_clk   |
+ *              +---------+------------------+
+ *                        |
+ *                        v
+ *              Input to DISPCC block
+ *              for DP pixel clock
+ *
+ */
+static int qmp_dp_pixel_clk_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
+{
+	switch (req->rate) {
+	case 1620000000UL / 2:
+	case 2700000000UL / 2:
+	/* 5.4 and 8.1 GHz are same link rate as 2.7GHz, i.e. div 4 and div 6 */
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static unsigned long qmp_dp_pixel_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+	const struct qmp_usbc *qmp;
+	const struct phy_configure_opts_dp *dp_opts;
+
+	qmp = container_of(hw, struct qmp_usbc, dp_pixel_hw);
+
+	dp_opts = &qmp->dp_opts;
+
+	switch (dp_opts->link_rate) {
+	case 1620:
+		return 1620000000UL / 2;
+	case 2700:
+		return 2700000000UL / 2;
+	case 5400:
+		return 5400000000UL / 4;
+	default:
+		return 0;
+	}
+}
+
+static const struct clk_ops qmp_dp_pixel_clk_ops = {
+	.determine_rate	= qmp_dp_pixel_clk_determine_rate,
+	.recalc_rate	= qmp_dp_pixel_clk_recalc_rate,
+};
+
+static int qmp_dp_link_clk_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
+{
+	switch (req->rate) {
+	case 162000000:
+	case 270000000:
+	case 540000000:
+		return 0;
+	default:
+		return -EINVAL;
+	}
+}
+
+static unsigned long qmp_dp_link_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
+{
+	const struct qmp_usbc *qmp;
+	const struct phy_configure_opts_dp *dp_opts;
+
+	qmp = container_of(hw, struct qmp_usbc, dp_link_hw);
+	dp_opts = &qmp->dp_opts;
+
+	switch (dp_opts->link_rate) {
+	case 1620:
+	case 2700:
+	case 5400:
+		return dp_opts->link_rate * 100000;
+	default:
+		return 0;
+	}
+}
+
+static const struct clk_ops qmp_dp_link_clk_ops = {
+	.determine_rate	= qmp_dp_link_clk_determine_rate,
+	.recalc_rate	= qmp_dp_link_clk_recalc_rate,
+};
+
+static int phy_dp_clks_register(struct qmp_usbc *qmp, struct device_node *np)
+{
+	struct clk_init_data init = { };
+	char name[64];
+	int ret;
+
+	snprintf(name, sizeof(name), "%s::link_clk", dev_name(qmp->dev));
+	init.ops = &qmp_dp_link_clk_ops;
+	init.name = name;
+	qmp->dp_link_hw.init = &init;
+	ret = devm_clk_hw_register(qmp->dev, &qmp->dp_link_hw);
+	if (ret < 0) {
+		dev_err(qmp->dev, "link clk reg fail ret=%d\n", ret);
+		return ret;
+	}
+
+	snprintf(name, sizeof(name), "%s::vco_div_clk", dev_name(qmp->dev));
+	init.ops = &qmp_dp_pixel_clk_ops;
+	init.name = name;
+	qmp->dp_pixel_hw.init = &init;
+	ret = devm_clk_hw_register(qmp->dev, &qmp->dp_pixel_hw);
+	if (ret) {
+		dev_err(qmp->dev, "pxl clk reg fail ret=%d\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+
+static int qmp_usbc_register_clocks(struct qmp_usbc *qmp, struct device_node *np)
+{
+	int ret;
+
+	if (!IS_ERR(qmp->pipe_clk)) {
+		ret = phy_pipe_clk_register(qmp, np);
+		if (ret)
+			return ret;
+	}
+
+	if (qmp->cfg->type == QMP_PHY_USBC_USB3_DP) {
+		ret = phy_dp_clks_register(qmp, np);
+		if (ret)
+			return ret;
+	}
+
+	return devm_of_clk_add_hw_provider(qmp->dev, qmp_usbc_clks_hw_get, qmp);
+}
+
+static struct phy *qmp_usbc_phy_xlate(struct device *dev, const struct of_phandle_args *args)
+{
+	struct qmp_usbc *qmp = dev_get_drvdata(dev);
+
+	if (args->args_count == 0)
+		return qmp->usb_phy;
+
+	switch (args->args[0]) {
+	case QMP_USB43DP_USB3_PHY:
+		return qmp->usb_phy;
+	case QMP_USB43DP_DP_PHY:
+		return qmp->dp_phy;
+	}
+
+	return ERR_PTR(-EINVAL);
+}
+
 static int qmp_usbc_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
@@ -1703,16 +1944,32 @@ static int qmp_usbc_probe(struct platform_device *pdev)
 	if (ret)
 		return ret;
 
-	/* Check for legacy binding with child node. */
-	np = of_get_child_by_name(dev->of_node, "phy");
-	if (np) {
-		ret = qmp_usbc_parse_usb_dt_legacy(qmp, np);
-	} else {
+	if (qmp->cfg->type == QMP_PHY_USBC_USB3_DP) {
 		np = of_node_get(dev->of_node);
-		ret = qmp_usbc_parse_usb_dt(qmp);
+
+		ret = qmp_usbc_parse_usb3dp_dt(qmp);
+		if (ret) {
+			dev_err(qmp->dev, "parse DP dt fail ret=%d\n", ret);
+			goto err_node_put;
+		}
+
+		ret = drm_aux_bridge_register(dev);
+		if (ret) {
+			dev_err(qmp->dev, "aux bridge reg fail ret=%d\n", ret);
+			goto err_node_put;
+		}
+	} else {
+		/* Check for legacy binding with child node. */
+		np = of_get_child_by_name(dev->of_node, "phy");
+		if (np) {
+			ret = qmp_usbc_parse_usb_dt_legacy(qmp, np);
+		} else {
+			np = of_node_get(dev->of_node);
+			ret = qmp_usbc_parse_usb_dt(qmp);
+		}
+		if (ret)
+			goto err_node_put;
 	}
-	if (ret)
-		goto err_node_put;
 
 	pm_runtime_set_active(dev);
 	ret = devm_pm_runtime_enable(dev);
@@ -1724,7 +1981,7 @@ static int qmp_usbc_probe(struct platform_device *pdev)
 	 */
 	pm_runtime_forbid(dev);
 
-	ret = phy_pipe_clk_register(qmp, np);
+	ret = qmp_usbc_register_clocks(qmp, np);
 	if (ret)
 		goto err_node_put;
 
@@ -1737,9 +1994,19 @@ static int qmp_usbc_probe(struct platform_device *pdev)
 
 	phy_set_drvdata(qmp->usb_phy, qmp);
 
+	if (qmp->cfg->type == QMP_PHY_USBC_USB3_DP) {
+		qmp->dp_phy = devm_phy_create(dev, np, &qmp_usbc_dp_phy_ops);
+		if (IS_ERR(qmp->dp_phy)) {
+			ret = PTR_ERR(qmp->dp_phy);
+			dev_err(dev, "failed to create PHY: %d\n", ret);
+			goto err_node_put;
+		}
+		phy_set_drvdata(qmp->dp_phy, qmp);
+	}
+
 	of_node_put(np);
 
-	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
+	phy_provider = devm_of_phy_provider_register(dev, qmp_usbc_phy_xlate);
 
 	return PTR_ERR_OR_ZERO(phy_provider);
 

-- 
2.34.1

On Wed, Aug 20, 2025 at 05:34:53PM +0800, Xiangxu Yin wrote:
> Complete USB/DP switchable PHY integration by adding DP clock
> registration, aux bridge setup, and DT parsing. Implement clock
> provider logic for USB and DP branches, and extend PHY translation
> to support both USB and DP instances.
> 
> Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 331 ++++++++++++++++++++++++++++---
>  1 file changed, 299 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> index 821398653bef23e1915d9d3a3a2950b0bfbefb9a..74b9f75c8864efe270f394bfbfd748793dada1f5 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
> @@ -995,6 +995,11 @@ static int qmp_usbc_usb_power_on(struct phy *phy)
>  	qmp_configure(qmp->dev, qmp->serdes, cfg->serdes_tbl,
>  		      cfg->serdes_tbl_num);
>  
> +	if (IS_ERR(qmp->pipe_clk)) {
> +		return dev_err_probe(qmp->dev, PTR_ERR(qmp->pipe_clk),
> +				     "pipe clock not defined\n");
> +	}

No, this should not be allowed.

> +
>  	ret = clk_prepare_enable(qmp->pipe_clk);
>  	if (ret) {
>  		dev_err(qmp->dev, "pipe_clk enable failed err=%d\n", ret);
> @@ -1365,11 +1370,13 @@ static int __maybe_unused qmp_usbc_runtime_resume(struct device *dev)
>  	if (ret)
>  		return ret;
>  
> -	ret = clk_prepare_enable(qmp->pipe_clk);
> -	if (ret) {
> -		dev_err(dev, "pipe_clk enable failed, err=%d\n", ret);
> -		clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
> -		return ret;
> +	if (!IS_ERR(qmp->pipe_clk)) {

Similarly.

> +		ret = clk_prepare_enable(qmp->pipe_clk);
> +		if (ret) {
> +			dev_err(dev, "pipe_clk enable failed, err=%d\n", ret);
> +			clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
> +			return ret;
> +		}
>  	}
>  
>  	qmp_usbc_disable_autonomous_mode(qmp);
> @@ -1422,9 +1429,23 @@ static int qmp_usbc_clk_init(struct qmp_usbc *qmp)
>  	return devm_clk_bulk_get_optional(dev, num, qmp->clks);
>  }
>  
> -static void phy_clk_release_provider(void *res)
> +static struct clk_hw *qmp_usbc_clks_hw_get(struct of_phandle_args *clkspec, void *data)
>  {
> -	of_clk_del_provider(res);
> +	struct qmp_usbc *qmp = data;
> +
> +	if (clkspec->args_count == 0)
> +		return &qmp->pipe_clk_fixed.hw;
> +
> +	switch (clkspec->args[0]) {
> +	case QMP_USB43DP_USB3_PIPE_CLK:
> +		return &qmp->pipe_clk_fixed.hw;
> +	case QMP_USB43DP_DP_LINK_CLK:
> +		return &qmp->dp_link_hw;
> +	case QMP_USB43DP_DP_VCO_DIV_CLK:
> +		return &qmp->dp_pixel_hw;
> +	}
> +
> +	return ERR_PTR(-EINVAL);
>  }
>  
>  /*
> @@ -1453,8 +1474,11 @@ static int phy_pipe_clk_register(struct qmp_usbc *qmp, struct device_node *np)
>  
>  	ret = of_property_read_string(np, "clock-output-names", &init.name);
>  	if (ret) {
> -		dev_err(qmp->dev, "%pOFn: No clock-output-names\n", np);
> -		return ret;
> +		char name[64];
> +
> +		/* Clock name is not mandatory. */
> +		snprintf(name, sizeof(name), "%s::pipe_clk", dev_name(qmp->dev));
> +		init.name = name;
>  	}
>  
>  	init.ops = &clk_fixed_rate_ops;
> @@ -1463,19 +1487,7 @@ static int phy_pipe_clk_register(struct qmp_usbc *qmp, struct device_node *np)
>  	fixed->fixed_rate = 125000000;
>  	fixed->hw.init = &init;
>  
> -	ret = devm_clk_hw_register(qmp->dev, &fixed->hw);
> -	if (ret)
> -		return ret;
> -
> -	ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &fixed->hw);
> -	if (ret)
> -		return ret;
> -
> -	/*
> -	 * Roll a devm action because the clock provider is the child node, but
> -	 * the child node is not actually a device.
> -	 */
> -	return devm_add_action_or_reset(qmp->dev, phy_clk_release_provider, np);
> +	return devm_clk_hw_register(qmp->dev, &fixed->hw);
>  }
>  
>  #if IS_ENABLED(CONFIG_TYPEC)
> @@ -1660,6 +1672,235 @@ static int qmp_usbc_parse_tcsr(struct qmp_usbc *qmp)
>  	return 0;
>  }
>  
> +static int qmp_usbc_parse_usb3dp_dt(struct qmp_usbc *qmp)
> +{
> +	struct platform_device *pdev = to_platform_device(qmp->dev);
> +	const struct qmp_phy_cfg *cfg = qmp->cfg;
> +	const struct qmp_usbc_offsets *offs = cfg->offsets;
> +	struct device *dev = qmp->dev;
> +	void __iomem *base;
> +	int ret;
> +
> +	base = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(base))
> +		return PTR_ERR(base);
> +
> +	qmp->dp_serdes = base + offs->dp_serdes;
> +	qmp->dp_tx = base + offs->dp_txa;
> +	qmp->dp_tx2 = base + offs->dp_txb;
> +	qmp->dp_dp_phy = base + offs->dp_dp_phy;

Squash this into qmp_usbc_parse_dt(). Set these fields if
dp_serdes != 0.

> +	qmp->serdes = base + offs->serdes;
> +	qmp->pcs = base + offs->pcs;
> +	if (offs->pcs_misc)
> +		qmp->pcs_misc = base + offs->pcs_misc;
> +	qmp->tx = base + offs->tx;
> +	qmp->rx = base + offs->rx;
> +
> +	qmp->tx2 = base + offs->tx2;
> +	qmp->rx2 = base + offs->rx2;
> +
> +	ret = qmp_usbc_clk_init(qmp);
> +	if (ret)
> +		return ret;
> +
> +	qmp->pipe_clk = devm_clk_get(dev, "pipe");
> +	if (IS_ERR(qmp->pipe_clk)) {
> +		/* usb3dp allow no pipe clk define */
> +		if (cfg->type == QMP_PHY_USBC_USB3_ONLY)
> +			return dev_err_probe(dev, PTR_ERR(qmp->pipe_clk),
> +						"failed to get pipe clock\n");
> +	}
> +
> +	return 0;
> +}
> +
> +/*
> + * Display Port PLL driver block diagram for branch clocks
> + *
> + *              +------------------------------+
> + *              |         DP_VCO_CLK           |
> + *              |                              |
> + *              |    +-------------------+     |
> + *              |    |   (DP PLL/VCO)    |     |
> + *              |    +---------+---------+     |
> + *              |              v               |
> + *              |   +----------+-----------+   |
> + *              |   | hsclk_divsel_clk_src |   |
> + *              |   +----------+-----------+   |
> + *              +------------------------------+
> + *                              |
> + *          +---------<---------v------------>----------+
> + *          |                                           |
> + * +--------v----------------+                          |
> + * |    dp_phy_pll_link_clk  |                          |
> + * |     link_clk            |                          |
> + * +--------+----------------+                          |
> + *          |                                           |
> + *          |                                           |
> + *          v                                           v
> + * Input to DISPCC block                                |
> + * for link clk, crypto clk                             |
> + * and interface clock                                  |
> + *                                                      |
> + *                                                      |
> + *      +--------<------------+-----------------+---<---+
> + *      |                     |                 |
> + * +----v---------+  +--------v-----+  +--------v------+
> + * | vco_divided  |  | vco_divided  |  | vco_divided   |
> + * |    _clk_src  |  |    _clk_src  |  |    _clk_src   |
> + * |              |  |              |  |               |
> + * |divsel_six    |  |  divsel_two  |  |  divsel_four  |
> + * +-------+------+  +-----+--------+  +--------+------+
> + *         |                 |                  |
> + *         v---->----------v-------------<------v
> + *                         |
> + *              +----------+-----------------+
> + *              |   dp_phy_pll_vco_div_clk   |
> + *              +---------+------------------+
> + *                        |
> + *                        v
> + *              Input to DISPCC block
> + *              for DP pixel clock
> + *
> + */
> +static int qmp_dp_pixel_clk_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
> +{
> +	switch (req->rate) {
> +	case 1620000000UL / 2:
> +	case 2700000000UL / 2:
> +	/* 5.4 and 8.1 GHz are same link rate as 2.7GHz, i.e. div 4 and div 6 */
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static unsigned long qmp_dp_pixel_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
> +{
> +	const struct qmp_usbc *qmp;
> +	const struct phy_configure_opts_dp *dp_opts;
> +
> +	qmp = container_of(hw, struct qmp_usbc, dp_pixel_hw);
> +
> +	dp_opts = &qmp->dp_opts;
> +
> +	switch (dp_opts->link_rate) {
> +	case 1620:
> +		return 1620000000UL / 2;
> +	case 2700:
> +		return 2700000000UL / 2;
> +	case 5400:
> +		return 5400000000UL / 4;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static const struct clk_ops qmp_dp_pixel_clk_ops = {
> +	.determine_rate	= qmp_dp_pixel_clk_determine_rate,
> +	.recalc_rate	= qmp_dp_pixel_clk_recalc_rate,
> +};
> +
> +static int qmp_dp_link_clk_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
> +{
> +	switch (req->rate) {
> +	case 162000000:
> +	case 270000000:
> +	case 540000000:
> +		return 0;
> +	default:
> +		return -EINVAL;
> +	}
> +}
> +
> +static unsigned long qmp_dp_link_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
> +{
> +	const struct qmp_usbc *qmp;
> +	const struct phy_configure_opts_dp *dp_opts;
> +
> +	qmp = container_of(hw, struct qmp_usbc, dp_link_hw);
> +	dp_opts = &qmp->dp_opts;
> +
> +	switch (dp_opts->link_rate) {
> +	case 1620:
> +	case 2700:
> +	case 5400:
> +		return dp_opts->link_rate * 100000;
> +	default:
> +		return 0;
> +	}
> +}
> +
> +static const struct clk_ops qmp_dp_link_clk_ops = {
> +	.determine_rate	= qmp_dp_link_clk_determine_rate,
> +	.recalc_rate	= qmp_dp_link_clk_recalc_rate,
> +};
> +
> +static int phy_dp_clks_register(struct qmp_usbc *qmp, struct device_node *np)
> +{
> +	struct clk_init_data init = { };
> +	char name[64];
> +	int ret;
> +
> +	snprintf(name, sizeof(name), "%s::link_clk", dev_name(qmp->dev));
> +	init.ops = &qmp_dp_link_clk_ops;
> +	init.name = name;
> +	qmp->dp_link_hw.init = &init;
> +	ret = devm_clk_hw_register(qmp->dev, &qmp->dp_link_hw);
> +	if (ret < 0) {
> +		dev_err(qmp->dev, "link clk reg fail ret=%d\n", ret);
> +		return ret;
> +	}
> +
> +	snprintf(name, sizeof(name), "%s::vco_div_clk", dev_name(qmp->dev));
> +	init.ops = &qmp_dp_pixel_clk_ops;
> +	init.name = name;
> +	qmp->dp_pixel_hw.init = &init;
> +	ret = devm_clk_hw_register(qmp->dev, &qmp->dp_pixel_hw);
> +	if (ret) {
> +		dev_err(qmp->dev, "pxl clk reg fail ret=%d\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int qmp_usbc_register_clocks(struct qmp_usbc *qmp, struct device_node *np)
> +{
> +	int ret;
> +
> +	if (!IS_ERR(qmp->pipe_clk)) {
> +		ret = phy_pipe_clk_register(qmp, np);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	if (qmp->cfg->type == QMP_PHY_USBC_USB3_DP) {

if dp_serdes != 0

> +		ret = phy_dp_clks_register(qmp, np);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return devm_of_clk_add_hw_provider(qmp->dev, qmp_usbc_clks_hw_get, qmp);
> +}
> +
> +static struct phy *qmp_usbc_phy_xlate(struct device *dev, const struct of_phandle_args *args)
> +{
> +	struct qmp_usbc *qmp = dev_get_drvdata(dev);
> +
> +	if (args->args_count == 0)
> +		return qmp->usb_phy;
> +
> +	switch (args->args[0]) {
> +	case QMP_USB43DP_USB3_PHY:
> +		return qmp->usb_phy;
> +	case QMP_USB43DP_DP_PHY:
> +		return qmp->dp_phy;
> +	}
> +
> +	return ERR_PTR(-EINVAL);
> +}
> +
>  static int qmp_usbc_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> @@ -1703,16 +1944,32 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>  	if (ret)
>  		return ret;
>  
> -	/* Check for legacy binding with child node. */
> -	np = of_get_child_by_name(dev->of_node, "phy");
> -	if (np) {
> -		ret = qmp_usbc_parse_usb_dt_legacy(qmp, np);
> -	} else {
> +	if (qmp->cfg->type == QMP_PHY_USBC_USB3_DP) {

Should not be necessary.

>  		np = of_node_get(dev->of_node);
> -		ret = qmp_usbc_parse_usb_dt(qmp);
> +
> +		ret = qmp_usbc_parse_usb3dp_dt(qmp);
> +		if (ret) {
> +			dev_err(qmp->dev, "parse DP dt fail ret=%d\n", ret);
> +			goto err_node_put;
> +		}
> +
> +		ret = drm_aux_bridge_register(dev);
> +		if (ret) {
> +			dev_err(qmp->dev, "aux bridge reg fail ret=%d\n", ret);
> +			goto err_node_put;
> +		}
> +	} else {
> +		/* Check for legacy binding with child node. */
> +		np = of_get_child_by_name(dev->of_node, "phy");
> +		if (np) {
> +			ret = qmp_usbc_parse_usb_dt_legacy(qmp, np);
> +		} else {
> +			np = of_node_get(dev->of_node);
> +			ret = qmp_usbc_parse_usb_dt(qmp);
> +		}
> +		if (ret)
> +			goto err_node_put;
>  	}
> -	if (ret)
> -		goto err_node_put;
>  
>  	pm_runtime_set_active(dev);
>  	ret = devm_pm_runtime_enable(dev);
> @@ -1724,7 +1981,7 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>  	 */
>  	pm_runtime_forbid(dev);
>  
> -	ret = phy_pipe_clk_register(qmp, np);
> +	ret = qmp_usbc_register_clocks(qmp, np);
>  	if (ret)
>  		goto err_node_put;
>  
> @@ -1737,9 +1994,19 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>  
>  	phy_set_drvdata(qmp->usb_phy, qmp);
>  
> +	if (qmp->cfg->type == QMP_PHY_USBC_USB3_DP) {

if dp_serdes != 0

> +		qmp->dp_phy = devm_phy_create(dev, np, &qmp_usbc_dp_phy_ops);
> +		if (IS_ERR(qmp->dp_phy)) {
> +			ret = PTR_ERR(qmp->dp_phy);
> +			dev_err(dev, "failed to create PHY: %d\n", ret);
> +			goto err_node_put;
> +		}
> +		phy_set_drvdata(qmp->dp_phy, qmp);
> +	}
> +
>  	of_node_put(np);
>  
> -	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
> +	phy_provider = devm_of_phy_provider_register(dev, qmp_usbc_phy_xlate);
>  
>  	return PTR_ERR_OR_ZERO(phy_provider);
>  
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

On 8/20/2025 7:42 PM, Dmitry Baryshkov wrote:
> On Wed, Aug 20, 2025 at 05:34:53PM +0800, Xiangxu Yin wrote:
>> Complete USB/DP switchable PHY integration by adding DP clock
>> registration, aux bridge setup, and DT parsing. Implement clock
>> provider logic for USB and DP branches, and extend PHY translation
>> to support both USB and DP instances.
>>
>> Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
>> ---
>>  drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 331 ++++++++++++++++++++++++++++---
>>  1 file changed, 299 insertions(+), 32 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
>> index 821398653bef23e1915d9d3a3a2950b0bfbefb9a..74b9f75c8864efe270f394bfbfd748793dada1f5 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
>> @@ -995,6 +995,11 @@ static int qmp_usbc_usb_power_on(struct phy *phy)
>>  	qmp_configure(qmp->dev, qmp->serdes, cfg->serdes_tbl,
>>  		      cfg->serdes_tbl_num);
>>  
>> +	if (IS_ERR(qmp->pipe_clk)) {
>> +		return dev_err_probe(qmp->dev, PTR_ERR(qmp->pipe_clk),
>> +				     "pipe clock not defined\n");
>> +	}
> No, this should not be allowed.


Ok, will fix in next patch.


>> +
>>  	ret = clk_prepare_enable(qmp->pipe_clk);
>>  	if (ret) {
>>  		dev_err(qmp->dev, "pipe_clk enable failed err=%d\n", ret);
>> @@ -1365,11 +1370,13 @@ static int __maybe_unused qmp_usbc_runtime_resume(struct device *dev)
>>  	if (ret)
>>  		return ret;
>>  
>> -	ret = clk_prepare_enable(qmp->pipe_clk);
>> -	if (ret) {
>> -		dev_err(dev, "pipe_clk enable failed, err=%d\n", ret);
>> -		clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
>> -		return ret;
>> +	if (!IS_ERR(qmp->pipe_clk)) {
> Similarly.


Ack.


>> +		ret = clk_prepare_enable(qmp->pipe_clk);
>> +		if (ret) {
>> +			dev_err(dev, "pipe_clk enable failed, err=%d\n", ret);
>> +			clk_bulk_disable_unprepare(qmp->num_clks, qmp->clks);
>> +			return ret;
>> +		}
>>  	}
>>  
>>  	qmp_usbc_disable_autonomous_mode(qmp);
>> @@ -1422,9 +1429,23 @@ static int qmp_usbc_clk_init(struct qmp_usbc *qmp)
>>  	return devm_clk_bulk_get_optional(dev, num, qmp->clks);
>>  }
>>  
>> -static void phy_clk_release_provider(void *res)
>> +static struct clk_hw *qmp_usbc_clks_hw_get(struct of_phandle_args *clkspec, void *data)
>>  {
>> -	of_clk_del_provider(res);
>> +	struct qmp_usbc *qmp = data;
>> +
>> +	if (clkspec->args_count == 0)
>> +		return &qmp->pipe_clk_fixed.hw;
>> +
>> +	switch (clkspec->args[0]) {
>> +	case QMP_USB43DP_USB3_PIPE_CLK:
>> +		return &qmp->pipe_clk_fixed.hw;
>> +	case QMP_USB43DP_DP_LINK_CLK:
>> +		return &qmp->dp_link_hw;
>> +	case QMP_USB43DP_DP_VCO_DIV_CLK:
>> +		return &qmp->dp_pixel_hw;
>> +	}
>> +
>> +	return ERR_PTR(-EINVAL);
>>  }
>>  
>>  /*
>> @@ -1453,8 +1474,11 @@ static int phy_pipe_clk_register(struct qmp_usbc *qmp, struct device_node *np)
>>  
>>  	ret = of_property_read_string(np, "clock-output-names", &init.name);
>>  	if (ret) {
>> -		dev_err(qmp->dev, "%pOFn: No clock-output-names\n", np);
>> -		return ret;
>> +		char name[64];
>> +
>> +		/* Clock name is not mandatory. */
>> +		snprintf(name, sizeof(name), "%s::pipe_clk", dev_name(qmp->dev));
>> +		init.name = name;
>>  	}
>>  
>>  	init.ops = &clk_fixed_rate_ops;
>> @@ -1463,19 +1487,7 @@ static int phy_pipe_clk_register(struct qmp_usbc *qmp, struct device_node *np)
>>  	fixed->fixed_rate = 125000000;
>>  	fixed->hw.init = &init;
>>  
>> -	ret = devm_clk_hw_register(qmp->dev, &fixed->hw);
>> -	if (ret)
>> -		return ret;
>> -
>> -	ret = of_clk_add_hw_provider(np, of_clk_hw_simple_get, &fixed->hw);
>> -	if (ret)
>> -		return ret;
>> -
>> -	/*
>> -	 * Roll a devm action because the clock provider is the child node, but
>> -	 * the child node is not actually a device.
>> -	 */
>> -	return devm_add_action_or_reset(qmp->dev, phy_clk_release_provider, np);
>> +	return devm_clk_hw_register(qmp->dev, &fixed->hw);
>>  }
>>  
>>  #if IS_ENABLED(CONFIG_TYPEC)
>> @@ -1660,6 +1672,235 @@ static int qmp_usbc_parse_tcsr(struct qmp_usbc *qmp)
>>  	return 0;
>>  }
>>  
>> +static int qmp_usbc_parse_usb3dp_dt(struct qmp_usbc *qmp)
>> +{
>> +	struct platform_device *pdev = to_platform_device(qmp->dev);
>> +	const struct qmp_phy_cfg *cfg = qmp->cfg;
>> +	const struct qmp_usbc_offsets *offs = cfg->offsets;
>> +	struct device *dev = qmp->dev;
>> +	void __iomem *base;
>> +	int ret;
>> +
>> +	base = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(base))
>> +		return PTR_ERR(base);
>> +
>> +	qmp->dp_serdes = base + offs->dp_serdes;
>> +	qmp->dp_tx = base + offs->dp_txa;
>> +	qmp->dp_tx2 = base + offs->dp_txb;
>> +	qmp->dp_dp_phy = base + offs->dp_dp_phy;
> Squash this into qmp_usbc_parse_dt(). Set these fields if
> dp_serdes != 0.


Ack.


>> +	qmp->serdes = base + offs->serdes;
>> +	qmp->pcs = base + offs->pcs;
>> +	if (offs->pcs_misc)
>> +		qmp->pcs_misc = base + offs->pcs_misc;
>> +	qmp->tx = base + offs->tx;
>> +	qmp->rx = base + offs->rx;
>> +
>> +	qmp->tx2 = base + offs->tx2;
>> +	qmp->rx2 = base + offs->rx2;
>> +
>> +	ret = qmp_usbc_clk_init(qmp);
>> +	if (ret)
>> +		return ret;
>> +
>> +	qmp->pipe_clk = devm_clk_get(dev, "pipe");
>> +	if (IS_ERR(qmp->pipe_clk)) {
>> +		/* usb3dp allow no pipe clk define */
>> +		if (cfg->type == QMP_PHY_USBC_USB3_ONLY)
>> +			return dev_err_probe(dev, PTR_ERR(qmp->pipe_clk),
>> +						"failed to get pipe clock\n");
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +/*
>> + * Display Port PLL driver block diagram for branch clocks
>> + *
>> + *              +------------------------------+
>> + *              |         DP_VCO_CLK           |
>> + *              |                              |
>> + *              |    +-------------------+     |
>> + *              |    |   (DP PLL/VCO)    |     |
>> + *              |    +---------+---------+     |
>> + *              |              v               |
>> + *              |   +----------+-----------+   |
>> + *              |   | hsclk_divsel_clk_src |   |
>> + *              |   +----------+-----------+   |
>> + *              +------------------------------+
>> + *                              |
>> + *          +---------<---------v------------>----------+
>> + *          |                                           |
>> + * +--------v----------------+                          |
>> + * |    dp_phy_pll_link_clk  |                          |
>> + * |     link_clk            |                          |
>> + * +--------+----------------+                          |
>> + *          |                                           |
>> + *          |                                           |
>> + *          v                                           v
>> + * Input to DISPCC block                                |
>> + * for link clk, crypto clk                             |
>> + * and interface clock                                  |
>> + *                                                      |
>> + *                                                      |
>> + *      +--------<------------+-----------------+---<---+
>> + *      |                     |                 |
>> + * +----v---------+  +--------v-----+  +--------v------+
>> + * | vco_divided  |  | vco_divided  |  | vco_divided   |
>> + * |    _clk_src  |  |    _clk_src  |  |    _clk_src   |
>> + * |              |  |              |  |               |
>> + * |divsel_six    |  |  divsel_two  |  |  divsel_four  |
>> + * +-------+------+  +-----+--------+  +--------+------+
>> + *         |                 |                  |
>> + *         v---->----------v-------------<------v
>> + *                         |
>> + *              +----------+-----------------+
>> + *              |   dp_phy_pll_vco_div_clk   |
>> + *              +---------+------------------+
>> + *                        |
>> + *                        v
>> + *              Input to DISPCC block
>> + *              for DP pixel clock
>> + *
>> + */
>> +static int qmp_dp_pixel_clk_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
>> +{
>> +	switch (req->rate) {
>> +	case 1620000000UL / 2:
>> +	case 2700000000UL / 2:
>> +	/* 5.4 and 8.1 GHz are same link rate as 2.7GHz, i.e. div 4 and div 6 */
>> +		return 0;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +}
>> +
>> +static unsigned long qmp_dp_pixel_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
>> +{
>> +	const struct qmp_usbc *qmp;
>> +	const struct phy_configure_opts_dp *dp_opts;
>> +
>> +	qmp = container_of(hw, struct qmp_usbc, dp_pixel_hw);
>> +
>> +	dp_opts = &qmp->dp_opts;
>> +
>> +	switch (dp_opts->link_rate) {
>> +	case 1620:
>> +		return 1620000000UL / 2;
>> +	case 2700:
>> +		return 2700000000UL / 2;
>> +	case 5400:
>> +		return 5400000000UL / 4;
>> +	default:
>> +		return 0;
>> +	}
>> +}
>> +
>> +static const struct clk_ops qmp_dp_pixel_clk_ops = {
>> +	.determine_rate	= qmp_dp_pixel_clk_determine_rate,
>> +	.recalc_rate	= qmp_dp_pixel_clk_recalc_rate,
>> +};
>> +
>> +static int qmp_dp_link_clk_determine_rate(struct clk_hw *hw, struct clk_rate_request *req)
>> +{
>> +	switch (req->rate) {
>> +	case 162000000:
>> +	case 270000000:
>> +	case 540000000:
>> +		return 0;
>> +	default:
>> +		return -EINVAL;
>> +	}
>> +}
>> +
>> +static unsigned long qmp_dp_link_clk_recalc_rate(struct clk_hw *hw, unsigned long parent_rate)
>> +{
>> +	const struct qmp_usbc *qmp;
>> +	const struct phy_configure_opts_dp *dp_opts;
>> +
>> +	qmp = container_of(hw, struct qmp_usbc, dp_link_hw);
>> +	dp_opts = &qmp->dp_opts;
>> +
>> +	switch (dp_opts->link_rate) {
>> +	case 1620:
>> +	case 2700:
>> +	case 5400:
>> +		return dp_opts->link_rate * 100000;
>> +	default:
>> +		return 0;
>> +	}
>> +}
>> +
>> +static const struct clk_ops qmp_dp_link_clk_ops = {
>> +	.determine_rate	= qmp_dp_link_clk_determine_rate,
>> +	.recalc_rate	= qmp_dp_link_clk_recalc_rate,
>> +};
>> +
>> +static int phy_dp_clks_register(struct qmp_usbc *qmp, struct device_node *np)
>> +{
>> +	struct clk_init_data init = { };
>> +	char name[64];
>> +	int ret;
>> +
>> +	snprintf(name, sizeof(name), "%s::link_clk", dev_name(qmp->dev));
>> +	init.ops = &qmp_dp_link_clk_ops;
>> +	init.name = name;
>> +	qmp->dp_link_hw.init = &init;
>> +	ret = devm_clk_hw_register(qmp->dev, &qmp->dp_link_hw);
>> +	if (ret < 0) {
>> +		dev_err(qmp->dev, "link clk reg fail ret=%d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	snprintf(name, sizeof(name), "%s::vco_div_clk", dev_name(qmp->dev));
>> +	init.ops = &qmp_dp_pixel_clk_ops;
>> +	init.name = name;
>> +	qmp->dp_pixel_hw.init = &init;
>> +	ret = devm_clk_hw_register(qmp->dev, &qmp->dp_pixel_hw);
>> +	if (ret) {
>> +		dev_err(qmp->dev, "pxl clk reg fail ret=%d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int qmp_usbc_register_clocks(struct qmp_usbc *qmp, struct device_node *np)
>> +{
>> +	int ret;
>> +
>> +	if (!IS_ERR(qmp->pipe_clk)) {
>> +		ret = phy_pipe_clk_register(qmp, np);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	if (qmp->cfg->type == QMP_PHY_USBC_USB3_DP) {
> if dp_serdes != 0


Ack.


>> +		ret = phy_dp_clks_register(qmp, np);
>> +		if (ret)
>> +			return ret;
>> +	}
>> +
>> +	return devm_of_clk_add_hw_provider(qmp->dev, qmp_usbc_clks_hw_get, qmp);
>> +}
>> +
>> +static struct phy *qmp_usbc_phy_xlate(struct device *dev, const struct of_phandle_args *args)
>> +{
>> +	struct qmp_usbc *qmp = dev_get_drvdata(dev);
>> +
>> +	if (args->args_count == 0)
>> +		return qmp->usb_phy;
>> +
>> +	switch (args->args[0]) {
>> +	case QMP_USB43DP_USB3_PHY:
>> +		return qmp->usb_phy;
>> +	case QMP_USB43DP_DP_PHY:
>> +		return qmp->dp_phy;
>> +	}
>> +
>> +	return ERR_PTR(-EINVAL);
>> +}
>> +
>>  static int qmp_usbc_probe(struct platform_device *pdev)
>>  {
>>  	struct device *dev = &pdev->dev;
>> @@ -1703,16 +1944,32 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>>  	if (ret)
>>  		return ret;
>>  
>> -	/* Check for legacy binding with child node. */
>> -	np = of_get_child_by_name(dev->of_node, "phy");
>> -	if (np) {
>> -		ret = qmp_usbc_parse_usb_dt_legacy(qmp, np);
>> -	} else {
>> +	if (qmp->cfg->type == QMP_PHY_USBC_USB3_DP) {
> Should not be necessary.


Got it. I’ll merge the parsing logic into a single qmp_usbc_parse_dt function.

Also, I checked the compatible strings in the dtsi files for this PHY series
looks like no current product uses the legacy binding. 
I’ll drop qmp_usbc_parse_usb_dt_legacy in the next version.


>>  		np = of_node_get(dev->of_node);
>> -		ret = qmp_usbc_parse_usb_dt(qmp);
>> +
>> +		ret = qmp_usbc_parse_usb3dp_dt(qmp);
>> +		if (ret) {
>> +			dev_err(qmp->dev, "parse DP dt fail ret=%d\n", ret);
>> +			goto err_node_put;
>> +		}
>> +
>> +		ret = drm_aux_bridge_register(dev);
>> +		if (ret) {
>> +			dev_err(qmp->dev, "aux bridge reg fail ret=%d\n", ret);
>> +			goto err_node_put;
>> +		}
>> +	} else {
>> +		/* Check for legacy binding with child node. */
>> +		np = of_get_child_by_name(dev->of_node, "phy");
>> +		if (np) {
>> +			ret = qmp_usbc_parse_usb_dt_legacy(qmp, np);
>> +		} else {
>> +			np = of_node_get(dev->of_node);
>> +			ret = qmp_usbc_parse_usb_dt(qmp);
>> +		}
>> +		if (ret)
>> +			goto err_node_put;
>>  	}
>> -	if (ret)
>> -		goto err_node_put;
>>  
>>  	pm_runtime_set_active(dev);
>>  	ret = devm_pm_runtime_enable(dev);
>> @@ -1724,7 +1981,7 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>>  	 */
>>  	pm_runtime_forbid(dev);
>>  
>> -	ret = phy_pipe_clk_register(qmp, np);
>> +	ret = qmp_usbc_register_clocks(qmp, np);
>>  	if (ret)
>>  		goto err_node_put;
>>  
>> @@ -1737,9 +1994,19 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>>  
>>  	phy_set_drvdata(qmp->usb_phy, qmp);
>>  
>> +	if (qmp->cfg->type == QMP_PHY_USBC_USB3_DP) {
> if dp_serdes != 0


Ack.


>> +		qmp->dp_phy = devm_phy_create(dev, np, &qmp_usbc_dp_phy_ops);
>> +		if (IS_ERR(qmp->dp_phy)) {
>> +			ret = PTR_ERR(qmp->dp_phy);
>> +			dev_err(dev, "failed to create PHY: %d\n", ret);
>> +			goto err_node_put;
>> +		}
>> +		phy_set_drvdata(qmp->dp_phy, qmp);
>> +	}
>> +
>>  	of_node_put(np);
>>  
>> -	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>> +	phy_provider = devm_of_phy_provider_register(dev, qmp_usbc_phy_xlate);
>>  
>>  	return PTR_ERR_OR_ZERO(phy_provider);
>>  
>>
>> -- 
>> 2.34.1
>>

On Wed, Aug 27, 2025 at 08:34:39PM +0800, Xiangxu Yin wrote:
> 
> On 8/20/2025 7:42 PM, Dmitry Baryshkov wrote:
> > On Wed, Aug 20, 2025 at 05:34:53PM +0800, Xiangxu Yin wrote:
> >> Complete USB/DP switchable PHY integration by adding DP clock
> >> registration, aux bridge setup, and DT parsing. Implement clock
> >> provider logic for USB and DP branches, and extend PHY translation
> >> to support both USB and DP instances.
> >>
> >> Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
> >> ---
> >>  drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 331 ++++++++++++++++++++++++++++---
> >>  1 file changed, 299 insertions(+), 32 deletions(-)
> >>
> >>  static int qmp_usbc_probe(struct platform_device *pdev)
> >>  {
> >>  	struct device *dev = &pdev->dev;
> >> @@ -1703,16 +1944,32 @@ static int qmp_usbc_probe(struct platform_device *pdev)
> >>  	if (ret)
> >>  		return ret;
> >>  
> >> -	/* Check for legacy binding with child node. */
> >> -	np = of_get_child_by_name(dev->of_node, "phy");
> >> -	if (np) {
> >> -		ret = qmp_usbc_parse_usb_dt_legacy(qmp, np);
> >> -	} else {
> >> +	if (qmp->cfg->type == QMP_PHY_USBC_USB3_DP) {
> > Should not be necessary.
> 
> 
> Got it. I’ll merge the parsing logic into a single qmp_usbc_parse_dt function.
> 
> Also, I checked the compatible strings in the dtsi files for this PHY series
> looks like no current product uses the legacy binding. 
> I’ll drop qmp_usbc_parse_usb_dt_legacy in the next version.


No. It's _legacy_, it has been implemented in order to support old DTs,
which existed at some point but then were refactored into the current
state. You can't randomly drop DT support.

> 
> 
> >>  		np = of_node_get(dev->of_node);
> >> -		ret = qmp_usbc_parse_usb_dt(qmp);
> >> +
> >> +		ret = qmp_usbc_parse_usb3dp_dt(qmp);
> >> +		if (ret) {
> >> +			dev_err(qmp->dev, "parse DP dt fail ret=%d\n", ret);
> >> +			goto err_node_put;
> >> +		}
> >> +
> >> +		ret = drm_aux_bridge_register(dev);
> >> +		if (ret) {
> >> +			dev_err(qmp->dev, "aux bridge reg fail ret=%d\n", ret);
> >> +			goto err_node_put;
> >> +		}

-- 
With best wishes
Dmitry

On 8/28/2025 12:14 AM, Dmitry Baryshkov wrote:
> On Wed, Aug 27, 2025 at 08:34:39PM +0800, Xiangxu Yin wrote:
>> On 8/20/2025 7:42 PM, Dmitry Baryshkov wrote:
>>> On Wed, Aug 20, 2025 at 05:34:53PM +0800, Xiangxu Yin wrote:
>>>> Complete USB/DP switchable PHY integration by adding DP clock
>>>> registration, aux bridge setup, and DT parsing. Implement clock
>>>> provider logic for USB and DP branches, and extend PHY translation
>>>> to support both USB and DP instances.
>>>>
>>>> Signed-off-by: Xiangxu Yin <xiangxu.yin@oss.qualcomm.com>
>>>> ---
>>>>  drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 331 ++++++++++++++++++++++++++++---
>>>>  1 file changed, 299 insertions(+), 32 deletions(-)
>>>>
>>>>  static int qmp_usbc_probe(struct platform_device *pdev)
>>>>  {
>>>>  	struct device *dev = &pdev->dev;
>>>> @@ -1703,16 +1944,32 @@ static int qmp_usbc_probe(struct platform_device *pdev)
>>>>  	if (ret)
>>>>  		return ret;
>>>>  
>>>> -	/* Check for legacy binding with child node. */
>>>> -	np = of_get_child_by_name(dev->of_node, "phy");
>>>> -	if (np) {
>>>> -		ret = qmp_usbc_parse_usb_dt_legacy(qmp, np);
>>>> -	} else {
>>>> +	if (qmp->cfg->type == QMP_PHY_USBC_USB3_DP) {
>>> Should not be necessary.
>>
>> Got it. I’ll merge the parsing logic into a single qmp_usbc_parse_dt function.
>>
>> Also, I checked the compatible strings in the dtsi files for this PHY series
>> looks like no current product uses the legacy binding. 
>> I’ll drop qmp_usbc_parse_usb_dt_legacy in the next version.
>
> No. It's _legacy_, it has been implemented in order to support old DTs,
> which existed at some point but then were refactored into the current
> state. You can't randomly drop DT support.


Ok, understand.


>>
>>>>  		np = of_node_get(dev->of_node);
>>>> -		ret = qmp_usbc_parse_usb_dt(qmp);
>>>> +
>>>> +		ret = qmp_usbc_parse_usb3dp_dt(qmp);
>>>> +		if (ret) {
>>>> +			dev_err(qmp->dev, "parse DP dt fail ret=%d\n", ret);
>>>> +			goto err_node_put;
>>>> +		}
>>>> +
>>>> +		ret = drm_aux_bridge_register(dev);
>>>> +		if (ret) {
>>>> +			dev_err(qmp->dev, "aux bridge reg fail ret=%d\n", ret);
>>>> +			goto err_node_put;
>>>> +		}