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 <[email protected]> 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/[email protected]?part=1
