Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: - [High] Inverted manipulation of MIPI_DSI_MODE_LPM in backlight update_status callback permanently corrupts DSI mode flags. - [High] Concurrent read-modify-write operations on dsi->mode_flags can cause state corruption. - [Medium] Backlight update_status callback sends DCS commands without checking if the panel is powered on. - [Medium] Incorrect devm registration order causes the DSI device to be detached while the DRM panel and backlight sysfs are still active. - [Medium] Missing NULL check for the return value of of_device_get_match_data() leads to a potential NULL pointer dereference. --
commit 97a4deef0ca55e71f3420d40260130de4242cae3 Author: Arpit Saini <[email protected]> drm/panel: add Ilitek ILI7807S panel driver This commit adds a DRM panel driver for the DLC DLC0697 1080x1920@60Hz MIPI DSI panel based on the Ilitek ILI7807S display controller. > diff --git a/drivers/gpu/drm/panel/panel-ilitek-ili7807s.c > b/drivers/gpu/drm/panel/panel-ilitek-ili7807s.c > new file mode 100644 > --- /dev/null > +++ b/drivers/gpu/drm/panel/panel-ilitek-ili7807s.c [ ... ] > +static int ili7807s_bl_update_status(struct backlight_device *bl) > +{ > + struct mipi_dsi_device *dsi = bl_get_data(bl); > + u16 brightness = backlight_get_brightness(bl); > + int ret; > + [Severity: Medium] Is it safe to send DCS commands here without checking if the panel is currently powered on? If userspace writes to the sysfs brightness control while the regulators are disabled and the panel is off, will this attempt to send commands and cause DSI bus timeouts? > + dsi->mode_flags &= ~MIPI_DSI_MODE_LPM; > + > + ret = mipi_dsi_dcs_set_display_brightness_large(dsi, brightness); > + > + dsi->mode_flags |= MIPI_DSI_MODE_LPM; [Severity: High] Does this sequence invert the intended mode flags? The default panel mode flags do not include MIPI_DSI_MODE_LPM. Clearing it beforehand and setting it afterward will permanently force the DSI host into Low Power Mode after the first backlight update, which might break subsequent video transmissions. [Severity: High] Can concurrent calls to ili7807s_on(), ili7807s_off(), and this backlight update callback cause a data race? These functions all modify dsi->mode_flags using non-atomic bitwise operations, which might clobber each other's changes if a backlight update is triggered from userspace simultaneously with a DRM DPMS event. [ ... ] > +static int ili7807s_probe(struct mipi_dsi_device *dsi) > +{ > + struct device *dev = &dsi->dev; > + const struct panel_desc *desc; > + struct ili7807s *ctx; > + int ret; > + > + ctx = devm_drm_panel_alloc(dev, struct ili7807s, panel, > + &ili7807s_panel_funcs, > + DRM_MODE_CONNECTOR_DSI); > + if (IS_ERR(ctx)) > + return PTR_ERR(ctx); > + > + desc = of_device_get_match_data(dev); > + ctx->desc = desc; [Severity: Medium] Can of_device_get_match_data() return NULL if the driver is bound manually via sysfs without an associated device tree node? If so, will dereferencing desc->lanes later in this function cause a NULL pointer dereference? [ ... ] > + ret = devm_drm_panel_add(dev, &ctx->panel); > + if (ret < 0) > + return dev_err_probe(dev, ret, "failed to add panel\n"); > + > + ret = devm_mipi_dsi_attach(dev, dsi); > + if (ret < 0) > + return dev_err_probe(dev, ret, "failed to attach dsi\n"); > + > + return 0; > +} [Severity: Medium] Does this devres registration order cause a teardown issue? Since devres cleans up in reverse order of allocation, devm_mipi_dsi_detach() will run before the DRM panel and backlight interfaces are removed. Could this leave the sysfs interfaces exposed to userspace while the underlying DSI device is already detached? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=2
