[PATCH v2 08/13] phy: qcom: edp: Generalize power-on path for per-SoC overrides
Dmitry Baryshkov
dmitry.baryshkov at oss.qualcomm.com
Tue Oct 13 03:26:07 PDT 2026
On Mon, Oct 12, 2026 at 07:02:18PM +0530, Nabige Aala wrote:
> From: Ritesh Kumar <ritesh.kumar at oss.qualcomm.com>
>
> Nord has a different register layout that the existing hard-coded
> power-on path cannot accommodate. Introduce qcom_edp_tx_res_entry
> and replace open-coded TX resistor writes with a data-driven loop
> that also handles base=2 (PHY/edp base) for Nord. Add
> phy_status_reg, bias1_en_2lane, tx2_tx3_lane_ctl, tx_res_cfg,
> cmn_status_reg and c_ready_status_reg to qcom_edp_phy_cfg, and
> make com_clk_fwd_cfg optional. No functional change for existing
> SoCs.
This becomes too much table-driven, while the edp driver preferred the
code-driven approach.
>
> Assisted-by: LLM
> Signed-off-by: Ritesh Kumar <ritesh.kumar at oss.qualcomm.com>
> Signed-off-by: Nabige Aala <nabige.aala at oss.qualcomm.com>
> ---
> drivers/phy/qualcomm/phy-qcom-edp.c | 82 +++++++++++++++++++++++++++++--------
> 1 file changed, 66 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
> index d5c6835b6c70..c5df1086607f 100644
> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> @@ -81,6 +81,15 @@ struct qcom_edp_swing_pre_emph_cfg {
>
> struct qcom_edp;
>
> +struct qcom_edp_tx_res_entry {
> + u8 base;
> + u32 offset;
> + u32 val;
> +};
> +
> +#define QCOM_EDP_TX_RES_ENTRY_END { .offset = 0, .val = 0, .base = 0xff }
> +#define qcom_edp_tx_res_entry_is_end(e) ((e)->base == 0xff)
> +
> struct phy_ver_ops {
> int (*com_power_on)(const struct qcom_edp *edp);
> int (*com_resetsm_cntrl)(const struct qcom_edp *edp);
> @@ -98,6 +107,12 @@ struct qcom_edp_phy_cfg {
> const struct qcom_edp_swing_pre_emph_cfg *dp_swing_pre_emph_cfg;
> const struct qcom_edp_swing_pre_emph_cfg *edp_swing_pre_emph_cfg;
> const struct phy_ver_ops *ver_ops;
> + u32 phy_status_reg;
> + u8 bias1_en_2lane;
> + u32 tx2_tx3_lane_ctl;
> + const struct qcom_edp_tx_res_entry *tx_res_cfg;
> + u32 cmn_status_reg;
> + u32 c_ready_status_reg;
If anything, it should become a part of the ver_ops. But, I think, it's
better if you add callbacks to ver_ops instead of just describing regs.
> };
>
> struct qcom_edp {
> @@ -313,9 +328,11 @@ static int qcom_edp_phy_init(struct phy *phy)
>
> memcpy(aux_cfg, edp->cfg->aux_cfg, sizeof(aux_cfg));
>
> - ret = edp->cfg->ver_ops->com_clk_fwd_cfg(edp);
> - if (ret)
> - return ret;
> + if (edp->cfg->ver_ops->com_clk_fwd_cfg) {
> + ret = edp->cfg->ver_ops->com_clk_fwd_cfg(edp);
> + if (ret)
> + return ret;
> + }
This is fine, but why?
>
> writel(DP_PHY_PD_CTL_PWRDN | DP_PHY_PD_CTL_AUX_PWRDN |
> DP_PHY_PD_CTL_PLL_PWRDN | DP_PHY_PD_CTL_DP_CLAMP_EN,
> @@ -672,6 +689,16 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v4 = {
> .com_ldo_config = qcom_edp_ldo_config_v4,
> };
>
> +static const struct qcom_edp_tx_res_entry tx_res_cfg_default[] = {
> + { 0, TXn_RES_CODE_LANE_OFFSET_TX0, 0x11 },
> + { 0, TXn_RES_CODE_LANE_OFFSET_TX1, 0x11 },
> + { 1, TXn_RES_CODE_LANE_OFFSET_TX0, 0x11 },
> + { 1, TXn_RES_CODE_LANE_OFFSET_TX1, 0x11 },
> + { 0, TXn_TX_EMP_POST1_LVL, 0x10 },
> + { 1, TXn_TX_EMP_POST1_LVL, 0x10 },
> + QCOM_EDP_TX_RES_ENTRY_END,
> +};
This becomes hard to manage. Please use ops instead.
> +
> static const struct qcom_edp_phy_cfg sa8775p_dp_phy_cfg = {
> .is_edp = false,
> .aux_cfg = edp_phy_aux_cfg_v5,
> @@ -679,6 +706,9 @@ static const struct qcom_edp_phy_cfg sa8775p_dp_phy_cfg = {
> .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
> .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
> .ver_ops = &qcom_edp_phy_ops_v4,
> + .phy_status_reg = DP_PHY_STATUS,
> + .tx2_tx3_lane_ctl = DP_PHY_TX2_TX3_LANE_CTL,
> + .tx_res_cfg = tx_res_cfg_default,
> };
>
> static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = {
> @@ -687,6 +717,9 @@ static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = {
> .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
> .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg_v3,
> .ver_ops = &qcom_edp_phy_ops_v3,
> + .phy_status_reg = DP_PHY_STATUS,
> + .tx2_tx3_lane_ctl = DP_PHY_TX2_TX3_LANE_CTL,
> + .tx_res_cfg = tx_res_cfg_default,
> };
>
> static const struct qcom_edp_phy_cfg sc8180x_dp_phy_cfg = {
> @@ -695,6 +728,9 @@ static const struct qcom_edp_phy_cfg sc8180x_dp_phy_cfg = {
> .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg_v2,
> .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg_v2,
> .ver_ops = &qcom_edp_phy_ops_v3,
> + .phy_status_reg = DP_PHY_STATUS,
> + .tx2_tx3_lane_ctl = DP_PHY_TX2_TX3_LANE_CTL,
> + .tx_res_cfg = tx_res_cfg_default,
> };
>
> static const struct qcom_edp_phy_cfg sc8280xp_dp_phy_cfg = {
> @@ -703,6 +739,9 @@ static const struct qcom_edp_phy_cfg sc8280xp_dp_phy_cfg = {
> .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
> .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
> .ver_ops = &qcom_edp_phy_ops_v4,
> + .phy_status_reg = DP_PHY_STATUS,
> + .tx2_tx3_lane_ctl = DP_PHY_TX2_TX3_LANE_CTL,
> + .tx_res_cfg = tx_res_cfg_default,
> };
>
> static const struct qcom_edp_phy_cfg sc8280xp_edp_phy_cfg = {
> @@ -712,6 +751,9 @@ static const struct qcom_edp_phy_cfg sc8280xp_edp_phy_cfg = {
> .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
> .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
> .ver_ops = &qcom_edp_phy_ops_v4,
> + .phy_status_reg = DP_PHY_STATUS,
> + .tx2_tx3_lane_ctl = DP_PHY_TX2_TX3_LANE_CTL,
> + .tx_res_cfg = tx_res_cfg_default,
> };
>
> static int qcom_edp_phy_power_on_v6(const struct qcom_edp *edp)
> @@ -913,6 +955,9 @@ static struct qcom_edp_phy_cfg x1e80100_phy_cfg = {
> .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg,
> .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
> .ver_ops = &qcom_edp_phy_ops_v6,
> + .phy_status_reg = DP_PHY_STATUS,
> + .tx2_tx3_lane_ctl = DP_PHY_TX2_TX3_LANE_CTL,
> + .tx_res_cfg = tx_res_cfg_default,
> };
>
> static int qcom_edp_com_configure_ssc_v8(const struct qcom_edp *edp)
> @@ -1045,8 +1090,8 @@ static int qcom_edp_phy_com_resetsm_cntrl_v8(const struct qcom_edp *edp)
>
> writel(0x20, edp->pll + DP_QSERDES_V8_COM_RESETSM_CNTRL);
>
> - return readl_poll_timeout(edp->pll + DP_QSERDES_V8_COM_C_READY_STATUS,
> - val, val & BIT(0), 500, 10000);
> + return readl_poll_timeout(edp->pll + edp->cfg->c_ready_status_reg,
> + val, val & BIT(0), 500, 10000);
This is already a version-specific callback. Can't you just add one for
Nord?
> }
>
> static int qcom_edp_com_clk_fwd_cfg_v8(const struct qcom_edp *edp)
> @@ -1074,8 +1119,8 @@ static int qcom_edp_phy_power_on_v8(const struct qcom_edp *edp)
> edp->edp + DP_PHY_PD_CTL);
> writel(0xfc, edp->edp + DP_PHY_MODE);
>
> - return readl_poll_timeout(edp->pll + DP_QSERDES_V8_COM_CMN_STATUS,
> - val, val & BIT(7), 5, 200);
> + return readl_poll_timeout(edp->pll + edp->cfg->cmn_status_reg,
> + val, val & BIT(7), 5, 200);
The same.
> }
>
> static const struct phy_ver_ops qcom_edp_phy_ops_v8 = {
> @@ -1094,6 +1139,11 @@ static struct qcom_edp_phy_cfg glymur_phy_cfg = {
> .dp_swing_pre_emph_cfg = &dp_phy_swing_pre_emph_cfg_v8,
> .edp_swing_pre_emph_cfg = &edp_phy_swing_pre_emph_cfg,
> .ver_ops = &qcom_edp_phy_ops_v8,
> + .phy_status_reg = DP_PHY_STATUS,
> + .tx2_tx3_lane_ctl = DP_PHY_TX2_TX3_LANE_CTL,
> + .tx_res_cfg = tx_res_cfg_default,
> + .cmn_status_reg = DP_QSERDES_V8_COM_CMN_STATUS,
> + .c_ready_status_reg = DP_QSERDES_V8_COM_C_READY_STATUS,
> };
>
> static int qcom_edp_phy_power_on(struct phy *phy)
> @@ -1128,7 +1178,7 @@ static int qcom_edp_phy_power_on(struct phy *phy)
>
> /* TX Lane configuration */
> writel(0x05, edp->edp + DP_PHY_TX0_TX1_LANE_CTL);
> - writel(0x05, edp->edp + DP_PHY_TX2_TX3_LANE_CTL);
> + writel(0x05, edp->edp + edp->cfg->tx2_tx3_lane_ctl);
This one is almost fine.
>
> /* TX-0 register configuration */
> writel(0x03, edp->tx0 + TXn_TRANSCEIVER_BIAS_EN);
> @@ -1166,13 +1216,13 @@ static int qcom_edp_phy_power_on(struct phy *phy)
> writel(0x00, edp->tx1 + TXn_TX_POL_INV);
> writel(0x10, edp->tx0 + TXn_TX_DRV_LVL_OFFSET);
> writel(0x10, edp->tx1 + TXn_TX_DRV_LVL_OFFSET);
> - writel(0x11, edp->tx0 + TXn_RES_CODE_LANE_OFFSET_TX0);
> - writel(0x11, edp->tx0 + TXn_RES_CODE_LANE_OFFSET_TX1);
> - writel(0x11, edp->tx1 + TXn_RES_CODE_LANE_OFFSET_TX0);
> - writel(0x11, edp->tx1 + TXn_RES_CODE_LANE_OFFSET_TX1);
> + for (const struct qcom_edp_tx_res_entry *e = edp->cfg->tx_res_cfg;
> + !qcom_edp_tx_res_entry_is_end(e); e++) {
> + void __iomem *base = e->base == 2 ? edp->edp :
> + e->base ? edp->tx1 : edp->tx0;
>
> - writel(0x10, edp->tx0 + TXn_TX_EMP_POST1_LVL);
> - writel(0x10, edp->tx1 + TXn_TX_EMP_POST1_LVL);
> + writel(e->val, base + e->offset);
> + }
Unreadable. Once we move on, these regs can be used to write anything.
If it's a really one-off for Nord, just check for the compatible string
or do something similar. I'll check the other patch.
> writel(0x1f, edp->tx0 + TXn_TX_DRV_LVL);
> writel(0x1f, edp->tx1 + TXn_TX_DRV_LVL);
>
> @@ -1184,7 +1234,7 @@ static int qcom_edp_phy_power_on(struct phy *phy)
> cfg1 = 0x1;
> } else if (edp->dp_opts.lanes == 2) {
> bias0_en = 0x03;
> - bias1_en = 0x00;
> + bias1_en = edp->cfg->bias1_en_2lane;
> drvr0_en = 0x04;
> drvr1_en = 0x07;
> cfg1 = 0x3;
> @@ -1207,7 +1257,7 @@ static int qcom_edp_phy_power_on(struct phy *phy)
>
> writel(0x19, edp->edp + DP_PHY_CFG);
>
> - ret = readl_poll_timeout(edp->edp + DP_PHY_STATUS,
> + ret = readl_poll_timeout(edp->edp + edp->cfg->phy_status_reg,
> val, val & BIT(1), 500, 10000);
> if (ret)
> return ret;
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
More information about the linux-phy
mailing list