Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] The panel is added to the DRM subsystem before its DSI links are fully configured and attached, creating a race condition. - [High] Asymmetric panel `.prepare`/`.enable` and `.disable`/`.unprepare` callbacks. - [High] Missing device link to the secondary DSI host allows it to be unbound while the panel is still active, leading to a use-after-free. --
commit c609396440aafc34634a32d089165e7df2d383ce Author: KancyJoe <[email protected]> drm: panel: add support for the Renesas R63419 based dual-DSI video mode Display Panels Implement support for the Renesas 63419 based dual-DSI video mode Display Panels found in the Ayaneo gaming handled devices. > diff --git a/drivers/gpu/drm/panel/panel-renesas-r63419.c > b/drivers/gpu/drm/panel/panel-renesas-r63419.c > new file mode 100644 > index 0000000000000..a70de95d4f215 > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-renesas-r63419.c > @@ -0,0 +1,366 @@ [ ... ] > +static const struct drm_panel_funcs renesas_r63419_panel_funcs = { > + .disable = renesas_r63419_disable, > + .prepare = renesas_r63419_prepare, > + .unprepare = renesas_r63419_unprepare, > + .get_modes = renesas_r63419_get_modes, > + .get_orientation = renesas_r63419_get_orientation, > +}; [Severity: High] Does this create asymmetric panel callbacks by providing .disable without an .enable? If the DRM core or a bridge driver performs a simple display cycle by calling drm_panel_disable() followed by drm_panel_enable() without a full unprepare/prepare cycle, the panel will be turned off in .disable but never turned back on since .enable is NULL. Should the DSI power-on commands in renesas_r63419_on() be moved to an .enable callback, or alternatively, should the power-off commands be moved to .unprepare and .disable removed entirely? [ ... ] > +static int renesas_r63419_probe(struct mipi_dsi_device *dsi) > +{ [ ... ] > + /* Register the second DSI device */ > + ctx->dsi[1] = devm_mipi_dsi_device_register_full(dev, dsi1_host, &info); > + if (IS_ERR(ctx->dsi[1])) > + return dev_err_probe(dev, PTR_ERR(ctx->dsi[1]), > + "Failed to register second DSI device\n"); [Severity: High] Does this leave the secondary DSI device vulnerable to uncoordinated unbinding? Since ctx->dsi[1] is registered as a child of dsi1_host, if dsi1_host is unbound via sysfs or module removal, the driver core will automatically unregister and free its children. Without a device link to dsi1_host->dev, the panel driver is not notified of this removal and will retain a dangling pointer to ctx->dsi[1]. If the panel is subsequently disabled or unbound, it could dereference ctx->dsi[1] in renesas_r63419_disable() or trigger a double-unregister in the devres cleanup path. [ ... ] > + ret = devm_drm_panel_add(dev, &ctx->panel); > + if (ret) > + return dev_err_probe(dev, ret, "Failed to add panel\n"); > + > + /* Configure and attach both DSI devices */ > + for (i = 0; i < ARRAY_SIZE(ctx->dsi); i++) { > + ctx->dsi[i]->lanes = ctx->desc->lanes; > + ctx->dsi[i]->format = ctx->desc->format; > + ctx->dsi[i]->mode_flags = ctx->desc->mode_flags; > + > + ret = devm_mipi_dsi_attach(dev, ctx->dsi[i]); > + if (ret < 0) > + return dev_err_probe(dev, ret, > + "Failed to attach DSI device > %d\n", i); > + } > + > + return 0; > +} [Severity: High] Could this expose the panel to the DRM subsystem too early? Calling devm_drm_panel_add() before configuring and attaching the DSI devices creates a race condition where a display controller could bind and attempt to enable the panel immediately. This would invoke the panel's .prepare() callback before the DSI lanes, format, and mode flags are applied, and before the DSI hosts are actually attached, potentially leading to DCS command transmission failures or hardware configuration errors. Should devm_drm_panel_add() be called after the DSI devices are successfully attached? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/20260605-topic-sm8650-ayaneo-pocket-s2-r63419-v7-0-b84b6da84...@linaro.org?part=2
