Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues: - [High] Data race on `dsi->irq_data` between process context and hardirq context can cause false positive interrupt completions. - [High] Unbounded hardware polling loop in hardirq handler can lock up the CPU. -- --- Patch [5]: [PATCH v6 05/11] drm/mediatek: mtk_dsi: Enable interrupt at component bind time --- commit 869a346c570d4022e4d54a10b57fcff068a735b6 Author: AngeloGioacchino Del Regno <[email protected]> drm/mediatek: mtk_dsi: Enable interrupt at component bind time Having the DSI interrupt enabled before actually binding the DSI component to the display controller driver is both useless and dangerous. This commit fixes this by requesting the interrupt with IRQF_NO_AUTOEN, and enabling it only when binding DSI to its display controller component. > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c > b/drivers/gpu/drm/mediatek/mtk_dsi.c > index e4a156a474e08..070d7d555bdbe 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c > @@ -225,6 +225,7 @@ struct mtk_dsi { > int refcount; > bool enabled; > bool lanes_ready; > + int irq; > u32 irq_data; [Severity: High] This isn't a bug introduced by this patch, but can the data race on dsi->irq_data between process context and hardirq context cause false positive interrupt completions? The dsi->irq_data variable is modified locklessly using read-modify-write operations: drivers/gpu/drm/mediatek/mtk_dsi.c:mtk_dsi_irq_data_clear() { ... dsi->irq_data &= ~irq_bit; ... } drivers/gpu/drm/mediatek/mtk_dsi.c:mtk_dsi_irq_data_set() { ... dsi->irq_data |= irq_bit; ... } If a delayed interrupt sets a flag while a process on another CPU is clearing a different flag for a new command, the process's clear operation can be lost if the interrupt handler overwrites it. The previously cleared flag remains set, causing mtk_dsi_wait_for_irq_done() to falsely return success. > wait_queue_head_t irq_wait_queue; > const struct mtk_dsi_driver_data *driver_data; > }; [ ... ] > @@ -1381,17 +1385,15 @@ static int mtk_dsi_probe(struct platform_device *pdev) > > platform_set_drvdata(pdev, dsi); > > + ret = devm_request_irq(&pdev->dev, dsi->irq, mtk_dsi_irq, > + IRQF_NO_AUTOEN, dev_name(&pdev->dev), dsi); [Severity: High] This isn't a bug introduced by this patch, but could the unbounded hardware polling loop in the mtk_dsi_irq() hardirq handler lock up the CPU? The mtk_dsi_irq() function runs in hardirq context and uses an unbounded loop to wait for the hardware: drivers/gpu/drm/mediatek/mtk_dsi.c:mtk_dsi_irq() { ... do { mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK); tmp = readl(dsi->regs + DSI_INTSTA); } while (tmp & DSI_BUSY); ... } If the DSI hardware hangs, loses its clock, or DSI_BUSY never clears for any other reason, the loop never terminates, which will permanently hang the CPU. > + if (ret) > + return dev_err_probe(&pdev->dev, ret, "Failed to request DSI > irq\n"); > + -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=5
