Re: [PATCH v7 5/5] drm/bridge: analogix_dp: Add support for optional data-lanes mapping
"Luca Ceresoli" <[email protected]>
| Newsgroups | dev.linux.lists.sashiko-reviews,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-devicetree |
|---|---|
| Message-ID | <[email protected]> |
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. Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com