Re: [PATCH v3 06/10] drm/rockchip: dw_dp: Implement out-of-band HPD handling

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Using IS_ERR() to validate the return value of platform_get_mem_or_io() fails to catch NULL, leading to a kernel crash when the resource is missing.
- [Medium] The `dp_dp_rockchip_get_vo_grf` function fails to implement the documented fallback to read the `vo-grf` phandle from the DisplayPort device node.
--

commit 2739e6354d59ce0c07c5ba0c2a12b2779e399b4a
Author: Sebastian Reichel <sebastian.reichel@collabora.com>
drm/rockchip: dw_dp: Implement out-of-band HPD handling

Implement out-of-band hotplug handling to receive external hotplug
information from the USB-C state machine. This properly detects if a cable
is plugged in for adapters.

> diff --git a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
> index 35598ab9fe84a..9c53f1d2c29ab 100644
> --- a/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
> +++ b/drivers/gpu/drm/rockchip/dw_dp-rockchip.c
[ ... ]
> @@ -72,14 +117,49 @@ static const struct drm_encoder_helper_funcs dw_dp_encoder_helper_funcs = {
>  	.atomic_check		= dw_dp_encoder_atomic_check,
>  };
>  
> +static struct regmap *dp_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, new platforms should
> +	 * add a VO GRF phandle in the DisplayPort 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]
Does this function actually implement the forward-compatibility behavior
described in the comment?

If a new platform follows the advice and adds a VO GRF phandle directly to
the DisplayPort DT node (dropping the "phys" property), it looks like the
unconditional of_parse_phandle_with_args() check for "phys" will fail and
return ERR_PTR(-ENODEV) early.

Should there be a primary lookup path for "rockchip,vo-grf" on the DP node
itself before falling back to this PHY workaround?

> +
> +out_put_node:
> +	of_node_put(args.np);
> +	return regmap;
> +}
> +
>  static int dw_dp_rockchip_bind(struct device *dev, struct device *master, void *data)
>  {
> -	const struct dw_dp_plat_data *plat_data;
> +	const struct rockchip_dw_dp_plat_data *plat_data_const;
> +	struct platform_device *pdev = to_platform_device(dev);
> +	struct dw_dp_plat_data *plat_data;
>  	struct drm_device *drm_dev = data;
>  	struct rockchip_dw_dp *dp;
>  	struct drm_encoder *encoder;
>  	struct drm_connector *connector;
> -	int ret;
> +	struct resource *res;
> +	int ret, id;
> +
> +	plat_data = drmm_kzalloc(drm_dev, sizeof(*plat_data), GFP_KERNEL);
> +	if (!plat_data)
> +		return -ENOMEM;
[ ... ]
> +	dp->pdata = plat_data_const;
> +
> +	res = platform_get_mem_or_io(pdev, 0);
> +	if (IS_ERR(res))
> +		return PTR_ERR(res);
> +
> +	/* find the DisplayPort ID from the io address */
> +	dp->id = -ENODEV;
> +	for (id = 0; id < plat_data_const->num_ctrls; id++) {
> +		if (res->start == plat_data_const->ctrl_ids[id]) {
> +			dp->id = id;
> +			break;
> +		}
> +	}

[Severity: High]
Can this code cause a NULL pointer dereference if the resource is missing
from the device tree?

Looking at platform_get_mem_or_io(), it returns NULL when the resource is
not found, rather than an ERR_PTR. Because IS_ERR(NULL) evaluates to false,
the check "if (IS_ERR(res))" would let a NULL pointer slip through. This
NULL pointer would then be dereferenced when accessing res->start in the
loop.

Should this be updated to check "if (!res)" instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260612-synopsys-dw-dp-improvements-v3-0-dc61e6352508@collabora.com?part=6