[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <c2d50c56-5a3b-41f4-90a8-1ebdff4dd898@oss.qualcomm.com>
Date: Tue, 10 Feb 2026 18:21:58 +0800
From: Yongxing Mou <yongxing.mou@....qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Vinod Koul <vkoul@...nel.org>, Neil Armstrong
<neil.armstrong@...aro.org>,
linux-arm-msm@...r.kernel.org, linux-phy@...ts.infradead.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] phy: qcom: edp: Add per-version LDO configuration
callback
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@....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 = {
>
Powered by blists - more mailing lists