Re: [PATCH 1/2] 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] Leftover debug `pr_warn` statements unconditionally print warnings on standard, valid hardware configurations during device probe.
--

--- Patch [1]: [PATCH 1/2] drm/bridge: ti-sn65dsi83: Support LVDS Channel B on SN65DSI84 ---
commit f232cbf82d686f0a82f41ef8eedda5654e9deea0
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 42b451432bbb6..32e9eadd7452f 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi83.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi83.c

[ ... ]

> @@ -904,29 +907,40 @@ 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);
> -		of_node_put(port2);
> -		of_node_put(port3);
>  
>  		if (dual_link == DRM_LVDS_DUAL_LINK_ODD_EVEN_PIXELS) {
> -			ctx->lvds_dual_link = true;
>  			/* Odd pixels to LVDS Channel A, even pixels to B */
> -			ctx->lvds_dual_link_even_odd_swap = false;
> -		} else if (dual_link == DRM_LVDS_DUAL_LINK_EVEN_ODD_PIXELS) {
>  			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_even_odd_swap = true;
> +			ctx->lvds_dual_link = true;
> +			ctx->lvds_channel_swap = true;
> +		} else if (port0 && !port1 && port2 && !port3) {
> +			/* DSI Channel A to LVDS Channel A */
> +			pr_warn("%s: DSI Channel A to LVDS Channel A\n", __func__);
> +		} else if (port0 && !port1 && !port2 && port3) {
> +			/* DSI Channel A to LVDS Channel B */
> +			pr_warn("%s: DSI Channel A to LVDS Channel B\n", __func__);

[Severity: Medium]
Are these pr_warn() statements leftover debugging code?

Since this patch introduces normal support for single-link modes, printing
unconditional warnings in sn65dsi83_parse_dt() on perfectly valid hardware 
configurations (single-link DSI to LVDS channel A or B) will pollute the 
dmesg log during device probe. This might cause false failures in CI systems 
that check for kernel warnings, and confuse users into thinking there is a 
hardware or driver issue.

> +			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/20260711-ti-sn65dsi83-fixes-v1-0-d85eb5342b98@geanix.com?part=1