Re: [PATCH 2/3] phy: qcom: edp: Add per-version LDO configuration callback

[Yongxing Mou <yongxing.mou@oss.qualcomm.com>]

On 2/7/2026 6:17 PM, Dmitry Baryshkov wrote:
> On Thu, Feb 05, 2026 at 05:20:54PM +0800, Yongxing Mou wrote:
>> Introduce the com_ldo_config callback to support per‑PHY LDO
>> configuration.
> 
> Missing the why part. Is the driver programming incorrect values, or is
> it an optimisation? In the former case it needs Fixes, maybe cc:stable,
> maybe Reported-by, etc.
> 
Sure.. will add next version
>>
>> Signed-off-by: Yongxing Mou <yongxing.mou@oss.qualcomm.com>
>> ---
>>   drivers/phy/qualcomm/phy-qcom-edp.c | 86 ++++++++++++++++++++++++++++++++-----
>>   1 file changed, 76 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
>> index 10cbb7d9a8a0..388226dbad7f 100644
>> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
>> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
>> @@ -81,6 +81,7 @@ struct phy_ver_ops {
>>   	int (*com_clk_fwd_cfg)(const struct qcom_edp *edp);
>>   	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);
>>   };
>>   
>>   struct qcom_edp_phy_cfg {
>> @@ -273,7 +274,7 @@ static int qcom_edp_set_voltages(struct qcom_edp *edp, const struct phy_configur
>>   	const struct qcom_edp_swing_pre_emph_cfg *cfg = edp->cfg->swing_pre_emph_cfg;
>>   	unsigned int v_level = 0;
>>   	unsigned int p_level = 0;
>> -	u8 ldo_config;
>> +	int ret;
>>   	u8 swing;
>>   	u8 emph;
>>   	int i;
>> @@ -300,13 +301,13 @@ static int qcom_edp_set_voltages(struct qcom_edp *edp, const struct phy_configur
>>   	if (swing == 0xff || emph == 0xff)
>>   		return -EINVAL;
>>   
>> -	ldo_config = edp->is_edp ? 0x0 : 0x1;
>> +	ret = edp->cfg->ver_ops->com_ldo_config(edp);
>> +	if (ret)
>> +		return ret;
>>   
>> -	writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
>>   	writel(swing, edp->tx0 + TXn_TX_DRV_LVL);
>>   	writel(emph, edp->tx0 + TXn_TX_EMP_POST1_LVL);
>>   
>> -	writel(ldo_config, edp->tx1 + TXn_LDO_CONFIG);
>>   	writel(swing, edp->tx1 + TXn_TX_DRV_LVL);
>>   	writel(emph, edp->tx1 + TXn_TX_EMP_POST1_LVL);
>>   
>> @@ -530,6 +531,52 @@ static int qcom_edp_com_configure_pll_v4(const struct qcom_edp *edp)
>>   	return 0;
>>   }
>>   
>> +static int qcom_edp_ldo_config_v3(const struct qcom_edp *edp)
>> +{
>> +	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
>> +	u32 ldo_config;
>> +
>> +	if (!edp->is_edp)
>> +		ldo_config = 0x0;
>> +	else if (dp_opts->link_rate <= 2700)
>> +		ldo_config = 0x81;
>> +	else
>> +		ldo_config = 0x41;
>> +
>> +	writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
>> +	writel(ldo_config, edp->tx1 + TXn_LDO_CONFIG);
>> +
>> +	return 0;
>> +}
>> +
>> +static int qcom_edp_ldo_config_v4(const struct qcom_edp *edp)
>> +{
>> +	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
>> +	u32 ldo_config;
>> +
>> +	if (!edp->is_edp)
>> +		ldo_config = 0x0;
>> +	else if (dp_opts->link_rate <= 2700)
>> +		ldo_config = 0xC1;
> 
> Lowercase hex
> 
Got it . will fix
>> +	else
>> +		ldo_config = 0x81;
>> +
>> +	writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
>> +	writel(ldo_config, edp->tx1 + TXn_LDO_CONFIG);
>> +
>> +	return 0;
>> +}
>> +
>> +static const struct phy_ver_ops qcom_edp_phy_ops_v3 = {
>> +	.com_power_on		= qcom_edp_phy_power_on_v4,
>> +	.com_resetsm_cntrl	= qcom_edp_phy_com_resetsm_cntrl_v4,
>> +	.com_bias_en_clkbuflr	= qcom_edp_com_bias_en_clkbuflr_v4,
>> +	.com_clk_fwd_cfg	= qcom_edp_com_clk_fwd_cfg_v4,
>> +	.com_configure_pll	= qcom_edp_com_configure_pll_v4,
>> +	.com_configure_ssc	= qcom_edp_com_configure_ssc_v4,
>> +	.com_ldo_config		= qcom_edp_ldo_config_v3,
>> +};
>> +
>>   static const struct phy_ver_ops qcom_edp_phy_ops_v4 = {
>>   	.com_power_on		= qcom_edp_phy_power_on_v4,
>>   	.com_resetsm_cntrl	= qcom_edp_phy_com_resetsm_cntrl_v4,
>> @@ -537,6 +584,7 @@ static const struct phy_ver_ops qcom_edp_phy_ops_v4 = {
>>   	.com_clk_fwd_cfg	= qcom_edp_com_clk_fwd_cfg_v4,
>>   	.com_configure_pll	= qcom_edp_com_configure_pll_v4,
>>   	.com_configure_ssc	= qcom_edp_com_configure_ssc_v4,
>> +	.com_ldo_config		= qcom_edp_ldo_config_v4,
>>   };
>>   
>>   static const struct qcom_edp_phy_cfg sa8775p_dp_phy_cfg = {
>> @@ -550,7 +598,7 @@ static const struct qcom_edp_phy_cfg sa8775p_dp_phy_cfg = {
>>   static const struct qcom_edp_phy_cfg sc7280_dp_phy_cfg = {
>>   	.aux_cfg = edp_phy_aux_cfg_v4,
>>   	.vco_div_cfg = edp_phy_vco_div_cfg_v4,
>> -	.ver_ops = &qcom_edp_phy_ops_v4,
>> +	.ver_ops = &qcom_edp_phy_ops_v3,
> 
> This looks like an extra change. Is it intentional in this patch? If so,
> mention it in the commit message.
> 
Yes.. due to kodiak ldo func not compatable with other phys, will update 
the commit message.
>>   };
>>   
>>   static const struct qcom_edp_phy_cfg sc8280xp_dp_phy_cfg = {
> 


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy