[media] omap3isp: Prevent pipelines that contain a crashed entity from starting
The OMAP3 ISP preview engine will violate the L4 bus protocol if we try
to write some of its internal registers after it failed to stop
properly. This generates an external abort on non-linefetch fault,
triggering a fatal kernel oops.
We can't always prevent preview engine stop failures (they can for
instance be caused by a sensor crash), but we can improve the system
reliability by refusing to start streaming on a pipeline that contains
the preview engine if it failed to stop. The driver will then eventually
reset the ISP (when all applications will have closed their file handles
related to OMAP3 ISP device nodes), making the ISP usable again.
Fixes: NB#291334 - camera: Recover gracefully from ISP crash instead of oopsing
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Sakari Ailus <sakari.ailus@maxwell.research.nokia.com>
Reviewed-by: Phil Carmody <ext-phil.2.carmody@nokia.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>
diff --git a/drivers/media/video/omap3isp/isp.c b/drivers/media/video/omap3isp/isp.c
index 12d5f92..3db8583 100644
--- a/drivers/media/video/omap3isp/isp.c
+++ b/drivers/media/video/omap3isp/isp.c
@@ -739,6 +739,17 @@
unsigned long flags;
int ret;
+ /* If the preview engine crashed it might not respond to read/write
+ * operations on the L4 bus. This would result in a bus fault and a
+ * kernel oops. Refuse to start streaming in that case. This check must
+ * be performed before the loop below to avoid starting entities if the
+ * pipeline won't start anyway (those entities would then likely fail to
+ * stop, making the problem worse).
+ */
+ if ((pipe->entities & isp->crashed) &
+ (1U << isp->isp_prev.subdev.entity.id))
+ return -EIO;
+
spin_lock_irqsave(&pipe->lock, flags);
pipe->state &= ~(ISP_PIPELINE_IDLE_INPUT | ISP_PIPELINE_IDLE_OUTPUT);
spin_unlock_irqrestore(&pipe->lock, flags);
@@ -879,13 +890,15 @@
if (ret) {
dev_info(isp->dev, "Unable to stop %s\n", subdev->name);
+ /* If the entity failed to stopped, assume it has
+ * crashed. Mark it as such, the ISP will be reset when
+ * applications will release it.
+ */
+ isp->crashed |= 1U << subdev->entity.id;
failure = -ETIMEDOUT;
}
}
- if (failure < 0)
- isp->needs_reset = true;
-
return failure;
}
@@ -1069,6 +1082,7 @@
udelay(1);
}
+ isp->crashed = 0;
return 0;
}
@@ -1496,10 +1510,11 @@
if (--isp->ref_count == 0) {
isp_disable_interrupts(isp);
isp_save_ctx(isp);
- if (isp->needs_reset) {
+ /* Reset the ISP if an entity has failed to stop. This is the
+ * only way to recover from such conditions.
+ */
+ if (isp->crashed)
isp_reset(isp);
- isp->needs_reset = false;
- }
isp_disable_clocks(isp);
}
mutex_unlock(&isp->isp_mutex);