Re: [PATCH v7 35/35] soc: qcom: pmic-glink-altmode: skip retimer reset on DP IRQ

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] soc: qcom: pmic-glink-altmode: destructive retimer reset on non-IRQ updates
--

commit 040403d2f53703bec3a8c1b6f7b327a08f248957
Author: Yongxing Mou <yongxing.mou@oss.qualcomm.com>

soc: qcom: pmic-glink-altmode: skip retimer reset on DP IRQ

A DP HPD IRQ reports a sink-side event and does not imply a Type-C routing
change. Reprogramming the retimer for every IRQ can reset or disturb its
datapath while link training or video transmission is in progress, causing
intermittent DisplayPort link failures.

> diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c
> index e903d4910e9c8..5ae554dc01d9a 100644
> --- a/drivers/soc/qcom/pmic_glink_altmode.c
> +++ b/drivers/soc/qcom/pmic_glink_altmode.c
[ ... ]
> @@ -200,10 +207,31 @@ static void pmic_glink_altmode_enable_dp(struct pmic_glink_altmode *altmode,
>  	port->retimer_state.alt = &port->dp_alt;
>  	port->retimer_state.data = &dp_data;
>  	port->retimer_state.mode = TYPEC_MODAL_STATE(mode);
> +	retimer_state_unchanged = port->retimer_dp_valid &&
> +		port->retimer_dp_mode == mode &&
> +		port->retimer_dp_hpd_state == hpd_state &&
> +		port->retimer_dp_orientation == port->orientation;
> +
> +	/*
> +	 * An HPD IRQ is a sink-side DP interrupt, not a Type-C routing change.
> +	 * Avoid rewriting an already configured retimer while DP link training or
> +	 * video transmission is in progress. The DRM HPD IRQ notification below
> +	 * is deliberately still delivered.
> +	 */
> +	if (hpd_irq && retimer_state_unchanged)
> +		return;

[Severity: High]
Does this code unnecessarily reprogram the DP retimer when the IRQ status
is cleared or when redundant state notifications arrive?

The retimer datapath is initially programmed with hpd_irq = 0. When an HPD IRQ
occurs (hpd_irq = 1), typec_retimer_set() is correctly skipped.

However, when the IRQ status is subsequently cleared (or a redundant state
notification arrives) with hpd_irq = 0, the condition if (hpd_irq &&
retimer_state_unchanged) evaluates to false. This circumvents the cache check
and forces a call to typec_retimer_set() with the exact same DP state.

As noted in the commit message, reprogramming the retimer disrupts the active
link, which seems to defeat the core purpose of this patch.

>  
>  	ret = typec_retimer_set(port->typec_retimer, &port->retimer_state);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260929-msm-dp-mst-v7-0-4b81473185e7@oss.qualcomm.com?part=35