[PATCH v2 0/2] phy: qcom: edp: Add support for X1E80100

[PATCH v2 0/2] phy: qcom: edp: Add support for X1E80100

[Abel Vesa]

This patchset adds support for the X1E80100 eDP/DP PHY and documents its
compatible.

This patchset depends on the QSERDES_V6_COM_SSC_ADJ_PER1 register offset
added by the following patchset:

https://lore.kernel.org/all/20231122-phy-qualcomm-v6-v6-20-v7-new-offsets-v1-0-d9340d362664@linaro.org/

Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
Changes in v2:
- Added Krzysztof's R-b tag
- Re-worded commit message for bindings to suggest same PHY can work in
  both eDP and DP mode rather than being different PHY types.
- Implemented different qcom_edp_configure_ssc and
  qcom_edp_configure_pll for each version of the PHY.
- Dropped the cfg8 override in qcom_edp_phy_init
- Used enum instead of defines for PHY versions
- Link to v1: https://lore.kernel.org/r/20231122-phy-qualcomm-edp-x1e80100-v1-0-a9938990edb3@linaro.org

---
Abel Vesa (2):
      dt-bindings: phy: qcom-edp: Add X1E80100 PHY compatibles
      phy: qcom: edp: Add X1E80100 eDP and DP PHYs

 .../devicetree/bindings/phy/qcom,edp-phy.yaml      |   2 +
 drivers/phy/qualcomm/phy-qcom-edp.c                | 230 +++++++++++++++++++--
 2 files changed, 214 insertions(+), 18 deletions(-)
---
base-commit: 629a3b49f3f957e975253c54846090b8d5ed2e9b
change-id: 20231122-phy-qualcomm-edp-x1e80100-a57c15fff32b

Best regards,
-- 
Abel Vesa <abel.vesa@linaro.org>

[PATCH v2 1/2] dt-bindings: phy: qcom-edp: Add X1E80100 PHY compatibles

[Abel Vesa]

The Qualcomm X1E80100 platform has multiple PHYs that can work in both eDP
or DP mode, add compatibles for these.

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
index 6566353f1a02..190f61c1d0f9 100644
--- a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
+++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
@@ -21,6 +21,8 @@ properties:
       - qcom,sc8180x-edp-phy
       - qcom,sc8280xp-dp-phy
       - qcom,sc8280xp-edp-phy
+      - qcom,x1e80100-dp-phy
+      - qcom,x1e80100-edp-phy
 
   reg:
     items:

-- 
2.34.1

Re: [PATCH v2 1/2] dt-bindings: phy: qcom-edp: Add X1E80100 PHY compatibles

[Johan Hovold]

On Mon, Dec 04, 2023 at 05:01:36PM +0200, Abel Vesa wrote:
> The Qualcomm X1E80100 platform has multiple PHYs that can work in both eDP
> or DP mode, add compatibles for these.
> 
> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
>  Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
> index 6566353f1a02..190f61c1d0f9 100644
> --- a/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
> +++ b/Documentation/devicetree/bindings/phy/qcom,edp-phy.yaml
> @@ -21,6 +21,8 @@ properties:
>        - qcom,sc8180x-edp-phy
>        - qcom,sc8280xp-dp-phy
>        - qcom,sc8280xp-edp-phy
> +      - qcom,x1e80100-dp-phy
> +      - qcom,x1e80100-edp-phy

As was discussed here:

	https://lore.kernel.org/lkml/20231017032810.GP3553829@hu-bjorande-lv.qualcomm.com/

there should most likely only be one compatible here too as this is just
the same hardware block operating in two different modes.

Johan

[PATCH v2 2/2] phy: qcom: edp: Add X1E80100 eDP and DP PHYs

[Abel Vesa]

The Qualcomm X1E80100 platform has a number of eDP and DP PHY instances,
add support for these.

Co-developed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
 drivers/phy/qualcomm/phy-qcom-edp.c | 230 +++++++++++++++++++++++++++++++++---
 1 file changed, 212 insertions(+), 18 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
index 8e5078304646..702d49c18044 100644
--- a/drivers/phy/qualcomm/phy-qcom-edp.c
+++ b/drivers/phy/qualcomm/phy-qcom-edp.c
@@ -23,6 +23,11 @@
 
 #include "phy-qcom-qmp.h"
 
+enum {
+	QSERDES_V4,
+	QSERDES_V6,
+};
+
 /* EDP_PHY registers */
 #define DP_PHY_CFG                              0x0010
 #define DP_PHY_CFG_1                            0x0014
@@ -70,6 +75,7 @@
 
 struct qcom_edp_cfg {
 	bool is_dp;
+	int qserdes_version;
 
 	/* DP PHY swing and pre_emphasis tables */
 	const u8 (*swing_hbr_rbr)[4][4];
@@ -94,7 +100,7 @@ struct qcom_edp {
 
 	struct phy_configure_opts_dp dp_opts;
 
-	struct clk_bulk_data clks[2];
+	struct clk_bulk_data clks[3];
 	struct regulator_bulk_data supplies[2];
 };
 
@@ -126,8 +132,18 @@ static const u8 dp_pre_emp_hbr2_hbr3[4][4] = {
 	{ 0x04, 0xff, 0xff, 0xff }
 };
 
-static const struct qcom_edp_cfg dp_phy_cfg = {
+static const struct qcom_edp_cfg dp_v4_phy_cfg = {
+	.is_dp = true,
+	.qserdes_version = QSERDES_V4,
+	.swing_hbr_rbr = &dp_swing_hbr_rbr,
+	.swing_hbr3_hbr2 = &dp_swing_hbr2_hbr3,
+	.pre_emphasis_hbr_rbr = &dp_pre_emp_hbr_rbr,
+	.pre_emphasis_hbr3_hbr2 = &dp_pre_emp_hbr2_hbr3,
+};
+
+static const struct qcom_edp_cfg dp_v6_phy_cfg = {
 	.is_dp = true,
+	.qserdes_version = QSERDES_V6,
 	.swing_hbr_rbr = &dp_swing_hbr_rbr,
 	.swing_hbr3_hbr2 = &dp_swing_hbr2_hbr3,
 	.pre_emphasis_hbr_rbr = &dp_pre_emp_hbr_rbr,
@@ -162,8 +178,18 @@ static const u8 edp_pre_emp_hbr2_hbr3[4][4] = {
 	{ 0x00, 0xff, 0xff, 0xff }
 };
 
-static const struct qcom_edp_cfg edp_phy_cfg = {
+static const struct qcom_edp_cfg edp_v4_phy_cfg = {
+	.is_dp = false,
+	.qserdes_version = QSERDES_V4,
+	.swing_hbr_rbr = &edp_swing_hbr_rbr,
+	.swing_hbr3_hbr2 = &edp_swing_hbr2_hbr3,
+	.pre_emphasis_hbr_rbr = &edp_pre_emp_hbr_rbr,
+	.pre_emphasis_hbr3_hbr2 = &edp_pre_emp_hbr2_hbr3,
+};
+
+static const struct qcom_edp_cfg edp_v6_phy_cfg = {
 	.is_dp = false,
+	.qserdes_version = QSERDES_V6,
 	.swing_hbr_rbr = &edp_swing_hbr_rbr,
 	.swing_hbr3_hbr2 = &edp_swing_hbr2_hbr3,
 	.pre_emphasis_hbr_rbr = &edp_pre_emp_hbr_rbr,
@@ -190,7 +216,10 @@ static int qcom_edp_phy_init(struct phy *phy)
 	       edp->edp + DP_PHY_PD_CTL);
 
 	/* Turn on BIAS current for PHY/PLL */
-	writel(0x17, edp->pll + QSERDES_V4_COM_BIAS_EN_CLKBUFLR_EN);
+	if (cfg->qserdes_version == QSERDES_V6)
+		writel(0x1f, edp->pll + QSERDES_V6_COM_PLL_BIAS_EN_CLK_BUFLR_EN);
+	else
+		writel(0x17, edp->pll + QSERDES_V4_COM_BIAS_EN_CLKBUFLR_EN);
 
 	writel(DP_PHY_PD_CTL_PSR_PWRDN, edp->edp + DP_PHY_PD_CTL);
 	msleep(20);
@@ -261,7 +290,10 @@ static int qcom_edp_set_voltages(struct qcom_edp *edp, const struct phy_configur
 	if (swing == 0xff || emph == 0xff)
 		return -EINVAL;
 
-	ldo_config = (cfg && cfg->is_dp) ? 0x1 : 0x0;
+	if (cfg->qserdes_version == QSERDES_V6)
+		ldo_config = 0x91;
+	else
+		ldo_config = (cfg && cfg->is_dp) ? 0x1 : 0x0;
 
 	writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
 	writel(swing, edp->tx0 + TXn_TX_DRV_LVL);
@@ -288,7 +320,7 @@ static int qcom_edp_phy_configure(struct phy *phy, union phy_configure_opts *opt
 	return ret;
 }
 
-static int qcom_edp_configure_ssc(const struct qcom_edp *edp)
+static int qcom_edp_configure_ssc_v4(const struct qcom_edp *edp)
 {
 	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
 	u32 step1;
@@ -322,7 +354,41 @@ static int qcom_edp_configure_ssc(const struct qcom_edp *edp)
 	return 0;
 }
 
-static int qcom_edp_configure_pll(const struct qcom_edp *edp)
+static int qcom_edp_configure_ssc_v6(const struct qcom_edp *edp)
+{
+	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
+	u32 step1;
+	u32 step2;
+
+	switch (dp_opts->link_rate) {
+	case 1620:
+	case 2700:
+	case 8100:
+		step1 = 0x92;
+		step2 = 0x01;
+		break;
+
+	case 5400:
+		step1 = 0x18;
+		step2 = 0x02;
+		break;
+
+	default:
+		/* Other link rates aren't supported */
+		return -EINVAL;
+	}
+
+	writel(0x01, edp->pll + QSERDES_V6_COM_SSC_EN_CENTER);
+	writel(0x00, edp->pll + QSERDES_V6_COM_SSC_ADJ_PER1);
+	writel(0x6b, edp->pll + QSERDES_V6_COM_SSC_PER1);
+	writel(0x02, edp->pll + QSERDES_V6_COM_SSC_PER2);
+	writel(step1, edp->pll + QSERDES_V6_COM_SSC_STEP_SIZE1_MODE0);
+	writel(step2, edp->pll + QSERDES_V6_COM_SSC_STEP_SIZE2_MODE0);
+
+	return 0;
+}
+
+static int qcom_edp_configure_pll_v4(const struct qcom_edp *edp)
 {
 	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
 	u32 div_frac_start2_mode0;
@@ -349,6 +415,7 @@ static int qcom_edp_configure_pll(const struct qcom_edp *edp)
 		div_frac_start3_mode0 = 0x07;
 		lock_cmp1_mode0 = 0x0f;
 		lock_cmp2_mode0 = 0x0e;
+
 		break;
 
 	case 5400:
@@ -358,6 +425,7 @@ static int qcom_edp_configure_pll(const struct qcom_edp *edp)
 		div_frac_start3_mode0 = 0x0a;
 		lock_cmp1_mode0 = 0x1f;
 		lock_cmp2_mode0 = 0x1c;
+
 		break;
 
 	case 8100:
@@ -367,6 +435,7 @@ static int qcom_edp_configure_pll(const struct qcom_edp *edp)
 		div_frac_start3_mode0 = 0x07;
 		lock_cmp1_mode0 = 0x2f;
 		lock_cmp2_mode0 = 0x2a;
+
 		break;
 
 	default:
@@ -408,6 +477,103 @@ static int qcom_edp_configure_pll(const struct qcom_edp *edp)
 	return 0;
 }
 
+static int qcom_edp_configure_pll_v6(const struct qcom_edp *edp)
+{
+	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
+	u32 div_frac_start2_mode0;
+	u32 div_frac_start3_mode0;
+	u32 dec_start_mode0;
+	u32 lock_cmp1_mode0;
+	u32 lock_cmp2_mode0;
+	u32 code1_mode0;
+	u32 code2_mode0;
+	u32 hsclk_sel;
+
+	switch (dp_opts->link_rate) {
+	case 1620:
+		hsclk_sel = 0x5;
+		dec_start_mode0 = 0x34;
+		div_frac_start2_mode0 = 0xc0;
+		div_frac_start3_mode0 = 0x0b;
+		lock_cmp1_mode0 = 0x37;
+		lock_cmp2_mode0 = 0x04;
+		code1_mode0 = 0x71;
+		code2_mode0 = 0x0C;
+		break;
+
+	case 2700:
+		hsclk_sel = 0x3;
+		dec_start_mode0 = 0x34;
+		div_frac_start2_mode0 = 0xc0;
+		div_frac_start3_mode0 = 0x0b;
+		lock_cmp1_mode0 = 0x07;
+		lock_cmp2_mode0 = 0x07;
+		code1_mode0 = 0x71;
+		code2_mode0 = 0x0C;
+		break;
+
+	case 5400:
+		hsclk_sel = 0x1;
+		dec_start_mode0 = 0x46;
+		div_frac_start2_mode0 = 0x00;
+		div_frac_start3_mode0 = 0x05;
+		lock_cmp1_mode0 = 0x0f;
+		lock_cmp2_mode0 = 0x0e;
+		code1_mode0 = 0x97;
+		code2_mode0 = 0x10;
+		break;
+
+	case 8100:
+		hsclk_sel = 0x0;
+		dec_start_mode0 = 0x34;
+		div_frac_start2_mode0 = 0xc0;
+		div_frac_start3_mode0 = 0x0b;
+		lock_cmp1_mode0 = 0x17;
+		lock_cmp2_mode0 = 0x15;
+		code1_mode0 = 0x71;
+		code2_mode0 = 0x0C;
+		break;
+
+	default:
+		/* Other link rates aren't supported */
+		return -EINVAL;
+	}
+
+	writel(0x01, edp->pll + QSERDES_V6_COM_SVS_MODE_CLK_SEL);
+	writel(0x0b, edp->pll + QSERDES_V6_COM_SYSCLK_EN_SEL);
+	writel(0x02, edp->pll + QSERDES_V6_COM_SYS_CLK_CTRL);
+	writel(0x0c, edp->pll + QSERDES_V6_COM_CLK_ENABLE1);
+	writel(0x06, edp->pll + QSERDES_V6_COM_SYSCLK_BUF_ENABLE);
+	writel(0x30, edp->pll + QSERDES_V6_COM_CLK_SELECT);
+	writel(hsclk_sel, edp->pll + QSERDES_V6_COM_HSCLK_SEL_1);
+	writel(0x07, edp->pll + QSERDES_V6_COM_PLL_IVCO);
+	writel(0x08, edp->pll + QSERDES_V6_COM_LOCK_CMP_EN);
+	writel(0x36, edp->pll + QSERDES_V6_COM_PLL_CCTRL_MODE0);
+	writel(0x16, edp->pll + QSERDES_V6_COM_PLL_RCTRL_MODE0);
+	writel(0x6, edp->pll + QSERDES_V6_COM_CP_CTRL_MODE0);
+	writel(dec_start_mode0, edp->pll + QSERDES_V6_COM_DEC_START_MODE0);
+	writel(0x00, edp->pll + QSERDES_V6_COM_DIV_FRAC_START1_MODE0);
+	writel(div_frac_start2_mode0, edp->pll + QSERDES_V6_COM_DIV_FRAC_START2_MODE0);
+	writel(div_frac_start3_mode0, edp->pll + QSERDES_V6_COM_DIV_FRAC_START3_MODE0);
+	writel(0x12, edp->pll + QSERDES_V6_COM_CMN_CONFIG_1);
+	writel(0x3f, edp->pll + QSERDES_V6_COM_INTEGLOOP_GAIN0_MODE0);
+	writel(0x00, edp->pll + QSERDES_V6_COM_INTEGLOOP_GAIN1_MODE0);
+	writel(0x00, edp->pll + QSERDES_V6_COM_VCO_TUNE_MAP);
+	writel(lock_cmp1_mode0, edp->pll + QSERDES_V6_COM_LOCK_CMP1_MODE0);
+	writel(lock_cmp2_mode0, edp->pll + QSERDES_V6_COM_LOCK_CMP2_MODE0);
+
+	writel(0x0a, edp->pll + QSERDES_V6_COM_BG_TIMER);
+	writel(0x14, edp->pll + QSERDES_V6_COM_PLL_CORE_CLK_DIV_MODE0);
+	writel(0x00, edp->pll + QSERDES_V6_COM_VCO_TUNE_CTRL);
+	writel(0x17, edp->pll + QSERDES_V6_COM_PLL_BIAS_EN_CLK_BUFLR_EN);
+	writel(0x0f, edp->pll + QSERDES_V6_COM_CORE_CLK_EN);
+
+	writel(code1_mode0, edp->pll + QSERDES_V6_COM_BIN_VCOCAL_CMP_CODE1_MODE0);
+	writel(code2_mode0, edp->pll + QSERDES_V6_COM_BIN_VCOCAL_CMP_CODE2_MODE0);
+
+	return 0;
+}
+
 static int qcom_edp_set_vco_div(const struct qcom_edp *edp, unsigned long *pixel_freq)
 {
 	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
@@ -462,13 +628,22 @@ static int qcom_edp_phy_power_on(struct phy *phy)
 	       edp->edp + DP_PHY_PD_CTL);
 	writel(0xfc, edp->edp + DP_PHY_MODE);
 
-	timeout = readl_poll_timeout(edp->pll + QSERDES_V4_COM_CMN_STATUS,
-				     val, val & BIT(7), 5, 200);
+	if (cfg->qserdes_version == QSERDES_V6) {
+		timeout = readl_poll_timeout(edp->pll + QSERDES_V6_COM_CMN_STATUS,
+					     val, val & BIT(7), 5, 200);
+	} else {
+		timeout = readl_poll_timeout(edp->pll + QSERDES_V4_COM_CMN_STATUS,
+					     val, val & BIT(7), 5, 200);
+	}
+
 	if (timeout)
 		return timeout;
 
 
-	ldo_config = (cfg && cfg->is_dp) ? 0x1 : 0x0;
+	if (cfg->qserdes_version == QSERDES_V6)
+		ldo_config = 0x91;
+	else
+		ldo_config = (cfg && cfg->is_dp) ? 0x1 : 0x0;
 
 	writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
 	writel(ldo_config, edp->tx1 + TXn_LDO_CONFIG);
@@ -476,12 +651,20 @@ static int qcom_edp_phy_power_on(struct phy *phy)
 	writel(0x00, edp->tx1 + TXn_LANE_MODE_1);
 
 	if (edp->dp_opts.ssc) {
-		ret = qcom_edp_configure_ssc(edp);
+		if (cfg->qserdes_version == QSERDES_V6)
+			ret = qcom_edp_configure_ssc_v6(edp);
+		else
+			ret = qcom_edp_configure_ssc_v4(edp);
+
 		if (ret)
 			return ret;
 	}
 
-	ret = qcom_edp_configure_pll(edp);
+	if (cfg->qserdes_version == QSERDES_V6)
+		ret = qcom_edp_configure_pll_v6(edp);
+	else
+		ret = qcom_edp_configure_pll_v4(edp);
+
 	if (ret)
 		return ret;
 
@@ -512,10 +695,18 @@ static int qcom_edp_phy_power_on(struct phy *phy)
 	writel(0x01, edp->edp + DP_PHY_CFG);
 	writel(0x09, edp->edp + DP_PHY_CFG);
 
-	writel(0x20, edp->pll + QSERDES_V4_COM_RESETSM_CNTRL);
+	if (cfg->qserdes_version == QSERDES_V6) {
+		writel(0x20, edp->pll + QSERDES_V6_COM_RESETSM_CNTRL);
+
+		timeout = readl_poll_timeout(edp->pll + QSERDES_V6_COM_C_READY_STATUS,
+					     val, val & BIT(0), 500, 10000);
+	} else {
+		writel(0x20, edp->pll + QSERDES_V4_COM_RESETSM_CNTRL);
+
+		timeout = readl_poll_timeout(edp->pll + QSERDES_V4_COM_C_READY_STATUS,
+					     val, val & BIT(0), 500, 10000);
+	}
 
-	timeout = readl_poll_timeout(edp->pll + QSERDES_V4_COM_C_READY_STATUS,
-				     val, val & BIT(0), 500, 10000);
 	if (timeout)
 		return timeout;
 
@@ -744,7 +935,6 @@ static int qcom_edp_clks_register(struct qcom_edp *edp, struct device_node *np)
 	data = devm_kzalloc(edp->dev, struct_size(data, hws, 2), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
-	data->num = 2;
 
 	snprintf(name, sizeof(name), "%s::link_clk", dev_name(edp->dev));
 	init.ops = &qcom_edp_dp_link_clk_ops;
@@ -764,6 +954,7 @@ static int qcom_edp_clks_register(struct qcom_edp *edp, struct device_node *np)
 
 	data->hws[0] = &edp->dp_link_hw;
 	data->hws[1] = &edp->dp_pixel_hw;
+	data->num = 2;
 
 	return devm_of_clk_add_hw_provider(edp->dev, of_clk_hw_onecell_get, data);
 }
@@ -806,6 +997,7 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
 
 	edp->supplies[0].supply = "vdda-phy";
 	edp->supplies[1].supply = "vdda-pll";
+	edp->clks[2].id = "tcsr";
 	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies), edp->supplies);
 	if (ret)
 		return ret;
@@ -841,8 +1033,10 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
 static const struct of_device_id qcom_edp_phy_match_table[] = {
 	{ .compatible = "qcom,sc7280-edp-phy" },
 	{ .compatible = "qcom,sc8180x-edp-phy" },
-	{ .compatible = "qcom,sc8280xp-dp-phy", .data = &dp_phy_cfg },
-	{ .compatible = "qcom,sc8280xp-edp-phy", .data = &edp_phy_cfg },
+	{ .compatible = "qcom,sc8280xp-dp-phy", .data = &dp_v4_phy_cfg },
+	{ .compatible = "qcom,sc8280xp-edp-phy", .data = &edp_v4_phy_cfg },
+	{ .compatible = "qcom,x1e80100-dp-phy", .data = &dp_v6_phy_cfg },
+	{ .compatible = "qcom,x1e80100-edp-phy", .data = &edp_v6_phy_cfg },
 	{ }
 };
 MODULE_DEVICE_TABLE(of, qcom_edp_phy_match_table);

-- 
2.34.1

Re: [PATCH v2 2/2] phy: qcom: edp: Add X1E80100 eDP and DP PHYs

[Johan Hovold]

On Mon, Dec 04, 2023 at 05:01:37PM +0200, Abel Vesa wrote:
> The Qualcomm X1E80100 platform has a number of eDP and DP PHY instances,
> add support for these.
> 
> Co-developed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Signed-off-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
  
>  	/* Turn on BIAS current for PHY/PLL */
> -	writel(0x17, edp->pll + QSERDES_V4_COM_BIAS_EN_CLKBUFLR_EN);
> +	if (cfg->qserdes_version == QSERDES_V6)
> +		writel(0x1f, edp->pll + QSERDES_V6_COM_PLL_BIAS_EN_CLK_BUFLR_EN);
> +	else
> +		writel(0x17, edp->pll + QSERDES_V4_COM_BIAS_EN_CLKBUFLR_EN);

Sprinkling version conditionals like this throughout the driver look a
bit sloppy and makes the code harder to read (and maintain long term).

Not as easy to clean up when you're using magic numbers instead of
proper value defines, but perhaps you can just fix that up as well as
you should access to the documentation.
  
>  	writel(DP_PHY_PD_CTL_PSR_PWRDN, edp->edp + DP_PHY_PD_CTL);
>  	msleep(20);
> @@ -261,7 +290,10 @@ static int qcom_edp_set_voltages(struct qcom_edp *edp, const struct phy_configur
>  	if (swing == 0xff || emph == 0xff)
>  		return -EINVAL;
>  
> -	ldo_config = (cfg && cfg->is_dp) ? 0x1 : 0x0;
> +	if (cfg->qserdes_version == QSERDES_V6)
> +		ldo_config = 0x91;
> +	else
> +		ldo_config = (cfg && cfg->is_dp) ? 0x1 : 0x0;

Similar here.

>  	writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
>  	writel(swing, edp->tx0 + TXn_TX_DRV_LVL);
> @@ -288,7 +320,7 @@ static int qcom_edp_phy_configure(struct phy *phy, union phy_configure_opts *opt
>  	return ret;
>  }
>  
> -static int qcom_edp_configure_ssc(const struct qcom_edp *edp)
> +static int qcom_edp_configure_ssc_v4(const struct qcom_edp *edp)
>  {
>  	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
>  	u32 step1;

> @@ -349,6 +415,7 @@ static int qcom_edp_configure_pll(const struct qcom_edp *edp)
>  		div_frac_start3_mode0 = 0x07;
>  		lock_cmp1_mode0 = 0x0f;
>  		lock_cmp2_mode0 = 0x0e;
> +
>  		break;
>  
>  	case 5400:
> @@ -358,6 +425,7 @@ static int qcom_edp_configure_pll(const struct qcom_edp *edp)
>  		div_frac_start3_mode0 = 0x0a;
>  		lock_cmp1_mode0 = 0x1f;
>  		lock_cmp2_mode0 = 0x1c;
> +
>  		break;
>  
>  	case 8100:
> @@ -367,6 +435,7 @@ static int qcom_edp_configure_pll(const struct qcom_edp *edp)
>  		div_frac_start3_mode0 = 0x07;
>  		lock_cmp1_mode0 = 0x2f;
>  		lock_cmp2_mode0 = 0x2a;
> +
>  		break;
>  
>  	default:

This looks like unrelated changes that do not belong in this patch.

> @@ -408,6 +477,103 @@ static int qcom_edp_configure_pll(const struct qcom_edp *edp)
>  	return 0;
>  }
>  
> +static int qcom_edp_configure_pll_v6(const struct qcom_edp *edp)
> +{
> +	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
> +	u32 div_frac_start2_mode0;
> +	u32 div_frac_start3_mode0;
> +	u32 dec_start_mode0;
> +	u32 lock_cmp1_mode0;
> +	u32 lock_cmp2_mode0;
> +	u32 code1_mode0;
> +	u32 code2_mode0;
> +	u32 hsclk_sel;
> +
> +	switch (dp_opts->link_rate) {
> +	case 1620:
> +		hsclk_sel = 0x5;
> +		dec_start_mode0 = 0x34;
> +		div_frac_start2_mode0 = 0xc0;
> +		div_frac_start3_mode0 = 0x0b;
> +		lock_cmp1_mode0 = 0x37;
> +		lock_cmp2_mode0 = 0x04;
> +		code1_mode0 = 0x71;
> +		code2_mode0 = 0x0C;
> +		break;
> +

Drop newline before case throughout? Doesn't seem to help with
readability. But I see now that some other functions do this already.

> +	case 2700:
> +		hsclk_sel = 0x3;
> +		dec_start_mode0 = 0x34;
> +		div_frac_start2_mode0 = 0xc0;
> +		div_frac_start3_mode0 = 0x0b;
> +		lock_cmp1_mode0 = 0x07;
> +		lock_cmp2_mode0 = 0x07;
> +		code1_mode0 = 0x71;
> +		code2_mode0 = 0x0C;
> +		break;
> +
> +	case 5400:

>  static int qcom_edp_set_vco_div(const struct qcom_edp *edp, unsigned long *pixel_freq)
>  {
>  	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
> @@ -462,13 +628,22 @@ static int qcom_edp_phy_power_on(struct phy *phy)
>  	       edp->edp + DP_PHY_PD_CTL);
>  	writel(0xfc, edp->edp + DP_PHY_MODE);
>  
> -	timeout = readl_poll_timeout(edp->pll + QSERDES_V4_COM_CMN_STATUS,
> -				     val, val & BIT(7), 5, 200);
> +	if (cfg->qserdes_version == QSERDES_V6) {
> +		timeout = readl_poll_timeout(edp->pll + QSERDES_V6_COM_CMN_STATUS,
> +					     val, val & BIT(7), 5, 200);
> +	} else {
> +		timeout = readl_poll_timeout(edp->pll + QSERDES_V4_COM_CMN_STATUS,
> +					     val, val & BIT(7), 5, 200);
> +	}

This too looks a bit sloppy. Should you not just abstract the status
register offset?

> +
>  	if (timeout)
>  		return timeout;
>  
>  
> -	ldo_config = (cfg && cfg->is_dp) ? 0x1 : 0x0;
> +	if (cfg->qserdes_version == QSERDES_V6)
> +		ldo_config = 0x91;
> +	else
> +		ldo_config = (cfg && cfg->is_dp) ? 0x1 : 0x0;

Same construct and comment as above.

>  
>  	writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
>  	writel(ldo_config, edp->tx1 + TXn_LDO_CONFIG);
  
> @@ -512,10 +695,18 @@ static int qcom_edp_phy_power_on(struct phy *phy)
>  	writel(0x01, edp->edp + DP_PHY_CFG);
>  	writel(0x09, edp->edp + DP_PHY_CFG);
>  
> -	writel(0x20, edp->pll + QSERDES_V4_COM_RESETSM_CNTRL);
> +	if (cfg->qserdes_version == QSERDES_V6) {
> +		writel(0x20, edp->pll + QSERDES_V6_COM_RESETSM_CNTRL);
> +
> +		timeout = readl_poll_timeout(edp->pll + QSERDES_V6_COM_C_READY_STATUS,
> +					     val, val & BIT(0), 500, 10000);
> +	} else {
> +		writel(0x20, edp->pll + QSERDES_V4_COM_RESETSM_CNTRL);
> +
> +		timeout = readl_poll_timeout(edp->pll + QSERDES_V4_COM_C_READY_STATUS,
> +					     val, val & BIT(0), 500, 10000);
> +	}

Similar here.
  
> -	timeout = readl_poll_timeout(edp->pll + QSERDES_V4_COM_C_READY_STATUS,
> -				     val, val & BIT(0), 500, 10000);
>  	if (timeout)
>  		return timeout;
>  
> @@ -744,7 +935,6 @@ static int qcom_edp_clks_register(struct qcom_edp *edp, struct device_node *np)
>  	data = devm_kzalloc(edp->dev, struct_size(data, hws, 2), GFP_KERNEL);
>  	if (!data)
>  		return -ENOMEM;
> -	data->num = 2;
>  
>  	snprintf(name, sizeof(name), "%s::link_clk", dev_name(edp->dev));
>  	init.ops = &qcom_edp_dp_link_clk_ops;
> @@ -764,6 +954,7 @@ static int qcom_edp_clks_register(struct qcom_edp *edp, struct device_node *np)
>  
>  	data->hws[0] = &edp->dp_link_hw;
>  	data->hws[1] = &edp->dp_pixel_hw;
> +	data->num = 2;

Another seemingly unrelated change.

>  	return devm_of_clk_add_hw_provider(edp->dev, of_clk_hw_onecell_get, data);
>  }
> @@ -806,6 +997,7 @@ static int qcom_edp_phy_probe(struct platform_device *pdev)
>  
>  	edp->supplies[0].supply = "vdda-phy";
>  	edp->supplies[1].supply = "vdda-pll";
> +	edp->clks[2].id = "tcsr";

The *clocks* will already have been requested above so this is at best a
noop.

>  	ret = devm_regulator_bulk_get(dev, ARRAY_SIZE(edp->supplies), edp->supplies);
>  	if (ret)
>  		return ret;

Johan