Devicetree
 help / color / mirror / Atom feed
From: "Luca Ceresoli" <luca.ceresoli@bootlin.com>
To: "Damon Ding" <damon.ding@rock-chips.com>,
	<sashiko-reviews@lists.linux.dev>
Cc: <robh@kernel.org>, <conor+dt@kernel.org>,
	<devicetree@vger.kernel.org>, <dri-devel@lists.freedesktop.org>,
	"Luca Ceresoli" <luca.ceresoli@bootlin.com>
Subject: Re: [PATCH v7 5/5] drm/bridge: analogix_dp: Add support for optional data-lanes mapping
Date: Wed, 19 Aug 2026 14:58:02 +0200	[thread overview]
Message-ID: <DKSXRX52W03E.2II5M5DIT6E1C@bootlin.com> (raw)
In-Reply-To: <f0151ee4-f660-43d2-a49d-13ab2ad12dbc@rock-chips.com>

Hello,

On Wed Aug 19, 2026 at 10:53 AM CEST, Damon Ding wrote:
> On 8/19/2026 4:04 PM, sashiko-bot@kernel.org 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 <damon.ding@rock-chips.com>
>>
>> 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.

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

  reply	other threads:[~2026-08-19 12:58 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-19  7:52 [PATCH v7 0/5] Add eDP lane mapping support Damon Ding
2026-08-19  7:52 ` [PATCH v7 1/5] dt-bindings: display: bridge: analogix-dp: Add data-lanes support for endpoint Damon Ding
2026-08-19 12:54   ` Luca Ceresoli
2026-08-19  7:52 ` [PATCH v7 2/5] drm/dp: Add helper to validate DP lane counts Damon Ding
2026-08-19  7:52 ` [PATCH v7 3/5] drm/bridge: analogix_dp: Restore mandatory samsung DP DT properties Damon Ding
2026-08-19 12:54   ` Luca Ceresoli
2026-08-19  7:52 ` [PATCH v7 4/5] drm/bridge: analogix_dp: Add validation for samsung,lane-count property Damon Ding
2026-08-19  7:52 ` [PATCH v7 5/5] drm/bridge: analogix_dp: Add support for optional data-lanes mapping Damon Ding
2026-08-19  8:04   ` sashiko-bot
2026-08-19  8:53     ` Damon Ding
2026-08-19 12:58       ` Luca Ceresoli [this message]
2026-08-19 12:54   ` Luca Ceresoli

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DKSXRX52W03E.2II5M5DIT6E1C@bootlin.com \
    --to=luca.ceresoli@bootlin.com \
    --cc=conor+dt@kernel.org \
    --cc=damon.ding@rock-chips.com \
    --cc=devicetree@vger.kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=robh@kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox