On Mon, 13 Jul 2026 10:26:20 +0200, Wojciech Dubowik <[email protected]>
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
