Re: [PATCH 2/3] phy: qcom: qmp-combo: Select LFPS detect only when SS link is confirmed present

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] phy: qcom: qmp-combo: invalid PCS_MISC clamp register access in
  autonomous mode [1]
--

commit ee7ffdc4089bfb74e59a7747c5a2c1ab2e3f8758
Author: Tapas Rana <tapas.rana@oss.qualcomm.com>

phy: qcom: qmp-combo: Select LFPS detect only when SS link is confirmed present

This commit stores the qcom_usb_ss_state submode in qmp_combo_usb_set_mode()
and uses it in qmp_combo_enable_autonomous_mode() to enable LFPS detection
only when a SuperSpeed link is confirmed present.

> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> index 187f0cd11f6c3..05d48ba81719a 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c

[ ... ]

> @@ -4657,11 +4669,20 @@ static void qmp_combo_enable_autonomous_mode(struct qmp_combo *qmp)
>  	void __iomem *pcs_aon = qmp->pcs_aon;
>  	u32 intr_mask;
>  
> -	if (qmp->phy_mode == PHY_MODE_USB_HOST_SS ||
> -	    qmp->phy_mode == PHY_MODE_USB_DEVICE_SS)
> +	/*
> +	 * Use LFPS detection only when a SuperSpeed device is known to be
> +	 * present. For absent or unknown state, use receiver detection.
> +	 */
> +	switch (qmp->ss_state) {
> +	case QCOM_USB_SS_PRESENT:
>  		intr_mask = ARCVR_DTCT_EN | ALFPS_DTCT_EN;
> -	else
> +		break;
> +	case QCOM_USB_SS_ABSENT:
> +	case QCOM_USB_SS_UNKNOWN:
> +	default:
>  		intr_mask = ARCVR_DTCT_EN | ARCVR_DTCT_EVENT_SEL;
> +		break;
> +	}
>  
>  	/* Clear any pending interrupts status */
>  	qphy_setbits(pcs_usb, cfg->regs[QPHY_PCS_LFPS_RXTERM_IRQ_CLEAR], IRQ_CLEAR);

[Severity: High]
This problem wasn't introduced by this patch, but can
qmp_combo_enable_autonomous_mode() and qmp_combo_disable_autonomous_mode()
corrupt the PCS_MISC block [1] on V6 platforms that do not define
usb3_pcs_aon?

drivers/phy/qualcomm/phy-qcom-qmp-combo.c:qmp_combo_enable_autonomous_mode() {
	...
	if (pcs_aon) {
		if (qmp->qmpphy_mode == QMPPHY_MODE_USB4)
			qphy_clrbits(pcs_aon, cfg->regs[QPHY_PCS_USB4_CLAMP_ENABLE], CLAMP_EN);
		else
			qphy_clrbits(pcs_aon, cfg->regs[QPHY_PCS_CLAMP_ENABLE], CLAMP_EN);
	} else if (pcs_misc)
		qphy_clrbits(pcs_misc, cfg->regs[QPHY_PCS_CLAMP_ENABLE], CLAMP_EN);
}

Configs such as sm8550_usb3dpphy_cfg and x1e80100_usb3dpphy_cfg pair
qmp_v6_usb3phy_regs_layout or qmp_v6_n4_usb3phy_regs_layout (where
QPHY_PCS_CLAMP_ENABLE is QPHY_V6_PCS_AON_CLAMP_ENABLE, 0x00) with offset
tables that leave usb3_pcs_aon at 0.  In qmp_combo_parse_dt(), qmp->pcs_aon
remains NULL while qmp->pcs_misc is populated unconditionally.

During runtime suspend and resume, the else if (pcs_misc) fallback writes
CLAMP_EN to offset 0x00 of pcs_misc (QPHY_V4_PCS_MISC_TYPEC_CTRL),
corrupting the Type-C control register instead of toggling the hardware
I/O clamp.

[1] https://sashiko.dev/#/bug/linux-975e0709-1f2c-4dcf-beb9-ae3f0e032aa9

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106082716.128968-1-tapas.rana@oss.qualcomm.com?part=2