Re: [PATCH 2/2] phy: qcom: edp: update v8 power-on programming sequence

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing handling for 1-lane eDP/DP configuration in the v8 PHY sequence, resulting in an inconsistent fallback to 4-lane configuration for tx1.
--

commit 86692efb0406428e7857a2fe90f5797a7385d5e6
Author: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com>

phy: qcom: edp: update v8 power-on programming sequence

This commit updates the v8 eDP PHY programming sequence to match documented 
hardware requirements. It adjusts settings across several configuration stages
including AUX, TX lanes, PCS, PLL, and DCC to improve support for various link
rates and lane configurations.

> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
> index 6588419d4860b..c5d1fa0ae6cb7 100644
> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c
[ ... ]
> @@ -427,6 +491,145 @@ static int qcom_edp_configure_pll(const struct qcom_edp *edp)
[ ... ]
> +static void qcom_edp_configure_lanes_after_pll_v8(const struct qcom_edp *edp)
> +{
> +	if (edp->dp_opts.lanes == 2) {
> +		writel(0x1b, edp->tx0 + TXn_HIGHZ_DRVR_EN);
[ ... ]
> +	} else {
> +		writel(0x1f, edp->tx0 + TXn_HIGHZ_DRVR_EN);
> +		writel(0x03, edp->tx0 + TXn_TRANSCEIVER_BIAS_EN);
> +		writel(0x04, edp->tx0 + TXn_HIGHZ_DRVR_EN);
> +		writel(0x00, edp->tx0 + TXn_TX_POL_INV);
> +
> +		writel(0x1f, edp->tx1 + TXn_HIGHZ_DRVR_EN);
> +		writel(0x03, edp->tx1 + TXn_TRANSCEIVER_BIAS_EN);

[Severity: Medium]
What happens if the DP framework requests a 1-lane configuration, such as
during a link training fallback?

Since there is no explicit check for edp->dp_opts.lanes == 1, a 1-lane
configuration will fall into this else block, which configures tx1 as if it
were in a 4-lane setup by enabling its transceiver driver and bias.

However, as seen in qcom_edp_ldo_config_v8() below, the tx1 LDO is correctly
kept unpowered (0x00) for lanes <= 2. Could this create an electrical conflict
or cause link training to fail, because the tx1 transceivers are enabled while
the LDO remains unpowered?

Should 1-lane mode be explicitly handled here or rejected with -EINVAL early
in the power-on sequence?

> +		writel(0x04, edp->tx1 + TXn_HIGHZ_DRVR_EN);
> +		writel(0x00, edp->tx1 + TXn_TX_POL_INV);
> +	}
> +}
[ ... ]
> @@ -911,6 +1114,22 @@ static int qcom_edp_ldo_config_v6(const struct qcom_edp *edp)
[ ... ]
> +static int qcom_edp_ldo_config_v8(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 = 0xd0;
> +	else
> +		ldo_config = 0x00;
> +
> +	writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
> +	writel(dp_opts->lanes > 2 ? ldo_config : 0x00, edp->tx1 + TXn_LDO_CONFIG);
> +
> +	return 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260622-glymur-edp-phy-v1-0-814b45089ac9@oss.qualcomm.com?part=2

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