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

[Yongxing Mou]

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@xxxxxxxxxxxxxxxx>
> > ---
> >   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 = {