Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues: - [Low] Redundant duplicate assignment of `prepare_prev_first` flag. Pre-existing issues: - [High] Missing cleanup in the error paths of `jadard_prepare()` causes a permanent leak of the regulator enable counts. - [High] Premature publication of the panel in `jadard_dsi_probe()` allows a race condition leading to a NULL pointer dereference. -- --- Patch [2]: [PATCH 2/5] drm: panel: jd9365da: Add prepare_prev_first flag to drm_panel --- commit 1a793dbc7476444fc7003da84dbce8a3bff9ad98 Author: Marc-Olivier Champagne <[email protected]> drm: panel: jd9365da: Add prepare_prev_first flag to drm_panel The DSI host must be enabled for the panel to be initialized in prepare(). Set the prepare_prev_first flag to guarantee this. This fixes the panel operation on Renesas RZ/G2LC SoC DSI host. > diff --git a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > index 1ef0c937863a2..a79dc8f8d5af9 100644 > --- a/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > +++ b/drivers/gpu/drm/panel/panel-jadard-jd9365da-h3.c > @@ -2993,6 +2993,8 @@ static int jadard_dsi_probe(struct mipi_dsi_device *dsi) > return dev_err_probe(&dsi->dev, PTR_ERR(jadard->vccio), > "failed to get vccio regulator\n"); > > + jadard->panel.prepare_prev_first = true; > + [Severity: Low] Does this introduce a redundant assignment? Looking further down in jadard_dsi_probe(), the exact same flag appears to be set again just before panel registration: jadard_dsi_probe() { ... jadard->panel.prepare_prev_first = true; drm_panel_add(&jadard->panel); ... } Can we remove one of these duplicate assignments? [Severity: High] This is a pre-existing issue, but does jadard_prepare() leak regulator reference counts on error paths? If regulator_enable(jadard->vccio) succeeds, but a subsequent step fails: jadard_prepare() { ... ret = regulator_enable(jadard->vccio); ... ret = regulator_enable(jadard->vdd); if (ret) return ret; and later with mipi_dsi_dcs_nop(): ret = mipi_dsi_dcs_nop(jadard->dsi); if (ret) return ret; } The function returns without disabling the previously enabled regulators. Because the panel prepare fails, the DRM core won't mark it as prepared, and jadard_unprepare() will never be called to clean up. Can this prevent the system from entering low-power states or re-initializing the display properly? [Severity: High] This isn't a bug introduced by this patch, but is there a race condition with premature panel publication in jadard_dsi_probe()? The panel is added to the global list before its required internal pointers are initialized: jadard_dsi_probe() { ... drm_panel_add(&jadard->panel); mipi_dsi_set_drvdata(dsi, jadard); jadard->dsi = dsi; jadard->desc = desc; } If a display controller finds the panel concurrently (e.g., via probe deferral resolution) and calls drm_panel_prepare(), jadard_prepare() could execute before jadard_dsi_probe() finishes: jadard_prepare() { ... if (jadard->desc->vcioo_to_lp11_delay_ms) ... } Can this dereference a NULL jadard->desc pointer? Should drm_panel_add() be moved to the very end of the probe function? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260708213500.1867092-1-marc-olivier.champa...@savoirfairelinux.com?part=2
