From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id A16323E2AA1 for ; Mon, 13 Jul 2026 10:11:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783937512; cv=none; b=Y1y4eQBArvftVc/fc+TIik2cYrd/AeyAmFkmCpeE3KBQMb/uW+ua0BuaSmLL07/8Wvu9Oc0arheoI5f/9KgPm5ggh8WH46DD4yDPxEUQqNk1F0eFNi8F8uM4by1srRp4cwVKdJh6AuUWfS4JZd+/aWa+TldkHaZ902j1+R8NxcA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1783937512; c=relaxed/simple; bh=nCczL1KgHWC55OYX1r4EXXcAlfHRK73cNlh78u6kx+8=; h=From:Subject:To:Cc:In-Reply-To:References:Content-Type:Date: Message-Id; b=c80B3HdzQhovZPBCvTgo/zFzaKO/2oqoX+AzIOwQjfPiGE31lk2p/2ude3IF9kRiu1fl4/n2SE9RE3PfEV5Hv5UiD0E4SL3kCsgS3M45MP9QQ9shwMFzLkV/F6mmN4nB6RiKAkPai7RkdOPVzAvzyeg/pXsRn7E5t850WZITz9A= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=F10EcXTg; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="F10EcXTg" Received: by smtp.kernel.org (Postfix) with ESMTPSA id B131A1F00A3A; Mon, 13 Jul 2026 10:11:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1783937511; bh=z0cA5q51bXw9VlmvO3E3YfQZz9L7jLWesCBtAsfOthM=; h=From:Subject:Reply-To:To:Cc:In-Reply-To:References:Date; b=F10EcXTgFQZUSUGcK6gmGIhKFzxbhrFJeeh7NxStMlnu/z/+ZCi0MOeWbi7OUuNCb GDMNW/lzljoSXKjelkUNmuZWib+IKruHKYbfsTdhLh9882B2cYJaupgk2k2SDjzyud KuvpfnfCYMFdp0GpJDHS2R+MN7nUzlFF4IpxIfS2T0S5McXUoV7qf3lxf3pW1rAF2s +zvxugIOG6T7bBM3qYNWKvDhmVH0sgWK1uXE8oEMJXCQmAKvYipgShzLShVM7GZmFE x+9H9cTUCbS+t2uZUmvX28TistVgMav/yDEJS01rzh+eBUhfPG547++1XlROOCnD0I aHIUcRKuH3xvA== From: sashiko-bot@kernel.org Subject: Re: [PATCH v2 04/10] drm/mediatek: mtk_dsi: Enable interrupt at component bind time Reply-To: sashiko-reviews@lists.linux.dev To: "AngeloGioacchino Del Regno" Cc: conor+dt@kernel.org, devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org, robh@kernel.org In-Reply-To: <20260713095837.16590-5-angelogioacchino.delregno@collabora.com> References: <20260713095837.16590-1-angelogioacchino.delregno@collabora.com> <20260713095837.16590-5-angelogioacchino.delregno@collabora.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Date: Mon, 13 Jul 2026 10:11:50 +0000 Message-Id: <20260713101150.B131A1F00A3A@smtp.kernel.org> Precedence: bulk X-Mailing-List: devicetree@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Thank you for your contribution! Sashiko AI review found 4 potential issue(= s) to consider: New issues: - [High] Calling `enable_irq()` in `mtk_dsi_bind()` executes before the IRQ= is requested, leaving the interrupt permanently disabled. Pre-existing issues: - [High] Data race on `dsi->irq_data` between process and hardirq context l= eads to dropped interrupts. - [High] `mtk_dsi_host_send_cmd()` incorrectly interprets `-ERESTARTSYS` as= success, causing the driver to process stale hardware memory. - [High] Unbounded busy-wait loop in `mtk_dsi_irq()` hard-locks the CPU if = hardware stalls. -- --- Patch [4]: [PATCH v2 04/10] drm/mediatek: mtk_dsi: Enable interrupt at = component bind time --- commit ab0ed7e073fb408fbb00443f437ebcc970005f69 Author: AngeloGioacchino Del Regno drm/mediatek: mtk_dsi: Enable interrupt at component bind time This patch changes the DSI interrupt to be requested with IRQF_NO_AUTOEN and enables it later during the component bind phase. This aims to prevent interrupt storms and artifacts caused by interrupts arriving before the display controller is fully ready. > diff --git a/drivers/gpu/drm/mediatek/mtk_dsi.c b/drivers/gpu/drm/mediate= k/mtk_dsi.c > index 3e22553ca990f..284dae372c7c2 100644 > --- a/drivers/gpu/drm/mediatek/mtk_dsi.c > +++ b/drivers/gpu/drm/mediatek/mtk_dsi.c [ ... ] > @@ -1052,7 +1053,13 @@ static int mtk_dsi_bind(struct device *dev, struct= device *master, void *data) > if (ret) > return ret; > =20 > - return device_reset_optional(dev); > + ret =3D device_reset_optional(dev); > + if (ret) > + return ret; > + > + enable_irq(dsi->irq); > + > + return 0; > } > =20 > static void mtk_dsi_unbind(struct device *dev, struct device *master, > @@ -1060,6 +1067,8 @@ static void mtk_dsi_unbind(struct device *dev, stru= ct device *master, > { > struct mtk_dsi *dsi =3D dev_get_drvdata(dev); > =20 > + disable_irq(dsi->irq); > + > drm_encoder_cleanup(&dsi->encoder); > } [ ... ] > @@ -1338,8 +1346,8 @@ static int mtk_dsi_probe(struct platform_device *pd= ev) > if (ret < 0) > return dev_err_probe(dev, ret, "Failed to register DSI host\n"); > =20 > - ret =3D devm_request_irq(&pdev->dev, irq_num, mtk_dsi_irq, > - IRQF_TRIGGER_NONE, dev_name(&pdev->dev), dsi); > + ret =3D devm_request_irq(&pdev->dev, dsi->irq, mtk_dsi_irq, > + IRQF_NO_AUTOEN, dev_name(&pdev->dev), dsi); [Severity: High] Does mipi_dsi_host_register() trigger component binding synchronously before the IRQ is actually requested here? If mtk_dsi_probe() invokes mipi_dsi_host_register() before devm_request_irq= (): 1. mipi_dsi_host_register() synchronously probes child panel devices 2. The child devices invoke component_add() for the DSI component 3. If the aggregate device is ready, this synchronously triggers mtk_dsi_bi= nd() 4. mtk_dsi_bind() then calls enable_irq(dsi->irq) Since devm_request_irq() hasn't run yet, will enable_irq() encounter an unrequested IRQ descriptor and abort without unmasking the hardware? When devm_request_irq() subsequently runs with IRQF_NO_AUTOEN, it seems the IRQ might remain disabled forever, completely breaking display operation. [Severity: High] This is a pre-existing issue, but can this code drop interrupts due to a da= ta race on dsi->irq_data? Process context uses a load-modify-store to clear flags: mtk_dsi_irq_data_clear() dsi->irq_data &=3D ~irq_bit; While hardirq context uses a non-atomic operation to set flags: mtk_dsi_irq_data_set() dsi->irq_data |=3D irq_bit; If an interrupt fires between the load and store in process context, the interrupt handler will set its status flag, but the process context's store will overwrite dsi->irq_data with its stale value, destroying the newly set flag and potentially causing wait_event polling timeouts. [Severity: High] This is a pre-existing issue, but does mtk_dsi_host_send_cmd() incorrectly interpret -ERESTARTSYS as success? mtk_dsi_wait_for_irq_done() returns -ERESTARTSYS if interrupted by a signal: mtk_dsi_wait_for_irq_done() ret =3D wait_event_interruptible_timeout(...); However, mtk_dsi_host_send_cmd() evaluates the return value as a boolean: mtk_dsi_host_send_cmd() if (!mtk_dsi_wait_for_irq_done(dsi, flag, 2000)) return -ETIME; else return 0; Since -ERESTARTSYS is non-zero, its negation evaluates to false, causing execution to bypass the error path and return 0. The caller then assumes the hardware operation completed successfully and might proceed to read stale memory from the hardware RX registers. [Severity: High] This is a pre-existing issue, but can the busy-wait loop in mtk_dsi_irq() cause a hard lockup? The mtk_dsi_irq() interrupt handler runs in hardirq context and contains a polling loop without a timeout: mtk_dsi_irq() do { mtk_dsi_mask(dsi, DSI_RACK, RACK, RACK); tmp =3D readl(dsi->regs + DSI_INTSTA); } while (tmp & DSI_BUSY); If the DSI hardware encounters an error, loses its clock, or gets stuck in a busy state, the DSI_BUSY bit will never clear and the CPU will spin indefinitely. --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260713095837.1659= 0-1-angelogioacchino.delregno@collabora.com?part=3D4