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 <[email protected]> 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/[email protected]?part=15
