[PATCH v4 2/2] drm/bridge: ti-sn65dsi83: Add reversed lvds lanes support

[Luca Ceresoli]

On Mon, 13 Jul 2026 10:26:20 +0200, Wojciech Dubowik <wojciech.dubowik at mt.com> wrote:

Hello Wojciech,

>
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index e6dbe51d0dba..d676ea3b40e8 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> @@ -148,6 +148,18 @@ enum sn65dsi83_lvds_term {
>  	OHM_200
>  };
>  
> +enum {
> +	NORMAL_LANE_MAPPING,
> +	REVERSE_LANE_MAPPING,

Please use a fixed prefix and use the suffix to differentiate:

  LANE_MAPPING_NORMAL,
  LANE_MAPPING_REVERSE,

> @@ -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;

The goto-based error handling is getting trickier and trickier with more
error conditions. Can you please add a preliminary commit in this series
that replaces:

 -struct device_node *endpoint;
 -endpoint = of_graph_get_endpoint_by_regs(dev->of_node, endpoint_reg, -1);
 +struct device_node *endpoint __free(device_node) =
 +         of_graph_get_endpoint_by_regs(dev->of_node, endpoint_reg, -1);

and removes the entire "ret = foo; goto exit;" logic with simple returns?

> +	}
> +
> +	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;
> +		}

I think a memcmp() can simplify the implementation if you use the same data
type (u32) for data_lanes and supported_data_lane_mapping[].

> +
> +		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;
> +		}
> +	}

And perhaps the whole if (!ret_data) here can be simplified a lot as this
(pseudocode, not tested):

  if (!ret_data) {
      if (memcmp(data_lanes[j], supported_data_lane_mapping[NORMAL_LANE_MAPPING], size) == 0)
          break;
      else if (memcmp(data_lanes[j], supported_data_lane_mapping[REVERSE_LANE_MAPPING], size) == 0)
          ctx->lvds_reverse_lanes_conf[channel] = true;
      else
          return dev_err_probe(...);
  }

Luca

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