Re: [PATCH v10 13/21] drm/rockchip: dw_dp: Implement out-of-band HPD handling

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The implementation of `dw_dp_rockchip_get_vo_grf()` contradicts its own documentation, lacking the intended fallback mechanism for future platforms.
--

--- Patch [13]: [PATCH v10 13/21] drm/rockchip: dw_dp: Implement out-of-band HPD handling ---
commit 4123f78de42a676e261f49ecbc6c7175eb33efa4
Author: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/[email protected]>

drm/rockchip: dw_dp: Implement out-of-band HPD handling

This commit implements out-of-band hotplug handling for the rockchip dw_dp
driver to receive external hotplug information from the USB-C state machine
via VO GRF, avoiding custom TypeC HPD info parsing in the USBDP PHY driver.

> diff --git a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
> index 38e8fe75718e4..9e49e7dbf420f 100644
> --- a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
[ ... ]
> @@ -71,6 +110,35 @@ static const struct drm_encoder_helper_funcs dw_dp_encoder_helper_funcs = {
>  	.atomic_check		= dw_dp_encoder_atomic_check,
>  };
>  
> +static struct regmap *dw_dp_rockchip_get_vo_grf(struct rockchip_dw_dp *dp)
> +{
> +	struct device_node *np = dev_of_node(dp->dev);
> +	struct of_phandle_args args;
> +	struct regmap *regmap;
> +	int ret;
> +
> +	ret = of_parse_phandle_with_args(np, "phys", "#phy-cells", 0, &args);
> +	if (ret)
> +		return ERR_PTR(-ENODEV);
> +
> +	/*
> +	 * Limit this workaround to RK3576 and RK3588, potential future platforms
> +	 * reusing the driver should just add a VO GRF phandle in the DisplayPort
> +	 * controller DT node.
> +	 */
> +	if (!of_device_is_compatible(args.np, "rockchip,rk3576-usbdp-phy") &&
> +	    !of_device_is_compatible(args.np, "rockchip,rk3588-usbdp-phy")) {
> +		regmap = ERR_PTR(-ENODEV);
> +		goto out_put_node;
> +	}
> +
> +	regmap = syscon_regmap_lookup_by_phandle(args.np, "rockchip,vo-grf");

[Severity: Medium]
The comment mentions that future platforms should add a VO GRF phandle directly
in the DisplayPort controller DT node, but the code doesn't seem to check for
it. Instead, it unconditionally requires a "phys" node that is compatible with
RK3576 or RK3588, and returns -ENODEV otherwise.

Should this function attempt to look up a VO GRF phandle on the controller node
(np) first, before falling back to this PHY workaround?

> +
> +out_put_node:
> +	of_node_put(args.np);
> +	return regmap;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260806-synopsys-dw-dp-improvements-v10-0-d84a26c303d6-ZGY8ohtN/[email protected]?part=13