Re: [PATCH v13 32/35] phy: rockchip: usbdp: Rename mode to hw_mode - sashiko-bot

From: sashiko-bot@kernel.org
To: "Sebastian Reichel" <sebastian.reichel@collabora.com>
Cc: linux-phy@lists.infradead.org, neil.armstrong@linaro.org,
	olteanv@gmail.com, devicetree@vger.kernel.org,
	conor+dt@kernel.org, robh@kernel.org, vkoul@kernel.org
Subject: Re: [PATCH v13 32/35] phy: rockchip: usbdp: Rename mode to hw_mode
Date: Tue, 14 Jul 2026 20:18:30 +0000	[thread overview]
Message-ID: <20260714201830.D7BD11F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260714-rockchip-usbdp-cleanup-v13-32-6cb3e769d4c5@collabora.com>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing explicit switch cases for TYPEC_STATE_SAFE and TYPEC_STATE_USB in rk_udphy_set_typec_state().
--

--- Patch [32]: [PATCH v13 32/35] phy: rockchip: usbdp: Rename mode to hw_mode ---
commit 10965afd6a33e65f3dd20fa9afe790f44181de1b
Author: Sebastian Reichel <sebastian.reichel@collabora.com>

phy: rockchip: usbdp: Rename mode to hw_mode

This commit renames the mode field to hw_mode in the rk_udphy struct. This
change clarifies that the field represents the modes currently supported by the
hardware, which are set by the USB-C state machine or device-tree routing,
rather than software-requested modes.

> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index a6fe634574bbc..405b3d9436023 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c

[ ... ]

> @@ -616,12 +616,12 @@ static void rk_udphy_set_typec_state(struct rk_udphy *udphy, unsigned long state
>  			udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
>  			udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
>  		}
> -		mode = UDPHY_MODE_DP_USB;
> +		hw_mode = UDPHY_MODE_DP_USB;

[Severity: High]
Is it possible the default branch in this switch statement improperly catches
TYPEC_STATE_SAFE and TYPEC_STATE_USB?

If the Type-C framework sets the state to TYPEC_STATE_SAFE (0) or
TYPEC_STATE_USB (1), rk_udphy_is_supported_mode() (as introduced in commit
"phy: rockchip: usbdp: Properly handle TYPEC_STATE_SAFE and TYPEC_STATE_USB")
explicitly allows these modes and passes them to rk_udphy_set_typec_state().

However, the switch statement appears to lack explicit cases for them:

	case TYPEC_DP_STATE_D:
	default:
		if (udphy->flip) {
		...

These states would fall into the default branch here and incorrectly configure
hw_mode = UDPHY_MODE_DP_USB and udphy->dp_lanes = 2, instead of pure USB mode.

Could this regression leave the DisplayPort hardware blocks powered on and
incorrectly muxed when DP AltMode is exited or the cable is disconnected?

>  		udphy->dp_lanes = 2;
>  		break;
>  	}
>  
> -	rk_udphy_mode_set(udphy, mode);
> +	rk_udphy_mode_set(udphy, hw_mode);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714-rockchip-usbdp-cleanup-v13-0-6cb3e769d4c5@collabora.com?part=32

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy