[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <nxoukhntz3mn6m5qca2lk6wubhwlmxoldh6xpyxx63voseiw35@5rrwfryx6vww>
Date: Sat, 7 Feb 2026 12:17:29 +0200
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Yongxing Mou <yongxing.mou@....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 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.
>
> 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
> + 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.
> };
>
> static const struct qcom_edp_phy_cfg sc8280xp_dp_phy_cfg = {
--
With best wishes
Dmitry
Powered by blists - more mailing lists