Re: [PATCH 2/2] phy: qcom: qmp-combo: Update SC8280XP USB3 PCS settings

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] phy: qcom: qmp-combo: PCS_USB offset in sc8280xp_usb43dp_pcs_tbl
--

commit 7e6adf1d452bf07884f60586ec434c1c30e6fd1b
Author: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>

phy: qcom: qmp-combo: Update SC8280XP USB3 PCS settings

This commit updates the USB3 PCS register initialization table for the
SC8280XP QMP combo PHY to match the latest hardware reference guide.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 2a8c39b81973f..6f83c9d4aa30c 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> @@ -1951,20 +1951,21 @@ static const struct qmp_phy_init_tbl sc8280xp_usb43dp_rx_tbl[] = {
>  static const struct qmp_phy_init_tbl sc8280xp_usb43dp_pcs_tbl[] = {
>  	QMP_PHY_INIT_CFG(QPHY_V5_PCS_RCVR_DTCT_DLY_P1U2_L, 0xe7),
>  	QMP_PHY_INIT_CFG(QPHY_V5_PCS_RCVR_DTCT_DLY_P1U2_H, 0x03),
> -	QMP_PHY_INIT_CFG(QPHY_V5_PCS_LOCK_DETECT_CONFIG1, 0xd0),
> -	QMP_PHY_INIT_CFG(QPHY_V5_PCS_LOCK_DETECT_CONFIG2, 0x07),
> +	QMP_PHY_INIT_CFG(QPHY_V5_PCS_LOCK_DETECT_CONFIG1, 0xc4),
> +	QMP_PHY_INIT_CFG(QPHY_V5_PCS_LOCK_DETECT_CONFIG2, 0x89),
>  	QMP_PHY_INIT_CFG(QPHY_V5_PCS_LOCK_DETECT_CONFIG3, 0x20),
>  	QMP_PHY_INIT_CFG(QPHY_V5_PCS_LOCK_DETECT_CONFIG6, 0x13),
>  	QMP_PHY_INIT_CFG(QPHY_V5_PCS_REFGEN_REQ_CONFIG1, 0x21),
> -	QMP_PHY_INIT_CFG(QPHY_V5_PCS_RX_SIGDET_LVL, 0xaa),
> +	QMP_PHY_INIT_CFG(QPHY_V5_PCS_RX_SIGDET_LVL, 0x33),
>  	QMP_PHY_INIT_CFG(QPHY_V5_PCS_RX_CONFIG, 0x0a),
> -	QMP_PHY_INIT_CFG(QPHY_V5_PCS_ALIGN_DETECT_CONFIG1, 0x88),
> -	QMP_PHY_INIT_CFG(QPHY_V5_PCS_ALIGN_DETECT_CONFIG2, 0x13),
> +	QMP_PHY_INIT_CFG(QPHY_V5_PCS_ALIGN_DETECT_CONFIG1, 0xd4),
> +	QMP_PHY_INIT_CFG(QPHY_V5_PCS_ALIGN_DETECT_CONFIG2, 0x30),
>  	QMP_PHY_INIT_CFG(QPHY_V5_PCS_PCS_TX_RX_CONFIG, 0x0c),
>  	QMP_PHY_INIT_CFG(QPHY_V5_PCS_EQ_CONFIG1, 0x4b),
>  	QMP_PHY_INIT_CFG(QPHY_V5_PCS_EQ_CONFIG5, 0x10),
>  	QMP_PHY_INIT_CFG(QPHY_V5_PCS_USB3_LFPS_DET_HIGH_COUNT_VAL, 0xf8),
>  	QMP_PHY_INIT_CFG(QPHY_V5_PCS_USB3_RXEQTRAINING_DFE_TIME_S2, 0x07),
> +	QMP_PHY_INIT_CFG(QPHY_V5_PCS_USB3_POWER_STATE_CONFIG1, 0x68),
>  };

[Severity: High]
Should QPHY_V5_PCS_USB3_POWER_STATE_CONFIG1 (and the two preceding
QPHY_V5_PCS_USB3_* entries) be placed in a separate pcs_usb_tbl rather than
sc8280xp_usb43dp_pcs_tbl?

In drivers/phy/qualcomm/phy-qcom-qmp-pcs-usb-v5.h,
QPHY_V5_PCS_USB3_POWER_STATE_CONFIG1 is defined at offset 0x000 (along with
QPHY_V5_PCS_USB3_LFPS_DET_HIGH_COUNT_VAL at 0x018 and
QPHY_V5_PCS_USB3_RXEQTRAINING_DFE_TIME_S2 at 0x03c) relative to the PCS_USB
register block (qmp->pcs_usb, offset 0x1700 in qmp_combo_offsets_v5).

In qmp_combo_usb_power_on(), pcs_tbl is written relative to qmp->pcs (offset
0x1400), while pcs_usb_tbl is written relative to qmp->pcs_usb:

	qmp_configure(qmp->dev, pcs, pcs_tbl, pcs_tbl_num);
	qmp_configure(qmp->dev, qmp->pcs_misc, cfg->pcs_misc_tbl, cfg->pcs_misc_tbl_num);

	if (qmp->qmpphy_mode == QMPPHY_MODE_USB4 && qmp->usb4_phy_submode == PHY_SUBMODE_TBT3)
		qmp_configure(qmp->dev, serdes, cfg->usb4_serdes_tb3_ovrd_tbl,
			      cfg->usb4_serdes_tb3_ovrd_num);
	else if (qmp->qmpphy_mode != QMPPHY_MODE_USB4 && pcs_usb)
		qmp_configure(qmp->dev, pcs_usb, cfg->pcs_usb_tbl,
			      cfg->pcs_usb_tbl_num);

Because sc8280xp_usb43dpphy_cfg only populates .pcs_tbl and omits
.pcs_usb_tbl, placing QPHY_V5_PCS_USB3_POWER_STATE_CONFIG1 in
sc8280xp_usb43dp_pcs_tbl causes qmp_combo_usb_power_on() to write 0x68 to
qmp->pcs + 0x000 (QPHY_V5_PCS_SW_RESET in
drivers/phy/qualcomm/phy-qcom-qmp-pcs-v5.h, where qphy_clrbits() subsequently
only clears BIT(0)) while leaving qmp->pcs_usb + 0x000 unprogrammed.
Similarly, the two existing QPHY_V5_PCS_USB3_* entries in
sc8280xp_usb43dp_pcs_tbl write to qmp->pcs + 0x018 and qmp->pcs + 0x03c
instead of qmp->pcs_usb.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261102-topic-usb3_sequence-v1-0-5b144f5e6865@oss.qualcomm.com?part=2

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