Re: [PATCH v2 3/3] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The logic to detect single-link LVDS Channel B requires `port@2` to be absent from the device tree, but the DT binding strictly requires `port@2` to be present.
--

--- Patch [3]: [PATCH v2 3/3] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84 ---
commit e7655ab4d66cbec701adf0102160008944fa2a88
Author: Esben Haabendal <esben@geanix.com>

drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84

This adds support for using SN65DSI84 in single-link mode with output to
LVDS Channel B.

> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi83.c b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> index 7e73035d77986..02ae2b8179b07 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c
[ ... ]
> @@ -904,29 +907,38 @@ static int sn65dsi83_parse_dt(struct sn65dsi83 *ctx, enum sn65dsi83_model model)
>  		return ret;
>  
>  	ctx->lvds_dual_link = false;
> -	ctx->lvds_dual_link_even_odd_swap = false;
> +	ctx->lvds_channel_swap = false;
>  	if (model != MODEL_SN65DSI83) {
> -		struct device_node *port2, *port3;
> +		struct device_node *port0, *port1, *port2, *port3;
>  		int dual_link;
>  
> +		port0 = of_graph_get_port_by_id(dev->of_node, 0);
> +		port1 = of_graph_get_port_by_id(dev->of_node, 1);
>  		port2 = of_graph_get_port_by_id(dev->of_node, 2);
>  		port3 = of_graph_get_port_by_id(dev->of_node, 3);
>  		dual_link = drm_of_lvds_get_dual_link_pixel_order(port2, port3);
>  
>  		if (dual_link == DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS) {
>  			/* Odd pixels to LVDS Channel A, even pixels to B */
>  			ctx->lvds_dual_link = true;
>  		} else if (dual_link == DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS) {
>  			/* Even pixels to LVDS Channel A, odd pixels to B */
>  			ctx->lvds_dual_link = true;
> +			ctx->lvds_channel_swap = true;
> +		} else if (port0 && !port1 && port2 && !port3) {
> +			/* DSI Channel A to LVDS Channel A */
> +		} else if (port0 && !port1 && !port2 && port3) {
                                              ^^^^^^

[Severity: Medium]
Will this !port2 condition prevent the single-link LVDS Channel B feature
from working with a standard-compliant device tree?

If a user provides a compliant device tree by including an empty or
unconnected port@2 to satisfy the DT schema required list for this binding,
of_graph_get_port_by_id() will return a valid node.

The driver would then fail this check, fall through leaving panel_port = 2,
and subsequently fail to probe when it cannot find a panel connected to the
empty port@2.

> +			/* DSI Channel A to LVDS Channel B */
> +			ctx->lvds_channel_swap = true;
> +			panel_port = 3;
>  		}
> +		of_node_put(port0);
> +		of_node_put(port1);
> +		of_node_put(port2);
> +		of_node_put(port3);
>  	}
>  
> -	panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node, 2, 0);
> +	panel_bridge = devm_drm_of_get_bridge(dev, dev->of_node, panel_port, 0);
>  	if (IS_ERR(panel_bridge))
>  		return dev_err_probe(dev, PTR_ERR(panel_bridge), "Failed to get panel bridge\n");

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260715-ti-sn65dsi83-fixes-v2-0-ebc4c3fe29b6@geanix.com?part=3