Thank you for your contribution! Sashiko AI review found 1 potential issue(s) 
to consider:
- [Medium] Strict parsing of the `data-lanes` property breaks device probe for 
valid configurations with fewer than 4 lanes or non-matching lane orderings.
--

--- Patch [1]: [PATCH v2] drm/bridge: ti-sn65dsi83: Add reversed lvds lanes 
support ---
commit 33b38b449204d2ada7177a7a3ded61d90bf26441
Author: Wojciech Dubowik <[email protected]>

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

This commit parses an optional data-lanes property on the LVDS endpoint to
inform the chip if the lanes have been reversed.

> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c 
> b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index e6dbe51d0dba9..d676ea3b40e87 100644
> --- 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) {
> +             ret = ret_data;
> +             goto exit;
> +     }

[Severity: Medium]
Will this break the device probe for valid configurations with fewer than 4
lanes?

If a device tree specifies a valid data-lanes property on the LVDS endpoint
that has fewer than 4 elements (such as <1 2 3> for an 18-bpp display using
3 lanes), of_property_read_u32_array() will return -EOVERFLOW.

Since -EOVERFLOW is not -EINVAL, the probe will be aborted here.

> +
> +     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;
> +                     goto exit;

[Severity: Medium]
Does this code also incorrectly reject valid 4-element arrays with alternative
numbering?

If the mapping doesn't exactly match the two hardcoded 1-based arrays (for
example, a 0-indexed mapping like <0 1 2 3>), it falls through to this
default case, returning -EINVAL and aborting the probe.

Could we make the parsing more lenient to avoid breaking existing setups
that specify valid but non-matching lane orderings?

> +             }
> +     }

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

Reply via email to