[PATCH 08/17] phy: qcom: edp: Generalize power-on path to support per-SoC overrides

[Nabige Aala]

On 9/14/2026 5:30 AM, Dmitry Baryshkov wrote:
> On Fri, Sep 11, 2026 at 09:06:39PM +0530, Nabige Aala wrote:
>> From: Ritesh Kumar <ritesh.kumar at oss.qualcomm.com>
>>
>> Nord has a different register layout and power-on sequence that the
>> existing hard-coded paths cannot accommodate.  Make com_clk_fwd_cfg
>> optional,
> Why?
As per the HPG, Nord does not require the com_clk_fwd_cfg callback 
implementation. However, qcom_edp_phy_init() currently invokes this 
callback unconditionally, without checking whether it is present.

I see two possible ways to address this:

1. Add a dummy com_clk_fwd_cfg implementation for Nord.
2. Check whether the com_clk_fwd_cfg callback is present before invoking 
it. This is the approach I have taken in this patch.

Do you have a preference between these approaches? Alternatively, if you 
think there is a better way to handle this, I would appreciate your 
guidance.
>
>> add phy_tx_lane_cfg and phy_tx_res_cfg callbacks to
>> phy_ver_ops, and add phy_status_reg and bias1_en_2lane fields to
>> qcom_edp_phy_cfg so each SoC can override the defaults.  No
>> functional change for existing SoCs; new fields default to zero/NULL.
>>
>> 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 | 43 ++++++++++++++++++++++++++-----------
>>   1 file changed, 30 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
>> index bf8a17336c7e..373f13b3d6d5 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
>> @@ -92,6 +92,8 @@ struct phy_ver_ops {
>>   	int (*com_configure_pll)(const struct qcom_edp *edp);
>>   	int (*com_configure_ssc)(const struct qcom_edp *edp);
>>   	int (*com_ldo_config)(const struct qcom_edp *edp);
>> +	int (*phy_tx_lane_cfg)(const struct qcom_edp *edp);
>> +	int (*phy_tx_res_cfg)(const struct qcom_edp *edp);
>>   };
>>   
>>   struct qcom_edp_phy_cfg {
>> @@ -101,6 +103,8 @@ 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;
>>   };
>>   
>>   struct qcom_edp {
>> @@ -316,9 +320,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) {
> Why?
>
>> +		ret = edp->cfg->ver_ops->com_clk_fwd_cfg(edp);
>> +		if (ret)
>> +			return ret;
>> +	}
>>   
>>   	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,
>> @@ -1131,7 +1137,13 @@ 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);
>> +	if (edp->cfg->ver_ops->phy_tx_lane_cfg) {
>> +		ret = edp->cfg->ver_ops->phy_tx_lane_cfg(edp);
>> +		if (ret)
>> +			return ret;
>> +	} else {
>> +		writel(0x05, edp->edp + DP_PHY_TX2_TX3_LANE_CTL);
> if it's about a single register, add a register to the map instead.
Ack, I will update this in the next revision.
>
>> +	}
>>   
>>   	/* TX-0 register configuration */
>>   	writel(0x03, edp->tx0 + TXn_TRANSCEIVER_BIAS_EN);
>> @@ -1169,13 +1181,18 @@ 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);
>> -
>> -	writel(0x10, edp->tx0 + TXn_TX_EMP_POST1_LVL);
>> -	writel(0x10, edp->tx1 + TXn_TX_EMP_POST1_LVL);
>> +	if (edp->cfg->ver_ops->phy_tx_res_cfg) {
>> +		ret = edp->cfg->ver_ops->phy_tx_res_cfg(edp);
>> +		if (ret)
>> +			return ret;
>> +	} else {
>> +		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);
>> +		writel(0x10, edp->tx0 + TXn_TX_EMP_POST1_LVL);
>> +		writel(0x10, edp->tx1 + TXn_TX_EMP_POST1_LVL);
> Either use a register map or move defaults to the callback too. My
> preference is towards the former option.
Ack, I will use the register map approach
>
>> +	}
>>   	writel(0x1f, edp->tx0 + TXn_TX_DRV_LVL);
>>   	writel(0x1f, edp->tx1 + TXn_TX_DRV_LVL);
>>   
>> @@ -1187,7 +1204,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;
>> @@ -1210,7 +1227,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 ?: DP_PHY_STATUS),
> No hidden defaults, thank you.
>
>>   				 val, val & BIT(1), 500, 10000);
>>   	if (ret)
>>   		return ret;
>>
>> -- 
>> 2.34.1
>>