Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [Medium] drm/panel: resource leak and unbalanced regulator disable on error path - [Medium] drm/panel: DCS write errors are ignored during initialization sequence - [Medium] drm/panel: commit message and file content mismatch (5-inch vs 7-inch panel) --
commit 6f19328aa9181fa01e0009c188e0a4fe6e32ccb1 Author: Maxime Ripard <[email protected]> drm/panel: dsi-bpf: Add Raspberry Pi 5-inch panel BPF program This commit translates the initialization sequence for the Raspberry Pi 5-inch MIPI-DSI panel into a BPF program. > drm/panel: dsi-bpf: Add Raspberry Pi 5-inch panel BPF program > > Translate the raspberrypi,dsi-5inch initialization sequence from > drivers/gpu/drm/panel/panel-ilitek-ili9881c.c into a BPF program. [Severity: Medium] Is there a mismatch between the commit message and the contents of this patch? The commit message states it adds the "5-inch panel" and translates the "raspberrypi,dsi-5inch" initialization sequence. However, the patch actually adds a file named Raspberrypi__dsi-7inch.bpf.c containing the 7-inch initialization sequence, and uses the "raspberrypi,dsi-7inch" compatible string. > diff --git a/drivers/gpu/drm/panel/bpf/progs/Raspberrypi__dsi-7inch.bpf.c > b/drivers/gpu/drm/panel/bpf/progs/Raspberrypi__dsi-7inch.bpf.c > new file mode 100644 > index 0000000000000..2055828de2f40 > --- /dev/null > +++ b/drivers/gpu/drm/panel/bpf/progs/Raspberrypi__dsi-7inch.bpf.c > @@ -0,0 +1,271 @@ [ ... ] > +#define PAGE(p) do { \ > + const __u8 _d[] = { 0x98, 0x81, (p) }; \ > + panel_bpf_mipi_dsi_dcs_write(pctx, 0xff, _d, sizeof(_d));\ > +} while (0) > + > +#define CMD(c, d) panel_bpf_mipi_dsi_dcs_write_byte(pctx, (c), (d)) [Severity: Medium] Do these macros ignore DCS write errors during the initialization sequence? By discarding the return value of panel_bpf_mipi_dsi_dcs_write() and panel_bpf_mipi_dsi_dcs_write_byte(), a failure (such as a DSI bus timeout) will not abort the program. It will blindly continue sending over 150 commands to a potentially broken panel, sequentially timing out on each command, which causes a massive initialization delay and falsely reports a successful panel initialization. > +SEC(PANEL_BPF_MIPI_DSI_PREPARE) > +int BPF_PROG(panel_prepare, struct panel_bpf_mipi_dsi_ctx *pctx) > +{ > + int ret; > + > + ret = panel_bpf_mipi_dsi_regulator_enable_and_wait(pctx, > PANEL_BPF_MIPI_DSI_SUPPLY_IOVCC, 5); > + if (ret) > + return ret; > + > + ret = panel_bpf_mipi_dsi_regulator_enable_and_wait(pctx, > PANEL_BPF_MIPI_DSI_SUPPLY_VCC, 5); > + if (ret) > + return ret; [Severity: Medium] Does this error path leak the previously enabled IOVCC regulator? If panel_bpf_mipi_dsi_regulator_enable_and_wait() for VCC fails here, or if subsequent operations like exit_sleep_mode() fail, panel_prepare() returns an error without disabling the already-enabled IOVCC regulator or reverting the GPIO state. [ ... ] > +SEC(PANEL_BPF_MIPI_DSI_UNPREPARE) > +int BPF_PROG(panel_unprepare, struct panel_bpf_mipi_dsi_ctx *pctx) > +{ > + panel_bpf_mipi_dsi_set_display_off(pctx); > + panel_bpf_mipi_dsi_enter_sleep_mode(pctx); > + > + panel_bpf_mipi_dsi_regulator_disable(pctx, > PANEL_BPF_MIPI_DSI_SUPPLY_VCC); > + panel_bpf_mipi_dsi_regulator_disable(pctx, > PANEL_BPF_MIPI_DSI_SUPPLY_IOVCC); [Severity: Medium] Does this unconditionally disable regulators, even on error paths? When the DRM core tears down the pipeline following an error in panel_prepare(), it unconditionally calls panel_unprepare(). Attempting to disable a regulator that failed to enable (e.g., VCC) will trigger a kernel warning for an unbalanced disable. > + panel_bpf_mipi_dsi_gpio_enable(pctx, PANEL_BPF_MIPI_DSI_GPIO_RESET); > + > + return 0; > +} > + > +PANEL_BPF_MIPI_DSI_OPS(raspberrypi_dsi_7inch) = { > + .panel_id = "/soc/dsi@7e700000/panel@0", > + .compatible = "raspberrypi,dsi-7inch", [Severity: Medium] Does this code explicitly configure the 7-inch panel? As mentioned in the commit message comment, this directly contradicts the claim of adding the 5-inch panel. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
