From 870565f063a58576e8a4529f122cac4325c6b395 Mon Sep 17 00:00:00 2001 From: Tomi Valkeinen Date: Thu, 7 Dec 2023 08:57:48 +0100 Subject: media: rkisp1: Fix IRQ disable race issue In rkisp1_isp_stop() and rkisp1_csi_disable() the driver masks the interrupts and then apparently assumes that the interrupt handler won't be running, and proceeds in the stop procedure. This is not the case, as the interrupt handler can already be running, which would lead to the ISP being disabled while the interrupt handler handling a captured frame. This brings up two issues: 1) the ISP could be powered off while the interrupt handler is still running and accessing registers, leading to board lockup, and 2) the interrupt handler code and the code that disables the streaming might do things that conflict. It is not clear to me if 2) causes a real issue, but 1) can be seen with a suitable delay (or printk in my case) in the interrupt handler, leading to board lockup. Link: https://lore.kernel.org/r/20231207-rkisp-irq-fix-v3-4-358a2c871a3c@ideasonboard.com Tested-by: Adam Ford #imx8mp-beacon Signed-off-by: Tomi Valkeinen Signed-off-by: Laurent Pinchart Signed-off-by: Mauro Carvalho Chehab --- drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c | 14 +++++++++++++- drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c | 20 +++++++++++++++++--- 2 files changed, 30 insertions(+), 4 deletions(-) (limited to 'drivers/media') diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c index e6642a2167ef..b6e47e2f1b94 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-csi.c @@ -125,8 +125,20 @@ static void rkisp1_csi_disable(struct rkisp1_csi *csi) struct rkisp1_device *rkisp1 = csi->rkisp1; u32 val; - /* Mask and clear interrupts. */ + /* Mask MIPI interrupts. */ rkisp1_write(rkisp1, RKISP1_CIF_MIPI_IMSC, 0); + + /* Flush posted writes */ + rkisp1_read(rkisp1, RKISP1_CIF_MIPI_IMSC); + + /* + * Wait until the IRQ handler has ended. The IRQ handler may get called + * even after this, but it will return immediately as the MIPI + * interrupts have been masked. + */ + synchronize_irq(rkisp1->irqs[RKISP1_IRQ_MIPI]); + + /* Clear MIPI interrupt status */ rkisp1_write(rkisp1, RKISP1_CIF_MIPI_ICR, ~0); val = rkisp1_read(rkisp1, RKISP1_CIF_MIPI_CTRL); diff --git a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c index 6f8bd9a6fa6b..da313f87e248 100644 --- a/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c +++ b/drivers/media/platform/rockchip/rkisp1/rkisp1-isp.c @@ -254,11 +254,25 @@ static void rkisp1_isp_stop(struct rkisp1_isp *isp) * ISP(mi) stop in mi frame end -> Stop ISP(mipi) -> * Stop ISP(isp) ->wait for ISP isp off */ - /* stop and clear MI and ISP interrupts */ - rkisp1_write(rkisp1, RKISP1_CIF_ISP_IMSC, 0); - rkisp1_write(rkisp1, RKISP1_CIF_ISP_ICR, ~0); + /* Mask MI and ISP interrupts */ + rkisp1_write(rkisp1, RKISP1_CIF_ISP_IMSC, 0); rkisp1_write(rkisp1, RKISP1_CIF_MI_IMSC, 0); + + /* Flush posted writes */ + rkisp1_read(rkisp1, RKISP1_CIF_MI_IMSC); + + /* + * Wait until the IRQ handler has ended. The IRQ handler may get called + * even after this, but it will return immediately as the MI and ISP + * interrupts have been masked. + */ + synchronize_irq(rkisp1->irqs[RKISP1_IRQ_ISP]); + if (rkisp1->irqs[RKISP1_IRQ_ISP] != rkisp1->irqs[RKISP1_IRQ_MI]) + synchronize_irq(rkisp1->irqs[RKISP1_IRQ_MI]); + + /* Clear MI and ISP interrupt status */ + rkisp1_write(rkisp1, RKISP1_CIF_ISP_ICR, ~0); rkisp1_write(rkisp1, RKISP1_CIF_MI_ICR, ~0); /* stop ISP */ -- cgit v1.2.3