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

[Damon Ding <damon.ding@rock-chips.com>]

Hi Luca,

On 6/27/2026 12:29 AM, Luca Ceresoli wrote:
> On Thu, 04 Jun 2026 16:52:19 +0800, Damon Ding <damon.ding@rock-chips.com> wrote:
> 
> Hello Damon,
> 
>>
>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> index 7a85774aaac1..e120ef3320c1 100644
>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>> @@ -1261,8 +1262,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");
> 
> I think this report by sashiko makes sense:
> 
>    >  sashiko-bot@kernel.org <sashiko-bot@kernel.org>:
>    >
>    >  [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?
> 
>   (via: https://patch.msgid.link/20260604090935.7FC051F00898@smtp.kernel.org)
> 
> Can you comment on this?
> 
> 

I was also confused about this handling at first. From commit 
0d0abd894ead ("drm: bridge: analogix/dp: add max link rate and lane 
count limit for RK3288"), its commit message does not explain why 
samsung,lane-count was changed from a mandatory to optional property.

After digging into the code flow, I found that 
analogix_dp_full_link_train() picks the smaller value between the 
platform-supported lane count and the lane count retrieved from sink 
DPCD for link training. If the samsung,lane-count property is 
missing/invalid here, link training will end up using an unexpected lane 
count configuration.

Additionally, I checked Samsung’s upstream device trees that utilize 
this DP controller, and all of them carry the lane-count property. Based 
on this observation, I believe restoring the strict mandatory 
requirement for this property makes sense.

Should I create an independent fix patch to revert these two properties 
to mandatory, instead of bundling the fix in this series?

Best regards,
Damon