Re: [PATCH 16/17] phy: qcom: edp: Introduce Nord phy_ver_ops and phy_cfg descriptor

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The common power-on flow unconditionally writes to the legacy TX_BAND offset, which corrupts registers on the Nord platform.
- [High] The Nord LDO configuration ignores DP mode and applies incorrect eDP settings, contradicting the Hardware Programming Guide.
--

commit ff420262cd547c67f8bafbb24b3a2db77628fd60
Author: Ritesh Kumar <ritesh.kumar@oss.qualcomm.com>

phy: qcom: edp: Introduce Nord phy_ver_ops and phy_cfg descriptor

Add qcom_edp_phy_ops_nord wiring all eight Nord-specific callbacks and
nord_edp_phy_cfg collecting the Nord AUX, VCO divider, swing/pre-emphasis
tables and ver_ops.

> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
> index 361f4e4673518..338f28cb404fb 100644
> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
> @@ -1847,6 +1847,29 @@ static int qcom_edp_phy_tx_res_cfg_nord(const struct qcom_edp *edp)
>  }
>  
>  
> +static const struct phy_ver_ops qcom_edp_phy_ops_nord = {
> +	.com_power_on		= qcom_edp_phy_power_on_nord,
> +	.com_resetsm_cntrl	= qcom_edp_phy_com_resetsm_cntrl_nord,
> +	.com_bias_en_clkbuflr	= qcom_edp_com_bias_en_clkbuflr_nord,
> +	.com_configure_pll	= qcom_edp_com_configure_pll_nord,
> +	.com_configure_ssc	= qcom_edp_com_configure_ssc_nord,
> +	.com_ldo_config		= qcom_edp_ldo_config_nord,

[Severity: High]
Does this hardcode the incorrect LDO configuration for DP mode?

The comments for qcom_edp_ldo_config_nord() document that DP mode must use
0x00 and eDP mode must use 0xD1.

However, the implementation seems to unconditionally hardcode 0xd0 and 0x03,
ignoring edp->is_edp completely:

qcom_edp_ldo_config_nord()
	ldo_config = 0xd0;
	phy_ldo_cfg = 0x03;

	writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);

Could this cause instability or link failures when running in DP mode due to
contradicting the hardware requirements?

> +	.phy_tx_lane_cfg	= qcom_edp_phy_tx_lane_cfg_nord,

[Severity: High]
Can the common power-on flow corrupt registers on the Nord platform here?

When qcom_edp_phy_power_on() runs, it calls this phy_tx_lane_cfg() callback
which correctly programs the Nord-specific TX_BAND offset at 0x0024:

qcom_edp_phy_power_on()
	if (edp->cfg->ver_ops->phy_tx_lane_cfg) {
		ret = edp->cfg->ver_ops->phy_tx_lane_cfg(edp);
        ...

But immediately afterward, the common flow unconditionally writes to the
legacy TX_BAND offset:

qcom_edp_phy_power_on()
	/* TX-0 register configuration */
    ...
	writel(0x04, edp->tx0 + TXn_TX_BAND);

Since TXn_TX_BAND moved to 0x0024 on Nord, writing to the legacy offset
(0x0028) might clobber whatever unknown register resides there now.

Would it be better to conditionally bypass the legacy TX_BAND write when the
Nord specific callback is used?

> +	.phy_tx_res_cfg		= qcom_edp_phy_tx_res_cfg_nord,
> +};
> +
> +static const struct qcom_edp_phy_cfg nord_edp_phy_cfg = {
> +	.is_edp			= false,
> +	.aux_cfg		= edp_phy_aux_cfg_nord,
> +	.vco_div_cfg		= edp_phy_vco_div_cfg_nord,
> +	.dp_swing_pre_emph_cfg	= &nord_dp_swing_pre_emph_cfg,
> +	.edp_swing_pre_emph_cfg	= &nord_edp_swing_pre_emph_cfg,
> +	.ver_ops		= &qcom_edp_phy_ops_nord,
> +	.phy_status_reg		= DP_PHY_STATUS_NORD,
> +	.bias1_en_2lane		= 0x03,
> +};

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911-nord-drv-dt-v1-0-e25c073cf1b6@oss.qualcomm.com?part=16