From mboxrd@z Thu Jan 1 00:00:00 1970 From: Claude Code Review Bot To: dri-devel-reviews@example.com Subject: Claude review: drm/mediatek: dsi: Store driver data before invoking mipi_dsi_host_register Date: Fri, 27 Feb 2026 13:42:15 +1000 Message-ID: In-Reply-To: <20260225094047.76780-1-l.scorcia@gmail.com> References: <20260225094047.76780-1-l.scorcia@gmail.com> <20260225094047.76780-1-l.scorcia@gmail.com> X-Mailer: Claude Code Patch Reviewer Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Patch Review **Correctness: Good** The commit message clearly explains the bug with a full crash trace showing= the call chain from `mipi_dsi_host_register` =E2=86=92 `mtk_dsi_host_attac= h` =E2=86=92 `component_add` =E2=86=92 `mtk_dsi_bind` =E2=86=92 `dev_get_dr= vdata` returning NULL. The fix moves `platform_set_drvdata(pdev, dsi)` befo= re the `mipi_dsi_host_register()` call, which is the right thing to do. **init_waitqueue_head move is also correct (v3 improvement):** The `init_waitqueue_head(&dsi->irq_wait_queue)` is also moved earlier, befo= re `platform_set_drvdata`. This is important because after `mipi_dsi_host_r= egister` completes and `mtk_dsi_bind` runs successfully, the DSI subsystem = could theoretically start operations that use the wait queue. More concrete= ly, the IRQ handler `mtk_dsi_irq()` calls `wake_up_interruptible(&dsi->irq_= wait_queue)` =E2=80=94 so the wait queue must be initialized before `devm_r= equest_irq()`, which is called right after `mipi_dsi_host_register()`. This= was already the case in the original code (init was after request_irq =E2= =80=94 actually a latent bug!), and this patch fixes that ordering too. **Minor observation =E2=80=94 latent pre-existing bug also fixed:** Looking at the original code in the drm-next tree: ```c ret =3D devm_request_irq(&pdev->dev, irq_num, mtk_dsi_irq, ...); // line 1= 243 ... init_waitqueue_head(&dsi->irq_wait_queue); // line 1250 ``` The original code initializes the wait queue *after* registering the IRQ ha= ndler that calls `wake_up_interruptible()` on it. This is a pre-existing ra= ce condition. This patch incidentally fixes it by moving `init_waitqueue_he= ad` before both `mipi_dsi_host_register` and `devm_request_irq`. The commit= message (v3 note) mentions this was intentional, which is good. **No error-path concerns:** `platform_set_drvdata` and `init_waitqueue_head= ` don't need cleanup on failure, so moving them earlier doesn't create any = resource leak issues on the error paths. **Fixes tag:** Correctly references commit `e4732b590a77` which moved `mipi= _dsi_host_register` earlier in the probe function without also moving `plat= form_set_drvdata` ahead of it. **Verdict:** The patch is correct and well-motivated. No issues found. Reviewed-by tags present: AngeloGioacchino Del Regno (Collabora) =E2=80=94 = appropriate reviewer for MediaTek DRM. --- Generated by Claude Code Patch Reviewer