Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] The panel reset GPIO is initialized to a de-asserted state during probe, violating power sequencing and potentially back-powering the display controller. - [Medium] The backlight update status function permanently clears the `MIPI_DSI_MODE_LPM` flag on error. --
commit ff679a020d6cdcdc5214b83056b055c35077760d Author: Alexander Koskovich <[email protected]> drm/panel: Add driver for Raydium RM69220 DDIC This patch adds a new DRM panel driver for the Raydium RM69220 display driver IC, primarily supporting the BOE BF068MWM-TD0 panel. > diff --git a/drivers/gpu/drm/panel/panel-raydium-rm69220.c > b/drivers/gpu/drm/panel/panel-raydium-rm69220.c > new file mode 100644 > index 0000000000000..4fa3036c4b5fb > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-raydium-rm69220.c [ ... ] > +static int raydium_rm69220_bl_update_status(struct backlight_device *bl) > +{ > + struct mipi_dsi_device *dsi = bl_get_data(bl); > + u16 brightness = backlight_get_brightness(bl); > + int ret; > + > + dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; > + > + ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness); > + if (ret < 0) > + return ret; [Severity: Medium] If mipi_dsi_dcs_set_display_brightness_large() fails here, does this return path leave MIPI_DSI_MODE_LPM permanently cleared? It seems this might cause all subsequent DSI commands to be sent in High Speed mode. > + > + dsi->mode_flags |= MIPI_DSI_MODE_LPM; > + > + return 0; > +} [ ... ] > +static int raydium_rm69220_probe(struct mipi_dsi_device *dsi) > +{ > + struct device *dev = &dsi->dev; > + struct raydium_rm69220 *ctx; > + int ret; [ ... ] > + ret = devm_regulator_bulk_get_const(dev, > + > ARRAY_SIZE(raydium_rm69220_supplies), > + raydium_rm69220_supplies, > + &ctx->supplies); > + if (ret < 0) > + return ret; > + > + ctx->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW); [Severity: High] Does initializing the reset GPIO to GPIOD_OUT_LOW (logically de-asserted) during probe risk back-powering the display controller? Since the panel's power regulators are not enabled yet, could this drive current through the internal ESD diodes into the unpowered rails? > + if (IS_ERR(ctx->reset_gpio)) > + return dev_err_probe(dev, PTR_ERR(ctx->reset_gpio), > + "Failed to get reset-gpios\n"); > + [ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2
