Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] A wildcard entry in vs_chip_identities shadows subsequent specific entries, making them unreachable. - [High] The IRQ handler is registered before DRM structures are initialized, which can cause a NULL pointer dereference if an interrupt fires early or if initialization fails. -- commit ef807eaf736a3cb84d7613b4d88301061b410a1b Author: Joey Lu <[email protected]> drm/verisilicon: add DCUltraLite chip identity to HWDB This commit registers the Nuvoton MA35D1 DCUltraLite chip identity in the hardware database. It places the entry last in the list to act as a gate for hardware recognition. > diff --git a/drivers/gpu/drm/verisilicon/vs_hwdb.c > b/drivers/gpu/drm/verisilicon/vs_hwdb.c > index 91524d16f7785..7d630a667a3f0 100644 > --- a/drivers/gpu/drm/verisilicon/vs_hwdb.c > +++ b/drivers/gpu/drm/verisilicon/vs_hwdb.c > @@ -129,6 +129,16 @@ static struct vs_chip_identity vs_chip_identities[] = { [Severity: High] This is a pre-existing issue, but does the wildcard entry at the start of vs_chip_identities prevent specific entries from being matched? Looking at vs_chip_identities, the first entry uses a wildcard for the customer ID: static struct vs_chip_identity vs_chip_identities[] = { { .model = 0x8200, .revision = 0x5720, .customer_id = ~0U, ... In vs_fill_chip_identity(), the search loop returns on the first match: for (i = 0; i < ARRAY_SIZE(vs_chip_identities); i++) { if (vs_chip_identities[i].model == model && vs_chip_identities[i].revision == revision && (vs_chip_identities[i].customer_id == customer_id || vs_chip_identities[i].customer_id == ~0U)) { ... Because the ~0U entry comes first, wouldn't it shadow the subsequent specific entries for the same model and revision (like customer_id 0x310 and 0x311), preventing their specific formats from ever being applied? > .max_cursor_size = 64, > .formats = &vs_formats_no_yuv444, > }, > + { > + .model = 0x0, /* DCUltraLite */ > + .revision = 0x5560, > + .customer_id = 0x305, > + > + .generation = VSDC_GEN_DC8000, > + .display_count = 1, > + .max_cursor_size = 32, > + .formats = &vs_formats_no_yuv444, > + }, > }; > > int vs_fill_chip_identity(struct regmap *regs, [Severity: High] This isn't a bug introduced by this patch, but is it possible for an early interrupt to cause a NULL pointer dereference during probe? In vs_dc_probe() from vs_dc.c, the IRQ handler is registered before DRM structures are initialized: ret = devm_request_irq(dev, irq, vs_dc_irq_handler, 0, dev_name(dev), dc); ... ret = vs_drm_initialize(dc, pdev); If an interrupt fires during this window, the handler seems to unconditionally dereference dc->drm_dev which is allocated later in vs_drm_initialize(): vs_drm_handle_irq() in vs_drm.c: ... if (dc->drm_dev->crtcs[i]) drm_crtc_handle_vblank(&dc->drm_dev->crtcs[i]->base); Could this lead to a panic if the hardware has a pending interrupt before DRM is ready? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
