Re: [PATCH 15/17] phy: qcom: edp: Add Nord LDO, TX lane and TX resistor calibration callbacks

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `qcom_edp_ldo_config_nord()` function unconditionally hardcodes LDO configuration values, contradicting the requirements documented in its block comment and the commit message.
--

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

phy: qcom: edp: Add Nord LDO, TX lane and TX resistor calibration callbacks

This commit adds PHY configuration callbacks for the Nord platform, introducing
specific LDO levels, TX lane control offsets, and TX resistor calibration codes
based on HPG Table 2-1-d.

> diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c b/drivers/phy/qualcomm/phy-qcom-edp.c
> index 4d1b2c81d9563..361f4e4673518 100644
> --- a/drivers/phy/qualcomm/phy-qcom-edp.c
> +++ b/drivers/phy/qualcomm/phy-qcom-edp.c

[ ... ]

> +/*
> + * nord LDO configuration.
> + * From HPG Table 2-1-d: LDO Configuration section.
> + * eDP mode: 0xD1 for rates <= 8.1 Gbps, 0x00 for UHBR (>8.1 Gbps).
> + * DP mode: always 0x00.
> + * Also programs EDP2_DP_PHY_LDO_CFG: 0x03 for eDP rates <= 8.1G, 0x00 otherwise.
> + */
> +static int qcom_edp_ldo_config_nord(const struct qcom_edp *edp)
> +{
> +	const struct phy_configure_opts_dp *dp_opts = &edp->dp_opts;
> +	u32 ldo_config;
> +	u32 phy_ldo_cfg;
> +
> +	ldo_config = 0xd0;
> +	phy_ldo_cfg = 0x03;

[Severity: High]
The block comment directly above specifies that the LDO configuration
should be set dynamically based on the operating mode and link rate, such as
0xD1 for eDP rates <= 8.1 Gbps and 0x00 for UHBR or DP mode.

Does unconditionally hardcoding these values to 0xd0 (which also appears to be
a typo for 0xD1) and 0x03 cause incorrect LDO voltages to be applied when the
PHY is initialized for DP mode or high-speed eDP links?

Applying these values unconditionally during device probe or link training
might lead to PHY instability or link failures when operating in DP mode
or high-speed eDP modes where the hardware programming guide requires 0x00.

> +
> +	writel(ldo_config, edp->tx0 + TXn_LDO_CONFIG);
> +	writel(dp_opts->lanes > 2 ? ldo_config : 0x00, edp->tx1 + TXn_LDO_CONFIG);
> +	writel(phy_ldo_cfg, edp->edp + DP_PHY_LDO_CFG);
> +
> +	return 0;
> +}

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