Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] phy: qcom-edp: legacy TXn_TX_BAND writes corrupt Nord hardware state - [High] phy: qcom-edp: hardcoded TXn_TX_DRV_LVL writes break voltage updates on Nord PHY --
commit 1393c79f29bd788a83ce23c45373ba39e69132d0 Author: Ritesh Kumar <[email protected]> phy: qcom: edp: Add Nord phy_cfg, ops and OF compatible This commit adds the configuration structures, operations, and device tree matching for the Nord DP PHY. It defines the Nord-specific tables for AUX, VCO, swing, and pre-emphasis, along with updated status register offsets. > diff --git a/drivers/phy/qualcomm/phy-qcom-edp.c > b/drivers/phy/qualcomm/phy-qcom-edp.c > index cb5f6e0ae607b..87400d61323a6 100644 > --- a/drivers/phy/qualcomm/phy-qcom-edp.c > +++ b/drivers/phy/qualcomm/phy-qcom-edp.c [ ... ] > @@ -1774,6 +1774,30 @@ static const struct qcom_edp_tx_res_entry > tx_res_cfg_nord[] = { > QCOM_EDP_TX_RES_ENTRY_END, > }; > > +static const struct phy_ver_ops qcom_edp_phy_ops_nord = { > + .com_power_on = qcom_edp_phy_power_on_v8, > + .com_resetsm_cntrl = qcom_edp_phy_com_resetsm_cntrl_v8, > + .com_bias_en_clkbuflr = qcom_edp_com_bias_en_clkbuflr_v8, > + .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, > +}; > + > +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, > + .tx2_tx3_lane_ctl = DP_PHY_TX2_TX3_LANE_CTL_NORD, > + .tx_res_cfg = tx_res_cfg_nord, [Severity: High] The tx_res_cfg_nord table correctly configures the relocated offset for TX_BAND (0x0024). However, does qcom_edp_phy_power_on() still unconditionally write to the legacy TX_BAND offset (0x0028) before applying the tx_res_cfg table? qcom_edp_phy_power_on() { ... writel(0x04, edp->tx0 + TXn_TX_BAND); ... writel(0x04, edp->tx1 + TXn_TX_BAND); ... } Could this cause a write to the wrong offset on Nord hardware, potentially corrupting whatever unrelated register now exists at 0x0028? Should this register offset be parameterized or moved entirely to tx_res_cfg? [Severity: High] Similarly, tx_res_cfg_nord initializes DP_PHY_TX_LN0_DRV_LVL_NORD (offset 0x00e4), indicating the drive level registers were also relocated. However, doesn't qcom_edp_set_voltages() still dynamically write the voltage swing values to the legacy TXn_TX_DRV_LVL offset (0x0014) during DP link training? qcom_edp_set_voltages() { ... writel(swing, edp->tx0 + TXn_TX_DRV_LVL); ... writel(swing, edp->tx1 + TXn_TX_DRV_LVL); ... } Will DP link training fail on Nord PHYs because the actual drive level register at 0x00e4 is never updated with the dynamic voltage levels? > + .cmn_status_reg = DP_QSERDES_V8_COM_CMN_STATUS_NORD, > + .c_ready_status_reg = DP_QSERDES_V8_COM_C_READY_STATUS_NORD, > +}; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=13
