media: atomisp: Remove watchdog timer

The watchdog timer code to recover from the ISP getting stuck has several
major issues:

1. There is no way to do fault injection and normally the ISP does not
get stuck, so is it is impossible to test it.

2. It in essence just stops all streams, resets the ISP and then brings
everything back up. Userspace can easily do this itself by using a
timeout on dqbuf and then closing (which causes a poweroff) +
re-opening the device. Doing this in userspace (if it ever turns out
to be necessary) greatly simplifies the kernel code and in general
will be a more robust solution.

Even just a quick look at the code finds several more issues:

3. The need to sync-cancel the timers + work on streamoff requires
isp->mutex to be dropped halfway during the ioctl opening all sorts of
races.

4. The atomisp code supports setting up 2 pipelines, streaming from
two sensors at the same time. But there is only a single wdt_work
and stopping one of the 2 streams will cancel the timers + work,
stopping the wdt even though the other stream might still be running.

5. In case atomisp_css_flush() the sync cancel is done while keeping
isp->mutex locked, causing a deadlock when racing with wdt_work which
also takes isp->mutex.

6. Even though the watchdog is purely a software/driver thing which
just checkes that new frames keep coming in, there are 2 completely
different implementations for the ISP2400/ISP2401 which is not
necessary at all.

So all in all I believe that it is better to just remove the current
watchdog implementation. Fixing all the issues with the current
implementation will be so much work, that if it turns out that we do
need something like this then doing a clean re-implementation from
scratch will be better anyways.

wdt_work was also (ab)used to reset the ISP after the firmware signalled
an fw-assert error through the irq, add a new assert_recover_work to
replace this.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@kernel.org>

1 files changed, 0 insertions, 14 deletions

diff --git a/drivers/staging/media/atomisp/pci/atomisp_subdev.h b/drivers/staging/media/atomisp/pci/atomisp_subdev.h
index b44f060b0bb5..43e6a1d1e410 100644
--- a/drivers/staging/media/atomisp/pci/atomisp_subdev.h
+++ b/drivers/staging/media/atomisp/pci/atomisp_subdev.h
@@ -108,15 +108,6 @@ struct atomisp_video_pipe {
 	 */
 	unsigned int frame_request_config_id[VIDEO_MAX_FRAME];
 	struct atomisp_css_params_with_list *frame_params[VIDEO_MAX_FRAME];
-
-	/*
-	* move wdt from asd struct to create wdt for each pipe
-	*/
-	/* ISP2401 */
-	struct timer_list wdt;
-	unsigned int wdt_duration;	/* in jiffies */
-	unsigned long wdt_expires;
-	atomic_t wdt_count;
 };
 
 struct atomisp_pad_format {
@@ -360,11 +351,6 @@ struct atomisp_sub_device {
 	int raw_buffer_locked_count;
 	spinlock_t raw_buffer_bitmap_lock;
 
-	/* ISP 2400 */
-	struct timer_list wdt;
-	unsigned int wdt_duration;	/* in jiffies */
-	unsigned long wdt_expires;
-
 	/* ISP2401 */
 	bool re_trigger_capture;