Re: [PATCH v4 01/16] dt-bindings: phy: rockchip-usbdp: add improved ports scheme

Re: [PATCH v4 01/16] dt-bindings: phy: rockchip-usbdp: add improved ports scheme

[Rob Herring]

On Tue, Apr 28, 2026 at 06:13:40PM +0200, Sebastian Reichel wrote:
> Currently the Rockchip USBDP PHY is missing a documented port scheme.
> Meanwhile upstream RK3588 DTS files are a bit messy and use different
> port schemes. The upstream USBDP PHY Linux kernel driver does not yet
> parse the ports at all and thus does not create any implicit ABI either.
> 
> But with the current mess it is not possible to properly support USB-C
> DP AltMode. Thus this introduces a proper port scheme following roughly
> the ports design of the Qualcomm QMP USB4-USB3-DP PHY controller binding
> with a slight difference that there is an additional port for the
> USB-C SBU port as the Rockchip USB-DP PHY also contains the SBU mux.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>

Missing my tag.

> ---
>  .../bindings/phy/phy-rockchip-usbdp.yaml           | 23 ++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/phy/phy-rockchip-usbdp.yaml b/Documentation/devicetree/bindings/phy/phy-rockchip-usbdp.yaml
> index 8b7059d5b182..f728acf057e4 100644
> --- a/Documentation/devicetree/bindings/phy/phy-rockchip-usbdp.yaml
> +++ b/Documentation/devicetree/bindings/phy/phy-rockchip-usbdp.yaml
> @@ -114,6 +114,29 @@ properties:
>        A port node to link the PHY to a TypeC controller for the purpose of
>        handling orientation switching.
>  
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          Output endpoint of the PHY for USB (or DP when configured into 4 lane
> +          mode), which should point to the superspeed port of a USB connector.
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: Incoming endpoint from the USB controller
> +
> +      port@2:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: Incoming endpoint from the DisplayPort controller
> +
> +      port@3:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description:
> +          Output endpoint of the PHY for DP, which should either point to the
> +          SBU port of a USB-C connector or a DisplayPort connector input port.
> +
>  required:
>    - compatible
>    - reg
> 
> -- 
> 2.53.0
> 

Re: [PATCH v4 01/16] dt-bindings: phy: rockchip-usbdp: add improved ports scheme

[Rob Herring (Arm)]

On Tue, 28 Apr 2026 18:13:40 +0200, Sebastian Reichel wrote:
> Currently the Rockchip USBDP PHY is missing a documented port scheme.
> Meanwhile upstream RK3588 DTS files are a bit messy and use different
> port schemes. The upstream USBDP PHY Linux kernel driver does not yet
> parse the ports at all and thus does not create any implicit ABI either.
> 
> But with the current mess it is not possible to properly support USB-C
> DP AltMode. Thus this introduces a proper port scheme following roughly
> the ports design of the Qualcomm QMP USB4-USB3-DP PHY controller binding
> with a slight difference that there is an additional port for the
> USB-C SBU port as the Rockchip USB-DP PHY also contains the SBU mux.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>  .../bindings/phy/phy-rockchip-usbdp.yaml           | 23 ++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 

Reviewed-by: Rob Herring (Arm) <robh@kernel.org>

Re: [PATCH v4 02/16] phy: rockchip: usbdp: Do not loose USB3 PHY status

[Neil Armstrong]

On 4/28/26 18:13, Sebastian Reichel wrote:
> By default (i.e. without manually enabling runtime PM) DWC3 requests the
> USB3 PHY once and keeps it enabled all the time. When DisplayPort is
> being requested later on, a mode change is needed. This re-initializes
> the PHY. During re-initialization the status variable has incorrectly
> been cleared, which means the tracking information for USB3 ist lost.

--------------------------------------------------------------/\ is

> 
> This is not an immediate problem, since the DP side keeps the PHY
> enabled. But once DP is toggled off, the whole PHY will be disabled.
> This is a problem, because the USB side still needs it powered.
> 
> Fix things by not clearing the status flags.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/phy/rockchip/phy-rockchip-usbdp.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index fba35510d88c..744cc7c642f4 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1009,7 +1009,6 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
>   			rk_udphy_u3_port_disable(udphy, false);
>   	} else if (udphy->mode_change) {
>   		udphy->mode_change = false;
> -		udphy->status = UDPHY_MODE_NONE;
>   		if (udphy->mode == UDPHY_MODE_DP)
>   			rk_udphy_u3_port_disable(udphy, true);
>   
> 

Looks good, but any fixes tag ?

Neil

Re: [PATCH v4 03/16] phy: rockchip: usbdp: Keep clocks running on PHY re-init

[Neil Armstrong]

On 4/28/26 18:13, Sebastian Reichel wrote:
> When a mode change is required rk_udphy_power_on() disables
> the clocks and then calls rk_udphy_setup(), which then enables
> all the clocks again before continuing with rk_udphy_init().
> 
> Considering that rk_udphy_init() does assert the reset lines,
> re-enabling the clocks is just delaying things. Avoid it by
> directly calling rk_udphy_init().
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/phy/rockchip/phy-rockchip-usbdp.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 744cc7c642f4..98562a888b42 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -1012,8 +1012,7 @@ static int rk_udphy_power_on(struct rk_udphy *udphy, u8 mode)
>   		if (udphy->mode == UDPHY_MODE_DP)
>   			rk_udphy_u3_port_disable(udphy, true);
>   
> -		rk_udphy_disable(udphy);
> -		ret = rk_udphy_setup(udphy);
> +		ret = rk_udphy_init(udphy);
>   		if (ret)
>   			return ret;
>   	}
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

Re: [PATCH v4 04/16] phy: rockchip: usbdp: Amend SSC modulation deviation

[Neil Armstrong]

On 4/28/26 18:13, Sebastian Reichel wrote:
> From: Frank Wang <frank.wang@rock-chips.com>
> 
> Move SSC modulation deviation into private config of clock
> 
>   - 24M: 0x00d4[5:0] = 0x30
>   - 26M: 0x00d4[5:0] = 0x33
> 
> Signed-off-by: Frank Wang <frank.wang@rock-chips.com>
> [Taken over from rockchip's kernel tree; register 0x00d4 is not
> described in the TRM]
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/phy/rockchip/phy-rockchip-usbdp.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 98562a888b42..1f686844c337 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -350,7 +350,8 @@ static const struct reg_sequence rk_udphy_24m_refclk_cfg[] = {
>   	{0x0a64, 0xa8}, {0x1a3c, 0xd0},
>   	{0x1a44, 0xd0}, {0x1a48, 0x01},
>   	{0x1a4c, 0x0d}, {0x1a54, 0xe0},
> -	{0x1a5c, 0xe0}, {0x1a64, 0xa8}
> +	{0x1a5c, 0xe0}, {0x1a64, 0xa8},
> +	{0x00d4, 0x30}
>   };
>   
>   static const struct reg_sequence rk_udphy_26m_refclk_cfg[] = {
> @@ -377,7 +378,7 @@ static const struct reg_sequence rk_udphy_26m_refclk_cfg[] = {
>   	{0x0c30, 0x0e}, {0x0c48, 0x06},
>   	{0x1c30, 0x0e}, {0x1c48, 0x06},
>   	{0x028c, 0x18}, {0x0af0, 0x00},
> -	{0x1af0, 0x00}
> +	{0x1af0, 0x00}, {0x00d4, 0x33}
>   };
>   
>   static const struct reg_sequence rk_udphy_init_sequence[] = {
> @@ -412,8 +413,7 @@ static const struct reg_sequence rk_udphy_init_sequence[] = {
>   	{0x0070, 0x7d}, {0x0074, 0x68},
>   	{0x0af4, 0x1a}, {0x1af4, 0x1a},
>   	{0x0440, 0x3f}, {0x10d4, 0x08},
> -	{0x20d4, 0x08}, {0x00d4, 0x30},
> -	{0x0024, 0x6e},
> +	{0x20d4, 0x08}, {0x0024, 0x6e}
>   };
>   
>   static inline int rk_udphy_grfreg_write(struct regmap *base,
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

Re: [PATCH v4 05/16] phy: rockchip: usbdp: Fix LFPS detect threshold control

[Neil Armstrong]

On 4/28/26 18:13, Sebastian Reichel wrote:
> From: William Wu <william.wu@rock-chips.com>
> 
> According to the LFPS Tx Low Power/LFPS Rx Detect Threshold [1],
> the device under test(DUT) must not respond if LFPS below the
> minimum LFPS Rx Detect Threshold 100mV. Test fail on Rockchip
> platforms, because the default LFPS detect threshold is set to
> 65mV.
> 
> The USBDP PHY LFPS detect threshold voltage could be set to
> 30mV ~ 140mV, and since there could be 10-20% PVT variation,
> we set LFPS detect threshold voltage to 110mV.
> 
> [1] https://compliance.usb.org/resources/LFPS_Rx_Tx_Low_Power_Compliance_Update_Rev5.pdf
> 
> Signed-off-by: William Wu <william.wu@rock-chips.com>
> [Taken over from rockchip's kernel tree; the registers are not described
> in the TRM]
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/phy/rockchip/phy-rockchip-usbdp.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 1f686844c337..97e53b933225 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -413,7 +413,8 @@ static const struct reg_sequence rk_udphy_init_sequence[] = {
>   	{0x0070, 0x7d}, {0x0074, 0x68},
>   	{0x0af4, 0x1a}, {0x1af4, 0x1a},
>   	{0x0440, 0x3f}, {0x10d4, 0x08},
> -	{0x20d4, 0x08}, {0x0024, 0x6e}
> +	{0x20d4, 0x08}, {0x0024, 0x6e},
> +	{0x09c0, 0x0a}, {0x19c0, 0x0a}
>   };
>   
>   static inline int rk_udphy_grfreg_write(struct regmap *base,
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

Re: [PATCH v4 06/16] phy: rockchip: usbdp: Add missing mode_change update

[Neil Armstrong]

On 4/28/26 18:13, Sebastian Reichel wrote:
> rk_udphy_set_typec_default_mapping() updates the available modes,
> but does not set the mode_change as required. This results in
> missing re-initialization and thus non-working DisplayPort.
> 
> Fix this issue by introducing a new helper to update the available
> modes.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/phy/rockchip/phy-rockchip-usbdp.c | 16 +++++++++++-----
>   1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 97e53b933225..febc148a754e 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -619,6 +619,15 @@ static void rk_udphy_dp_hpd_event_trigger(struct rk_udphy *udphy, bool hpd)
>   	rk_udphy_grfreg_write(udphy->vogrf, &cfg->vogrfcfg[udphy->id].hpd_trigger, hpd);
>   }
>   
> +static void rk_udphy_mode_set(struct rk_udphy *udphy, u8 mode)
> +{
> +	if (udphy->mode == mode)
> +		return;
> +
> +	udphy->mode_change = true;
> +	udphy->mode = mode;
> +}
> +
>   static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
>   {
>   	if (udphy->flip) {
> @@ -649,7 +658,7 @@ static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
>   		gpiod_set_value_cansleep(udphy->sbu2_dc_gpio, 1);
>   	}
>   
> -	udphy->mode = UDPHY_MODE_DP_USB;
> +	rk_udphy_mode_set(udphy, UDPHY_MODE_DP_USB);
>   }
>   
>   static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
> @@ -1385,10 +1394,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);
>   			rk_udphy_dp_hpd_event_trigger(udphy, true);
>   		} else {
>   			rk_udphy_dp_hpd_event_trigger(udphy, false);
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

Re: [PATCH v4 07/16] phy: rockchip: usbdp: Support single-lane DP

[Neil Armstrong]

On 4/28/26 18:13, Sebastian Reichel wrote:
> From: Zhang Yubing <yubing.zhang@rock-chips.com>
> 
> Implement support for using just a single DisplayPort line.
> 
> Signed-off-by: Zhang Yubing <yubing.zhang@rock-chips.com>
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/phy/rockchip/phy-rockchip-usbdp.c | 61 +++++++++++++------------------
>   1 file changed, 25 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index febc148a754e..bf8394174294 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -193,6 +193,7 @@ struct rk_udphy {
>   	int id;
>   
>   	bool dp_in_use;
> +	int dp_lanes;
>   
>   	/* PHY const config */
>   	const struct rk_udphy_cfg *cfgs;
> @@ -537,6 +538,13 @@ static void rk_udphy_usb_bvalid_enable(struct rk_udphy *udphy, u8 enable)
>    * <0 1>                  dpln0         dpln1       usbrx         usbtx
>    * <2 3>                  usbrx         usbtx       dpln0         dpln1
>    * ---------------------------------------------------------------------------
> + * if 1 lane for dp function, 2 lane for usb function, define rockchip,dp-lane-mux = <x>;
> + * sample as follow:
> + * ---------------------------------------------------------------------------
> + *                        B11-B10       A2-A3       A11-A10       B2-B3
> + * rockchip,dp-lane-mux   ln0(tx/rx)    ln1(tx)     ln2(tx/rx)    ln3(tx)
> + * <0>                    dpln0         \           usbrx         usbtx
> + * ---------------------------------------------------------------------------
>    */
>   
>   static void rk_udphy_dplane_select(struct rk_udphy *udphy)
> @@ -544,18 +552,18 @@ static void rk_udphy_dplane_select(struct rk_udphy *udphy)
>   	const struct rk_udphy_cfg *cfg = udphy->cfgs;
>   	u32 value = 0;
>   
> -	switch (udphy->mode) {
> -	case UDPHY_MODE_DP:
> -		value |= 2 << udphy->dp_lane_sel[2] * 2;
> +	switch (udphy->dp_lanes) {
> +	case 4:
>   		value |= 3 << udphy->dp_lane_sel[3] * 2;
> +		value |= 2 << udphy->dp_lane_sel[2] * 2;
>   		fallthrough;
>   
> -	case UDPHY_MODE_DP_USB:
> -		value |= 0 << udphy->dp_lane_sel[0] * 2;
> +	case 2:
>   		value |= 1 << udphy->dp_lane_sel[1] * 2;
> -		break;
> +		fallthrough;
>   
> -	case UDPHY_MODE_USB:
> +	case 1:
> +		value |= 0 << udphy->dp_lane_sel[0] * 2;

What's the point of keeping this no-op calculation ?

>   		break;
>   
>   	default:
> @@ -568,28 +576,6 @@ static void rk_udphy_dplane_select(struct rk_udphy *udphy)
>   		     FIELD_PREP(DP_AUX_DOUT_SEL, udphy->dp_aux_dout_sel) | value);
>   }
>   
> -static int rk_udphy_dplane_get(struct rk_udphy *udphy)
> -{
> -	int dp_lanes;
> -
> -	switch (udphy->mode) {
> -	case UDPHY_MODE_DP:
> -		dp_lanes = 4;
> -		break;
> -
> -	case UDPHY_MODE_DP_USB:
> -		dp_lanes = 2;
> -		break;
> -
> -	case UDPHY_MODE_USB:
> -	default:
> -		dp_lanes = 0;
> -		break;
> -	}
> -
> -	return dp_lanes;
> -}
> -
>   static void rk_udphy_dplane_enable(struct rk_udphy *udphy, int dp_lanes)
>   {
>   	u32 val = 0;
> @@ -659,6 +645,7 @@ static void rk_udphy_set_typec_default_mapping(struct rk_udphy *udphy)
>   	}
>   
>   	rk_udphy_mode_set(udphy, UDPHY_MODE_DP_USB);
> +	udphy->dp_lanes = 2;
>   }
>   
>   static int rk_udphy_orien_sw_set(struct typec_switch_dev *sw,
> @@ -897,7 +884,7 @@ static int rk_udphy_parse_lane_mux_data(struct rk_udphy *udphy)
>   		return 0;
>   	}
>   
> -	if (num_lanes != 2 && num_lanes != 4)
> +	if (num_lanes != 1 && num_lanes != 2 && num_lanes != 4)
>   		return dev_err_probe(udphy->dev, -EINVAL,
>   				     "invalid number of lane mux\n");
>   
> @@ -923,7 +910,8 @@ static int rk_udphy_parse_lane_mux_data(struct rk_udphy *udphy)
>   	}
>   
>   	udphy->mode = UDPHY_MODE_DP;
> -	if (num_lanes == 2) {
> +	udphy->dp_lanes = num_lanes;
> +	if (num_lanes == 1 || num_lanes == 2) {
>   		udphy->mode |= UDPHY_MODE_USB;
>   		udphy->flip = (udphy->lane_mux_sel[0] == PHY_LANE_MUX_DP);
>   	}
> @@ -1074,18 +1062,17 @@ static int rk_udphy_dp_phy_exit(struct phy *phy)
>   static int rk_udphy_dp_phy_power_on(struct phy *phy)
>   {
>   	struct rk_udphy *udphy = phy_get_drvdata(phy);
> -	int ret, dp_lanes;
> +	int ret;
>   
>   	mutex_lock(&udphy->mutex);
>   
> -	dp_lanes = rk_udphy_dplane_get(udphy);
> -	phy_set_bus_width(phy, dp_lanes);
> +	phy_set_bus_width(phy, udphy->dp_lanes);
>   
>   	ret = rk_udphy_power_on(udphy, UDPHY_MODE_DP);
>   	if (ret)
>   		goto unlock;
>   
> -	rk_udphy_dplane_enable(udphy, dp_lanes);
> +	rk_udphy_dplane_enable(udphy, udphy->dp_lanes);
>   
>   	rk_udphy_dplane_select(udphy);
>   
> @@ -1365,6 +1352,7 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>   		udphy->lane_mux_sel[2] = PHY_LANE_MUX_DP;
>   		udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
>   		mode = UDPHY_MODE_DP;
> +		udphy->dp_lanes = 4;
>   		break;
>   
>   	case TYPEC_DP_STATE_D:
> @@ -1381,6 +1369,7 @@ static int rk_udphy_typec_mux_set(struct typec_mux_dev *mux,
>   			udphy->lane_mux_sel[3] = PHY_LANE_MUX_DP;
>   		}
>   		mode = UDPHY_MODE_DP_USB;
> +		udphy->dp_lanes = 2;
>   		break;
>   	}
>   
> @@ -1529,7 +1518,7 @@ static int rk_udphy_probe(struct platform_device *pdev)
>   		ret = PTR_ERR(udphy->phy_dp);
>   		return dev_err_probe(dev, ret, "failed to create DP phy\n");
>   	}
> -	phy_set_bus_width(udphy->phy_dp, rk_udphy_dplane_get(udphy));
> +	phy_set_bus_width(udphy->phy_dp, udphy->dp_lanes);
>   	udphy->phy_dp->attrs.max_link_rate = 8100;
>   	phy_set_drvdata(udphy->phy_dp, udphy);
>   
> 

Re: [PATCH v4 08/16] phy: rockchip: usbdp: Rename DP lane functions

[Neil Armstrong]

On 4/28/26 18:13, Sebastian Reichel wrote:
> The common prefix for DisplayPort related functions is rk_udphy_dp_
> (with a final _), so update the two DP lane functions to follow that
> scheme.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/phy/rockchip/phy-rockchip-usbdp.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index bf8394174294..6d7ca11b308e 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -547,7 +547,7 @@ static void rk_udphy_usb_bvalid_enable(struct rk_udphy *udphy, u8 enable)
>    * ---------------------------------------------------------------------------
>    */
>   
> -static void rk_udphy_dplane_select(struct rk_udphy *udphy)
> +static void rk_udphy_dp_lane_select(struct rk_udphy *udphy)
>   {
>   	const struct rk_udphy_cfg *cfg = udphy->cfgs;
>   	u32 value = 0;
> @@ -576,7 +576,7 @@ static void rk_udphy_dplane_select(struct rk_udphy *udphy)
>   		     FIELD_PREP(DP_AUX_DOUT_SEL, udphy->dp_aux_dout_sel) | value);
>   }
>   
> -static void rk_udphy_dplane_enable(struct rk_udphy *udphy, int dp_lanes)
> +static void rk_udphy_dp_lane_enable(struct rk_udphy *udphy, int dp_lanes)
>   {
>   	u32 val = 0;
>   	int i;
> @@ -1072,9 +1072,9 @@ static int rk_udphy_dp_phy_power_on(struct phy *phy)
>   	if (ret)
>   		goto unlock;
>   
> -	rk_udphy_dplane_enable(udphy, udphy->dp_lanes);
> +	rk_udphy_dp_lane_enable(udphy, udphy->dp_lanes);
>   
> -	rk_udphy_dplane_select(udphy);
> +	rk_udphy_dp_lane_select(udphy);
>   
>   unlock:
>   	mutex_unlock(&udphy->mutex);
> @@ -1092,7 +1092,7 @@ static int rk_udphy_dp_phy_power_off(struct phy *phy)
>   	struct rk_udphy *udphy = phy_get_drvdata(phy);
>   
>   	mutex_lock(&udphy->mutex);
> -	rk_udphy_dplane_enable(udphy, 0);
> +	rk_udphy_dp_lane_enable(udphy, 0);
>   	rk_udphy_power_off(udphy, UDPHY_MODE_DP);
>   	mutex_unlock(&udphy->mutex);
>   
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

Re: [PATCH v4 09/16] phy: rockchip: usbdp: Use FIELD_PREP_WM16_CONST

[Neil Armstrong]

On 4/28/26 18:13, Sebastian Reichel wrote:
> Cleanup code by replacing open-coded version of FIELD_PREP_WM16_CONST
> with the existing helper macro.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/phy/rockchip/phy-rockchip-usbdp.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 6d7ca11b308e..1bfc365e2b2c 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -12,6 +12,7 @@
>   #include <linux/clk.h>
>   #include <linux/delay.h>
>   #include <linux/gpio.h>
> +#include <linux/hw_bitfield.h>
>   #include <linux/mfd/syscon.h>
>   #include <linux/mod_devicetable.h>
>   #include <linux/module.h>
> @@ -75,7 +76,6 @@
>   #define TRSV_LN2_MON_RX_CDR_DONE_OFFSET		0x1b84	/* trsv_reg06E1 */
>   #define TRSV_LN2_MON_RX_CDR_LOCK_DONE		BIT(0)
>   
> -#define BIT_WRITEABLE_SHIFT			16
>   #define PHY_AUX_DP_DATA_POL_NORMAL		0
>   #define PHY_AUX_DP_DATA_POL_INVERT		1
>   #define PHY_LANE_MUX_USB			0
> @@ -104,8 +104,8 @@ struct rk_udphy_grf_reg {
>   #define _RK_UDPHY_GEN_GRF_REG(offset, mask, disable, enable) \
>   {\
>   	offset, \
> -	FIELD_PREP_CONST(mask, disable) | (mask << BIT_WRITEABLE_SHIFT), \
> -	FIELD_PREP_CONST(mask, enable) | (mask << BIT_WRITEABLE_SHIFT), \
> +	FIELD_PREP_WM16_CONST(mask, disable), \
> +	FIELD_PREP_WM16_CONST(mask, enable), \
>   }
>   
>   #define RK_UDPHY_GEN_GRF_REG(offset, bitend, bitstart, disable, enable) \
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

Re: [PATCH v4 10/16] phy: rockchip: usbdp: Cleanup DP lane selection function

[Neil Armstrong]

On 4/28/26 18:13, Sebastian Reichel wrote:
> Use FIELD_PREP_WM16() helpers to simplify the DP lane selection
> logic.
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/phy/rockchip/phy-rockchip-usbdp.c | 28 +++++++---------------------
>   1 file changed, 7 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index 1bfc365e2b2c..beab20e4c512 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -550,30 +550,16 @@ static void rk_udphy_usb_bvalid_enable(struct rk_udphy *udphy, u8 enable)
>   static void rk_udphy_dp_lane_select(struct rk_udphy *udphy)
>   {
>   	const struct rk_udphy_cfg *cfg = udphy->cfgs;
> -	u32 value = 0;
> -
> -	switch (udphy->dp_lanes) {
> -	case 4:
> -		value |= 3 << udphy->dp_lane_sel[3] * 2;
> -		value |= 2 << udphy->dp_lane_sel[2] * 2;
> -		fallthrough;
> -
> -	case 2:
> -		value |= 1 << udphy->dp_lane_sel[1] * 2;
> -		fallthrough;
> +	u32 value = FIELD_PREP_WM16(DP_LANE_SEL_ALL, 0);
> +	int i;
>   
> -	case 1:
> -		value |= 0 << udphy->dp_lane_sel[0] * 2;
> -		break;
> +	for (i = 0; i < udphy->dp_lanes; i++)
> +		value |= field_prep(DP_LANE_SEL_N(udphy->dp_lane_sel[i]), i);
>   
> -	default:
> -		break;
> -	}
> +	value |= FIELD_PREP_WM16(DP_AUX_DIN_SEL, udphy->dp_aux_din_sel);
> +	value |= FIELD_PREP_WM16(DP_AUX_DOUT_SEL, udphy->dp_aux_dout_sel);
>   
> -	regmap_write(udphy->vogrf, cfg->vogrfcfg[udphy->id].dp_lane_reg,
> -		     ((DP_AUX_DIN_SEL | DP_AUX_DOUT_SEL | DP_LANE_SEL_ALL) << 16) |
> -		     FIELD_PREP(DP_AUX_DIN_SEL, udphy->dp_aux_din_sel) |
> -		     FIELD_PREP(DP_AUX_DOUT_SEL, udphy->dp_aux_dout_sel) | value);
> +	regmap_write(udphy->vogrf, cfg->vogrfcfg[udphy->id].dp_lane_reg, value);
>   }
>   
>   static void rk_udphy_dp_lane_enable(struct rk_udphy *udphy, int dp_lanes)
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

Re: [PATCH v4 11/16] phy: rockchip: usbdp: Register DP aux bridge

[Neil Armstrong]

On 4/28/26 18:13, Sebastian Reichel wrote:
> Add support to use USB-C connectors with the DP altmode helper code on
> devicetree based platforms. To get this working there must be a DRM
> bridge chain from the DisplayPort controller to the USB-C connector.
> E.g. on Rockchip RK3576:
> 
> root@rk3576 # cat /sys/kernel/debug/dri/0/encoder-0/bridges
> bridge[0]: dw_dp_bridge_funcs
>          refcount: 7
>          type: [10] DP
>          OF: /soc/dp@27e40000:rockchip,rk3576-dp
>          ops: [0x47] detect edid hpd
> bridge[1]: drm_aux_bridge_funcs
>          refcount: 4
>          type: [0] Unknown
>          OF: /soc/phy@2b010000:rockchip,rk3576-usbdp-phy
>          ops: [0x0]
> bridge[2]: drm_aux_hpd_bridge_funcs
>          refcount: 5
>          type: [10] DP
>          OF: /soc/i2c@2ac50000/typec-portc@22/connector:usb-c-connector
>          ops: [0x4] hpd
> 
> Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> ---
>   drivers/phy/rockchip/Kconfig              |  2 ++
>   drivers/phy/rockchip/phy-rockchip-usbdp.c | 14 ++++++++++++++
>   2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/phy/rockchip/Kconfig b/drivers/phy/rockchip/Kconfig
> index 14698571b607..39759bb2fa1d 100644
> --- a/drivers/phy/rockchip/Kconfig
> +++ b/drivers/phy/rockchip/Kconfig
> @@ -136,8 +136,10 @@ config PHY_ROCKCHIP_USBDP
>   	tristate "Rockchip USBDP COMBO PHY Driver"
>   	depends on ARCH_ROCKCHIP && OF
>   	depends on TYPEC
> +	depends on DRM || DRM=n
>   	select GENERIC_PHY
>   	select USB_COMMON
> +	select DRM_AUX_BRIDGE if DRM_BRIDGE
>   	help
>   	  Enable this to support the Rockchip USB3.0/DP combo PHY with
>   	  Samsung IP block. This is required for USB3 support on RK3588.
> diff --git a/drivers/phy/rockchip/phy-rockchip-usbdp.c b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> index beab20e4c512..77ad2a89d4f2 100644
> --- a/drivers/phy/rockchip/phy-rockchip-usbdp.c
> +++ b/drivers/phy/rockchip/phy-rockchip-usbdp.c
> @@ -6,6 +6,7 @@
>    * Copyright (C) 2024 Collabora Ltd
>    */
>   
> +#include <drm/bridge/aux-bridge.h>
>   #include <dt-bindings/phy/phy.h>
>   #include <linux/bitfield.h>
>   #include <linux/bits.h>
> @@ -1434,6 +1435,7 @@ static int rk_udphy_probe(struct platform_device *pdev)
>   {
>   	struct device *dev = &pdev->dev;
>   	struct phy_provider *phy_provider;
> +	struct fwnode_handle *dp_aux_ep;
>   	struct resource *res;
>   	struct rk_udphy *udphy;
>   	void __iomem *base;
> @@ -1492,6 +1494,18 @@ static int rk_udphy_probe(struct platform_device *pdev)
>   			return ret;
>   	}
>   
> +	/*
> +	 * Only register the DRM bridge, if the DP aux channel is connected.
> +	 * Some boards use the USBDP PHY only for its USB3 capabilities.
> +	 */
> +	dp_aux_ep = fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 3, 0, 0);
> +	if (dp_aux_ep) {
> +		ret = drm_aux_bridge_register(dev);
> +		fwnode_handle_put(dp_aux_ep);
> +		if (ret)
> +			return ret;
> +	}
> +
>   	udphy->phy_u3 = devm_phy_create(dev, dev->of_node, &rk_udphy_usb3_phy_ops);
>   	if (IS_ERR(udphy->phy_u3)) {
>   		ret = PTR_ERR(udphy->phy_u3);
> 

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

Re: [PATCH v4 00/16] phy: rockchip: usbdp: Fixes, DP 1-lane support and cleanups

[Vinod Koul]

Hi Heiko,

On 03-05-26, 21:12, Heiko Stuebner wrote:
> Hi Vinod,
> 
> Am Sonntag, 3. Mai 2026, 18:50:12 Mitteleuropäische Sommerzeit schrieb Vinod Koul:
> > Hi Sebastian,
> > 
> > On 28-04-26, 18:13, Sebastian Reichel wrote:
> > > This series overhauls the Rockchip USBDP driver; apart from a
> > > a bunch of cleanups and small improvements the main goal is to
> > > get the driver ready for proper USB-C DP AltMode support.
> > > 
> > > Once this series has landed, it unblocks enabling proper USB-C
> > > DP AltMode on the RK3588 and RK3576 platforms incl. runtime PM
> > > for the Synopsys DesignWare DisplayPort controller.
> > > 
> > > Apart from this series, further changes are required on the
> > > DRM side. There are no compile-time dependencies between the
> > > DRM side and the PHY side, but the PHY side must be applied
> > > to avoid SErrors once runtime PM is added to the DisplayPort
> > > controller driver. Thus it would be really good to land this
> > > series in the next merge window.
> > 
> > Looks like sasiko has flagged 8 high warning, can you please check them
> 
> (1) as mentioned in a different patch thread, review should be happening
> on the lists. Tools regularly dump old results, so anyone looking at the
> mailinglist thread might now be able to follow along with what happened
> in 2 months or so.
> 
> Also other robot tools can reply on the lists just fine.

It was not available in past, have enabled email reply as that is best
way for us. It should happen going forward

> (2) if 1. is so hugely impossible, please at least provide some reference
> to what you mean. Not everybody knows all the hype-tools-of-the-week and
> simply searching for that mysterious "sasiko" [0] did not provide any
> meaningful results for me, despite some amazon or youtube links for some
> non-kernel uses of that term ;-)

I am surprised that the hype of the week seems to missing for you or
Sebastian as Sashiko has been, IMO covered quite well. Instead of a random
web search I would have looked at lwn [1], [2].

> So a link should be present at least please.

Agree, I have sent to people who I thought might not be aware. I guess
better to not assume. In any case here are the comments, which maybe
valid or maybe not. As usual with tools we should use our discretion
https://sashiko.dev/#/patchset/20260428-rockchip-usbdp-cleanup-v4-0-7775671ece22%40collabora.com

[1]: https://lwn.net/Articles/1063292/
[2]: https://lwn.net/Articles/1063303/

-- 
~Vinod