Re: [PATCH v7 12/27] phy: rockchip: usbdp: Add missing mode_change update

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] When dynamically exiting DisplayPort Alternate Mode, the PHY mode is not updated to reflect the new state, which can silently prevent USB SuperSpeed from powering on. A subsequent patch in the series attempts to fix this but fails due to a logical bug.
--

commit a13f2ad68259618e56bf0f7f6e7be4e8d3391f8e
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

phy: rockchip: usbdp: Add missing mode_change update

This commit introduces a new helper rk_udphy_mode_set() to update the
available modes. It ensures the mode_change flag is set when required
so that DisplayPort properly re-initializes.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 1067ddf2d7422..f079715f7da21 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c

[ ... ]

> @@ -1394,10 +1403,7 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>  			usleep_range(750, 800);
>  			rk_udphy_dp_hpd_event_trigger(udphy, true);
>  		} else if (data->status & DP_STATUS_HPD_STATE) {
> -			if (udphy->mode != mode) {
> -				udphy->mode = mode;
> -				udphy->mode_change = true;
> -			}
> +			rk_udphy_mode_set(udphy, mode);

[Severity: High]
This isn't a bug introduced by this patch, but there is a pre-existing issue
in rk_udphy_typec_mux_set() regarding how state transitions are handled.

When dynamically exiting DisplayPort Alternate Mode, does the driver
silently prevent USB SuperSpeed from powering on?

If a user unplugs a DP accessory, the Type-C subsystem sends a mux state
update with state->mode set to TYPEC_STATE_SAFE or TYPEC_STATE_USB, and
state->alt set to NULL.

The code evaluates this condition:

    if (state->mode >= TYPEC_STATE_MODAL || !state->alt ||
        state->alt->svid != USB_TYPEC_DP_SID)
        return 0;

For TYPEC_STATE_SAFE and TYPEC_STATE_USB, state->alt is NULL. This makes
!state->alt evaluate to true. Because of the || operator, the entire
statement evaluates to true, causing the function to return 0 and
ignore the event entirely instead of repowering the PHY correctly.

A later patch in the series ("phy: rockchip: usbdp: Properly handle
TYPEC_STATE_SAFE and TYPEC_STATE_USB") attempts to fix this, but the logic
in that patch still contains this error because it fails to restrict the
state->alt checks to only the modal states.

Could the condition be grouped to properly handle the non-modal states?

>  			rk_udphy_dp_hpd_event_trigger(udphy, true);
>  		} else {
>  			rk_udphy_dp_hpd_event_trigger(udphy, false);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625-rockchip-usbdp-cleanup-v7-0-38eb3cf654fd@collabora.com?part=12