Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [High] Probe definitively fails on valid legacy 3-lane or 0-indexed device 
trees due to rigid array bounds.
--

--- Patch [1]: [PATCH v3 1/2] drm/bridge: ti-sn65dsi83: Add reversed lvds lanes 
support ---
commit 47fc41c5744e78e6f8072b1226e9548c69361da5
Author: Wojciech Dubowik <[email protected]>

drm/bridge: ti-sn65dsi83: Add reversed lvds lanes support

The chip supports output lvds lanes in two orders, default <1 2 3 4>
and <4 3 2 1>. Add parsing of an optional output lvds data-lanes
property so we can inform chip that the lanes have been reversed.

> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c 
> b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
[ ... ]
> @@ -854,6 +873,37 @@ static int sn65dsi83_parse_lvds_endpoint(struct 
> sn65dsi83 *ctx, int channel)
>               goto exit;
>       }
>  
> +     ret_data = of_property_read_u32_array(endpoint, "data-lanes", 
> data_lanes,
> +                     ARRAY_SIZE(data_lanes));
> +     if (ret_data != 0 && ret_data != -EINVAL) {

[Severity: High]
Does this code break valid legacy device trees that define fewer than 4 lanes?

If a device tree has a data-lanes property on the LVDS endpoint with fewer
than 4 lanes (e.g., <1 2 3>), of_property_read_u32_array() will return
-EOVERFLOW because it rigidly expects exactly ARRAY_SIZE(data_lanes) elements.
This will immediately abort the probe, breaking previously working displays.

> +             ret = ret_data;
> +             goto exit;
> +     }
> +
> +     if (!ret_data) {
> +             for (i = 0; i < ARRAY_SIZE(supported_data_lane_mapping); i++) {
> +                     for (j = 0; j < DATA_LANES_COUNT; j++) {
> +                             if (data_lanes[j] != 
> supported_data_lane_mapping[i][j])
> +                                     break;
> +                     }
> +
> +                     if (j == DATA_LANES_COUNT)
> +                             break;
> +             }
> +
> +             switch (i) {
> +             case NORMAL_LANE_MAPPING:
> +                     break;
> +             case REVERSE_LANE_MAPPING:
> +                     ctx->lvds_reverse_lanes_conf[channel] = true;
> +                     break;
> +             default:
> +                     dev_err(dev, "invalid data lanes mapping\n");
> +                     ret = -EINVAL;

[Severity: High]
Will this cause the probe to fail on 0-indexed device tree configurations?

The array comparison logic above this switch strictly checks for 1-indexed
mappings. If the device tree uses a valid 0-indexed scheme (e.g., <0 1 2 3>),
it falls through to this default case, causing the probe to fail with -EINVAL.

> +                     goto exit;
> +             }
> +     }

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=1

Reply via email to