Re: [PATCH v2] drm/bridge: ti-sn65dsi83: Add reversed lvds lanes support

[sashiko-bot@kernel.org]

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 <Wojciech.Dubowik@mt.com>

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/20260710082630.394040-1-wojciech.dubowik@mt.com?part=1