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