Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Possible NULL pointer dereference in the IRQ handler if an interrupt triggers before DRM initialization completes. -- commit 2cd6077ab2fb5ed065ee2cef55004b713c80f827 Author: Joey Lu <[email protected]> drm/verisilicon: make axi and ahb clocks optional The Nuvoton MA35D1 SoC integrates a DCUltraLite display controller whose AXI and AHB bus clocks share a single gate enable bit with the display core clock, so the clock driver does not expose them separately. This patch makes the axi and ahb clocks optional in the probe. > diff --git a/drivers/gpu/drm/verisilicon/vs_dc.c > b/drivers/gpu/drm/verisilicon/vs_dc.c > index 9729b693d360e..fd1f5fe67a68b 100644 > --- a/drivers/gpu/drm/verisilicon/vs_dc.c > +++ b/drivers/gpu/drm/verisilicon/vs_dc.c [ ... ] > @@ -90,13 +90,13 @@ static int vs_dc_probe(struct platform_device *pdev) > return PTR_ERR(dc->core_clk); > } > > - dc->axi_clk = devm_clk_get_enabled(dev, "axi"); > + dc->axi_clk = devm_clk_get_optional_enabled(dev, "axi"); > if (IS_ERR(dc->axi_clk)) { [Severity: High] This is a pre-existing issue, but is it possible for an interrupt to trigger a NULL pointer dereference during probe if the bootloader left the display active? Looking further down in vs_dc_probe(), the IRQ handler is registered before vs_drm_initialize() is called: ret = devm_request_irq(dev, irq, vs_dc_irq_handler, 0, dev_name(dev), dc); if (ret) { dev_err(dev, "can't request irq\n"); goto err_rst_assert; } dev_set_drvdata(dev, dc); ret = vs_drm_initialize(dc, pdev); If a pending interrupt (such as VSYNC) fires immediately after devm_request_irq() is called, vs_dc_irq_handler() will be triggered and will eventually call vs_drm_handle_irq(). Looking at vs_drm_handle_irq() in vs_drm.c, it unconditionally dereferences the drm_dev pointer: if (dc->drm_dev->crtcs[i]) drm_crtc_handle_vblank(&dc->drm_dev->crtcs[i]->base); Since vs_drm_initialize() hasn't allocated or assigned dc->drm_dev yet, could this cause a kernel panic during boot on platforms where the bootloader leaves interrupts enabled? Would it be safer to request the IRQ after the DRM device is fully initialized, or to mask interrupts in the hardware before registering the handler? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=4
