drivers/soc/qcom/pmic_glink_altmode.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-)
From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
The commit referenced in Fixes started evaluating the value of
alt_port->mux_ctrl before checking the active SVID. This led to
drm_aux_hpd_bridge_notify() no longer being called for the 'DP unplug'
case.
Perhaps somewhat interestingly, the firmware sends a notification with
SVID=DP, mux_ctrl=MUX_CTRL_STATE_NO_CONN and pin_assignment=0 on
unplug. 'pin_assignment' was previously interpreted as a bitfield
excerpt from the second byte of the DP pg_altmode payload (and stored
as an u8).
That value is used in pmic_glink_altmode_sc8280xp_notify(), decremented
by 1 (DPAM_HPD_A). Previously, this would result in an u8 underflow
that would rollover to 0xff (which prior to the Fixes patch would have
caused a pmic_glink_altmode_safe() and 'disconnected' bridge
notification). That check was removed, without a replacement.
Resolve this issue by making sure the SID=DP && mux_ctrl=NO_CONN combo
once again results in a HPD bridge notification.
Fixes: 0539c5a6fdef ("soc: qcom: pmic_glink_altmode: Consume TBT3/USB4 mode notifications")
Reported-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Tested-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
---
drivers/soc/qcom/pmic_glink_altmode.c | 18 +++++++++++-------
1 file changed, 11 insertions(+), 7 deletions(-)
diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c
index d0afdcb96ee1..b496b88842a2 100644
--- a/drivers/soc/qcom/pmic_glink_altmode.c
+++ b/drivers/soc/qcom/pmic_glink_altmode.c
@@ -350,15 +350,17 @@ static void pmic_glink_altmode_worker(struct work_struct *work)
typec_switch_set(alt_port->typec_switch, alt_port->orientation);
- if (alt_port->mux_ctrl == MUX_CTRL_STATE_NO_CONN) {
- pmic_glink_altmode_safe(altmode, alt_port);
- } else if (alt_port->svid == USB_TYPEC_TBT_SID) {
+ if (alt_port->svid == USB_TYPEC_TBT_SID) {
pmic_glink_altmode_enable_tbt(altmode, alt_port);
} else if (alt_port->svid == USB_TYPEC_DP_SID) {
- pmic_glink_altmode_enable_dp(altmode, alt_port,
- alt_port->mode,
- alt_port->hpd_state,
- alt_port->hpd_irq);
+ if (alt_port->mux_ctrl == MUX_CTRL_STATE_NO_CONN) {
+ pmic_glink_altmode_safe(altmode, alt_port);
+ } else {
+ pmic_glink_altmode_enable_dp(altmode, alt_port,
+ alt_port->mode,
+ alt_port->hpd_state,
+ alt_port->hpd_irq);
+ }
if (alt_port->hpd_state)
conn_status = connector_status_connected;
@@ -368,6 +370,8 @@ static void pmic_glink_altmode_worker(struct work_struct *work)
drm_aux_hpd_bridge_notify(&alt_port->bridge->dev, conn_status);
} else if (alt_port->mux_ctrl == MUX_CTRL_STATE_TUNNELING) {
pmic_glink_altmode_enable_usb4(altmode, alt_port);
+ } else if (alt_port->mux_ctrl == MUX_CTRL_STATE_NO_CONN) {
+ pmic_glink_altmode_safe(altmode, alt_port);
} else {
pmic_glink_altmode_enable_usb(altmode, alt_port);
}
---
base-commit: fc7b1a72c6cd5cbbd989c6c32a6486e3e4e3594d
change-id: 20260306-topic-pgaltmode_fixup-6b488960e355
Best regards,
--
Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
On Fri, 06 Mar 2026 12:20:14 +0100, Konrad Dybcio wrote:
> The commit referenced in Fixes started evaluating the value of
> alt_port->mux_ctrl before checking the active SVID. This led to
> drm_aux_hpd_bridge_notify() no longer being called for the 'DP unplug'
> case.
>
> Perhaps somewhat interestingly, the firmware sends a notification with
> SVID=DP, mux_ctrl=MUX_CTRL_STATE_NO_CONN and pin_assignment=0 on
> unplug. 'pin_assignment' was previously interpreted as a bitfield
> excerpt from the second byte of the DP pg_altmode payload (and stored
> as an u8).
>
> [...]
Applied, thanks!
[1/1] soc: qcom: pmic_glink_altmode: Fix SVID=DP && unconnected edge case
commit: d487085006109e5981e059476818243759d2e925
Best regards,
--
Bjorn Andersson <andersson@kernel.org>
On 3/6/26 12:20, Konrad Dybcio wrote:
> From: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
>
> The commit referenced in Fixes started evaluating the value of
> alt_port->mux_ctrl before checking the active SVID. This led to
> drm_aux_hpd_bridge_notify() no longer being called for the 'DP unplug'
> case.
>
> Perhaps somewhat interestingly, the firmware sends a notification with
> SVID=DP, mux_ctrl=MUX_CTRL_STATE_NO_CONN and pin_assignment=0 on
> unplug. 'pin_assignment' was previously interpreted as a bitfield
> excerpt from the second byte of the DP pg_altmode payload (and stored
> as an u8).
>
> That value is used in pmic_glink_altmode_sc8280xp_notify(), decremented
> by 1 (DPAM_HPD_A). Previously, this would result in an u8 underflow
> that would rollover to 0xff (which prior to the Fixes patch would have
> caused a pmic_glink_altmode_safe() and 'disconnected' bridge
> notification). That check was removed, without a replacement.
>
> Resolve this issue by making sure the SID=DP && mux_ctrl=NO_CONN combo
> once again results in a HPD bridge notification.
>
> Fixes: 0539c5a6fdef ("soc: qcom: pmic_glink_altmode: Consume TBT3/USB4 mode notifications")
> Reported-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> Tested-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> Signed-off-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> ---
> drivers/soc/qcom/pmic_glink_altmode.c | 18 +++++++++++-------
> 1 file changed, 11 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/soc/qcom/pmic_glink_altmode.c b/drivers/soc/qcom/pmic_glink_altmode.c
> index d0afdcb96ee1..b496b88842a2 100644
> --- a/drivers/soc/qcom/pmic_glink_altmode.c
> +++ b/drivers/soc/qcom/pmic_glink_altmode.c
> @@ -350,15 +350,17 @@ static void pmic_glink_altmode_worker(struct work_struct *work)
>
> typec_switch_set(alt_port->typec_switch, alt_port->orientation);
>
> - if (alt_port->mux_ctrl == MUX_CTRL_STATE_NO_CONN) {
> - pmic_glink_altmode_safe(altmode, alt_port);
> - } else if (alt_port->svid == USB_TYPEC_TBT_SID) {
> + if (alt_port->svid == USB_TYPEC_TBT_SID) {
> pmic_glink_altmode_enable_tbt(altmode, alt_port);
> } else if (alt_port->svid == USB_TYPEC_DP_SID) {
> - pmic_glink_altmode_enable_dp(altmode, alt_port,
> - alt_port->mode,
> - alt_port->hpd_state,
> - alt_port->hpd_irq);
> + if (alt_port->mux_ctrl == MUX_CTRL_STATE_NO_CONN) {
> + pmic_glink_altmode_safe(altmode, alt_port);
> + } else {
> + pmic_glink_altmode_enable_dp(altmode, alt_port,
> + alt_port->mode,
> + alt_port->hpd_state,
> + alt_port->hpd_irq);
> + }
>
> if (alt_port->hpd_state)
> conn_status = connector_status_connected;
> @@ -368,6 +370,8 @@ static void pmic_glink_altmode_worker(struct work_struct *work)
> drm_aux_hpd_bridge_notify(&alt_port->bridge->dev, conn_status);
> } else if (alt_port->mux_ctrl == MUX_CTRL_STATE_TUNNELING) {
> pmic_glink_altmode_enable_usb4(altmode, alt_port);
> + } else if (alt_port->mux_ctrl == MUX_CTRL_STATE_NO_CONN) {
> + pmic_glink_altmode_safe(altmode, alt_port);
> } else {
> pmic_glink_altmode_enable_usb(altmode, alt_port);
> }
This looks fishy, because you had:
if (alt_port->mux_ctrl == MUX_CTRL_STATE_NO_CONN)
pmic_glink_altmode_safe()
else (blah)
...
else (alt_port->svid == USB_TYPEC_DP_SID)
pmic_glink_altmode_enable_dp
else (blah)
...
else (blah)
...
So what's the difference with :
if (blah)
...
else (alt_port->svid == USB_TYPEC_DP_SID) {
if (alt_port->mux_ctrl == MUX_CTRL_STATE_NO_CONN)
pmic_glink_altmode_safe()
else
pmic_glink_altmode_enable_dp
}
else (blah)
...
else (blah)
...
else (alt_port->mux_ctrl == MUX_CTRL_STATE_NO_CONN)
pmic_glink_altmode_safe()
Before, if mux_ctrl was MUX_CTRL_STATE_NO_CONN, it would set to safe mode whatever the svid,
now it's either in the DP case or at the end.
I don't see the difference here, except if your desire is to ignore
the MUX_CTRL_STATE_NO_CONN for USB_TYPEC_TBT_SID and MUX_CTRL_STATE_TUNNELING.
But it doesn't match the commit message at all.
Neil
>
> ---
> base-commit: fc7b1a72c6cd5cbbd989c6c32a6486e3e4e3594d
> change-id: 20260306-topic-pgaltmode_fixup-6b488960e355
>
> Best regards,
On 3/6/26 3:47 PM, Neil Armstrong wrote:
> On 3/6/26 12:20, Konrad Dybcio wrote:[...]
> So what's the difference with :
>
>
> if (blah)
> ...
> else (alt_port->svid == USB_TYPEC_DP_SID) {
> if (alt_port->mux_ctrl == MUX_CTRL_STATE_NO_CONN)
> pmic_glink_altmode_safe()
> else
> pmic_glink_altmode_enable_dp
> }
> else (blah)
> ...
> else (blah)
> ...
> else (alt_port->mux_ctrl == MUX_CTRL_STATE_NO_CONN)
> pmic_glink_altmode_safe()
>
>
> Before, if mux_ctrl was MUX_CTRL_STATE_NO_CONN, it would set to safe mode whatever the svid,
> now it's either in the DP case or at the end.
>
> I don't see the difference here, except if your desire is to ignore
> the MUX_CTRL_STATE_NO_CONN for USB_TYPEC_TBT_SID and MUX_CTRL_STATE_TUNNELING.
>
> But it doesn't match the commit message at all.
The difference is the call to drm_aux_hpd_bridge_notify() in the DP
case, which isn't very visible in the patch diff.
Konrad
On 3/9/26 11:38, Konrad Dybcio wrote:
> On 3/6/26 3:47 PM, Neil Armstrong wrote:
>> On 3/6/26 12:20, Konrad Dybcio wrote:[...]
>
>> So what's the difference with :
>>
>>
>> if (blah)
>> ...
>> else (alt_port->svid == USB_TYPEC_DP_SID) {
>> if (alt_port->mux_ctrl == MUX_CTRL_STATE_NO_CONN)
>> pmic_glink_altmode_safe()
>> else
>> pmic_glink_altmode_enable_dp
>> }
>> else (blah)
>> ...
>> else (blah)
>> ...
>> else (alt_port->mux_ctrl == MUX_CTRL_STATE_NO_CONN)
>> pmic_glink_altmode_safe()
>>
>>
>> Before, if mux_ctrl was MUX_CTRL_STATE_NO_CONN, it would set to safe mode whatever the svid,
>> now it's either in the DP case or at the end.
>>
>> I don't see the difference here, except if your desire is to ignore
>> the MUX_CTRL_STATE_NO_CONN for USB_TYPEC_TBT_SID and MUX_CTRL_STATE_TUNNELING.
>>
>> But it doesn't match the commit message at all.
>
>
> The difference is the call to drm_aux_hpd_bridge_notify() in the DP
> case, which isn't very visible in the patch diff.
Oh I see it now thx, but with this change MUX_CTRL_STATE_NO_CONN will be ignored for USB_TYPEC_TBT_SID and MUX_CTRL_STATE_TUNNELING.
Neil
>
> Konrad
On 3/9/26 11:43 AM, Neil Armstrong wrote:
> On 3/9/26 11:38, Konrad Dybcio wrote:
>> On 3/6/26 3:47 PM, Neil Armstrong wrote:
>>> On 3/6/26 12:20, Konrad Dybcio wrote:[...]
>>
>>> So what's the difference with :
>>>
>>>
>>> if (blah)
>>> ...
>>> else (alt_port->svid == USB_TYPEC_DP_SID) {
>>> if (alt_port->mux_ctrl == MUX_CTRL_STATE_NO_CONN)
>>> pmic_glink_altmode_safe()
>>> else
>>> pmic_glink_altmode_enable_dp
>>> }
>>> else (blah)
>>> ...
>>> else (blah)
>>> ...
>>> else (alt_port->mux_ctrl == MUX_CTRL_STATE_NO_CONN)
>>> pmic_glink_altmode_safe()
>>>
>>>
>>> Before, if mux_ctrl was MUX_CTRL_STATE_NO_CONN, it would set to safe mode whatever the svid,
>>> now it's either in the DP case or at the end.
>>>
>>> I don't see the difference here, except if your desire is to ignore
>>> the MUX_CTRL_STATE_NO_CONN for USB_TYPEC_TBT_SID and MUX_CTRL_STATE_TUNNELING.
>>>
>>> But it doesn't match the commit message at all.
>>
>>
>> The difference is the call to drm_aux_hpd_bridge_notify() in the DP
>> case, which isn't very visible in the patch diff.
>
> Oh I see it now thx, but with this change MUX_CTRL_STATE_NO_CONN will be ignored for USB_TYPEC_TBT_SID and MUX_CTRL_STATE_TUNNELING.
Both are very much intended
For TBT SVID, it can only show up if STATE_TUNNELING
In the other case, STATE_TUNNELING and STATE_NO_CONN appear in the same
field, so only one of them can be present at once
Actually that's why I made the mistake of ignoring the DP case in the first
place - I assumed NO_CONN must not be valid with any active (alt)mode, but
the DP path was wired up in this.. interesting.. way
Konrad
© 2016 - 2026 Red Hat, Inc.