Re: [PATCH v7 5/5] drm/bridge: analogix_dp: Add support for optional data-lanes mapping

Hi Luca,

On 8/19/2026 8:58 PM, Luca Ceresoli wrote:
> Hello,
> 
> On Wed Aug 19, 2026 at 10:53 AM CEST, Damon Ding wrote:
>> On 8/19/2026 4:04 PM, [email protected] wrote:
>>> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
>>> - [High] Logic error in hardware lane mapping configuration causes inverted assignments for non-symmetric maps.
>>> - [Low] Device Tree endpoint lookup inconsistency when querying lane counts versus reading properties.
>>> --
>>>
>>> --- Patch [5]: [PATCH v7 5/5] drm/bridge: analogix_dp: Add support for optional data-lanes mapping ---
>>> commit 1c15eb6132eedc3d606168644e5f97918c8e3651
>>> Author: Damon Ding <[email protected]>
>>>
>>> drm/bridge: analogix_dp: Add support for optional data-lanes mapping
>>>
>>> Parse the optional 'data-lanes' device tree property to support
>>> custom physical lane mapping configuration. If no valid configuration
>>> is found, fall back to the default lane map automatically.
>>>
>>>> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>>> index 039e076d2b8ab..cb06347196cef 100644
>>>> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>>> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
>>> [ ... ]
>>>> @@ -1240,6 +1240,59 @@ static const struct drm_bridge_funcs analogix_dp_bridge_funcs = {
>>>>    	.detect = analogix_dp_bridge_detect,
>>>>    };
>>>>
>>>> +static int analogix_dp_dt_parse_lanes_map(struct analogix_dp_device *dp)
>>>> +{
>>> [ ... ]
>>>> +	num_lanes = drm_of_get_data_lanes_count_ep(dp->dev->of_node, 1, 0, 1,
>>>> +						   video_info->max_lane_count);
>>>> +	if (num_lanes < 0)
>>>> +		return -EINVAL;
>>>> +
>>>> +	endpoint = of_graph_get_endpoint_by_regs(dp->dev->of_node, 1, -1);
>>>
>>
>> Hi Luca,
>>
>> Thanks for taking a look at previous v6 patch series. This Sashiko
>> warning was also present in v6. Sorry for CC'ing you out‑of‑the‑blue
>> without prior heads‑up.
>>
>>> [Severity: Low]
>>> Does passing -1 here potentially retrieve a different endpoint from the one
>>> validated with reg=0 above?
>>>
>>> If multiple endpoints exist on the DP port and the target endpoint is not
>>> positionally first, could this mismatch result in reading the data-lanes
>>> property from the wrong endpoint?
>>>
>>
>> For Analogix DP, port@1 will only ever have a single endpoint.
>> Multi‑output configurations are not supported by the hardware, so
>> passing -1 cannot select a mismatched endpoint. There is no risk of
>> retrieving the 'data‑lanes' property from an unintended endpoint.
> 
> I understand (1, 0) and (1, -1) will always return the same node in the
> real cases, but why doing it? Use the same indices both times and avoid
> future readers scratch their head thinking why it's different and whether
> they might report a different node in a future case. Also, if parsing a
> broken device tree, you may point at two different nodes here. Keep things
> simple, readable and reliable.
> 
> 

Thanks for your comment.

Agreed, inconsistent indices are confusing and unsafe with broken DT.
Will switch of_graph_get_endpoint_by_regs() to reg=0 to match 
drm_of_get_data_lanes_count_ep() in v8.

Best regards,
Damon