Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Incomplete fix for the unrecoverable hardware state. The patch removes the reset assertion from `sw43408_unprepare()` to fix a hardware lockup, but the exact same reset assertion remains in the error path of `sw43408_prepare()`. - [High] Missing `drm_panel_remove()` in the probe error path leaves a dangling panel in the global registry, leading to a Use-After-Free. -- --- Patch [1]: [PATCH v2 1/2] drm/panel: lg-sw43408: do not assert reset across unprepare --- commit c7e02c477c8a5888497460cbf8b8bcfc7759b23d Author: David Heidelberg <[email protected]> drm/panel: lg-sw43408: do not assert reset across unprepare This patch removes the reset assertion from the unprepare() function. Asserting the reset GPIO while the logic rail remains powered puts the DDIC into a state it cannot recover from. > diff --git a/drivers/gpu/drm/panel/panel-lg-sw43408.c > b/drivers/gpu/drm/panel/panel-lg-sw43408.c > index 293826c5006b9..0bf8e08f10b9c 100644 > --- a/drivers/gpu/drm/panel/panel-lg-sw43408.c > +++ b/drivers/gpu/drm/panel/panel-lg-sw43408.c > @@ -55,8 +55,14 @@ static int sw43408_unprepare(struct drm_panel *panel) > > mipi_dsi_msleep(&ctx, 100); > > - gpiod_set_value(sw43408->reset_gpio, 1); > - > + /* > + * Keep reset deasserted: vddi may stay powered (it is shared with > + * the touchscreen on Pixel 3) and holding the DDIC in reset while > + * its logic rail is up puts it into a state that no init sequence > + * recovers from - the panel keeps answering DCS commands and > + * reports display-on, but never lights up again. The vendor stack > + * only ever toggles reset as part of a powered-up init sequence. > + */ > ret = regulator_bulk_disable(ARRAY_SIZE(sw43408_supplies), > sw43408->supplies); > > return ret ? : ctx.accum_err; [Severity: High] This isn't a bug introduced by this patch, but the same reset assertion that causes the unrecoverable hardware state still exists in the error path of sw43408_prepare(). If sw43408_program() fails, the driver branches to poweroff and asserts the reset GPIO while vddi might still be powered: drivers/gpu/drm/panel/panel-lg-sw43408.c:sw43408_prepare() { ... ret = sw43408_program(panel); if (ret) goto poweroff; return 0; poweroff: gpiod_set_value(ctx->reset_gpio, 1); regulator_bulk_disable(ARRAY_SIZE(sw43408_supplies), ctx->supplies); return ret; } Could this leave the DDIC in the same unrecoverable state if a DCS command fails during initialization? [Severity: High] This is a pre-existing issue, but there appears to be a potential use-after-free in the probe function. If mipi_dsi_attach() fails in sw43408_probe(), the driver returns an error without calling drm_panel_remove(): drivers/gpu/drm/panel/panel-lg-sw43408.c:sw43408_probe() { ... ret = sw43408_add(ctx); if (ret < 0) return ret; ... return mipi_dsi_attach(dsi); } Since sw43408_add() calls drm_panel_add() rather than devm_drm_panel_add(), doesn't this leave the panel globally registered? If devres then frees the panel's regulators and GPIOs upon probe failure, could a display controller attempting to use the registered panel encounter a use-after-free? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
