Re: [PATCH v5 3/4] drm/bridge: analogix_dp: Add validation for samsung,lane-count property

[sashiko-bot@kernel.org]

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

New issues:
- [High] The patch makes the deprecated and optional `samsung,lane-count` property a strict requirement, causing an unconditional probe failure if it is omitted.

Pre-existing issues:
- [High] Missing handling for absent DT properties causes lane count and link rate to be incorrectly clamped to 0, breaking link training.
--

commit 65a66d32582ccfce55e0e84f53bc63077d25103c
Author: Damon Ding <damon.ding@rock-chips.com>

drm/bridge: analogix_dp: Add validation for samsung,lane-count property

Add validity check for samsung,lane-count to ensure DT-provided
lane count values are specification-compliant.

> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index 8cf6b73bceac0..f23c2ae6e10d0 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
[ ... ]
> @@ -1260,8 +1261,11 @@ static int analogix_dp_dt_parse_pdata(struct analogix_dp_device *dp)
>  		 */
>  		of_property_read_u32(dp_node, "samsung,link-rate",
>  				     &video_info->max_link_rate);
> -		of_property_read_u32(dp_node, "samsung,lane-count",
> -				     &video_info->max_lane_count);
> +		ret = of_property_read_u32(dp_node, "samsung,lane-count",
> +					   &video_info->max_lane_count);
> +		if (ret || !drm_dp_lane_count_is_valid(video_info->max_lane_count))
> +			return dev_err_probe(dp->dev, ret ? ret : -EINVAL,
> +					     "failed to parse samsung,lane-count\n");

[Severity: High]
Does this make the optional and deprecated samsung,lane-count property a
strict requirement?

If samsung,lane-count is absent from the device tree, of_property_read_u32()
returns -EINVAL. This causes the condition to evaluate to true, aborting the
probe with an error.

According to the device tree bindings
(Documentation/devicetree/bindings/display/samsung/samsung,exynos5-dp.yaml),
this property is marked as deprecated and explicitly optional because the
lane count can be read from the monitor. Does this patch break compatibility
with device trees that rightfully omit this deprecated property?

[Severity: High]
This is a pre-existing issue, but there is an underlying problem in
analogix_dp_full_link_train() when these optional properties are omitted:

analogix_dp_full_link_train() {
    ...
	/* Setup TX lane count & rate */
	if (dp->link_train.lane_count > max_lanes)
		dp->link_train.lane_count = max_lanes;
	if (dp->link_train.link_rate > max_rate)
		dp->link_train.link_rate = max_rate;
    ...
}

If the deprecated samsung,lane-count or samsung,link-rate properties are
rightfully omitted from the device tree, their corresponding limits remain 0.

The driver then executes the above clamp logic, which incorrectly clamps the
valid DPCD values to 0. This completely breaks DP link training and results
in a non-functional display.

Should the driver ignore the limits if they are 0 (e.g., checking if
max_lanes is non-zero before clamping), allowing it to correctly fall back
to the monitor's DPCD capabilities? Fixing the probe failure above will
just expose this runtime breakage.

>  		break;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604085220.2862986-1-damon.ding@rock-chips.com?part=3