Revert "remoteproc: Modify recovery path to use rproc_{start,stop}()"

This reverts commit 7e83cab824a86704cdbd7735c19d34e0ce423dc5.

The commit 7e83cab824a8 ("remoteproc: Modify recovery path to use
rproc_{start,stop}()") has modified the rproc_trigger_recovery()
function to leverage rproc_stop() and rproc_start() functions
introduced in commit 1efa30d0895e ("remoteproc: Introduce
rproc_{start,stop}() functions") in place of the previous usage
of rproc_shutdown() and rproc_boot() functions. These new functions
execute only a portion of the previous functions, and are primarily
designed to optimize the allocation of memory resources. They do
not handle other resources like virtio devices and the resetting
of any IOMMUs. The new functions also do not re-initialize the
memory used by the firmware image sections during recovery. This
results in a kernel crash like below when performing error recovery
on remoteprocs using both MMUs and virtio devices like the OMAP
remoteproc driver.

  omap-iommu 55082000.mmu: 55082000.mmu: version 2.1
  omap-iommu 55082000.mmu: iommu fault: da 0x8004cc60 flags 0x0
  remoteproc remoteproc1: crash detected in 55020000.ipu: type mmufault
  omap-iommu 55082000.mmu: 55082000.mmu: errs:0x00000002 da:0x8004cc60 pgd:0x63ed70cb *pgd:px95f00002
  remoteproc remoteproc1: handling crash #1 in 55020000.ipu
  remoteproc remoteproc1: recovering 55020000.ipu
  omap-rproc 55020000.ipu: omap rproc 55020000.ipu crashed
  remoteproc remoteproc1: stopped remote processor 55020000.ipu
  kobject (44f48dcd): tried to init an initialized object, something is seriously wrong.
  CPU: 0 PID: 23 Comm: kworker/0:1 Not tainted 4.19.0-00420-gd9f97bb86434 #658
  Hardware name: Generic DRA74X (Flattened Device Tree)
  Workqueue: events rproc_crash_handler_work [remoteproc]
  [<c0114224>] (unwind_backtrace) from [<c010e618>] (show_stack+0x10/0x14)
  [<c010e618>] (show_stack) from [<c09585c8>] (dump_stack+0xb0/0xe8)
  [<c09585c8>] (dump_stack) from [<c095cec0>] (kobject_init+0x78/0x94)
  [<c095cec0>] (kobject_init) from [<c06224bc>] (device_initialize+0x20/0xf0)
  [<c06224bc>] (device_initialize) from [<bf0003d4>] (register_virtio_device+0x1c/0x110 [virtio])
  [<bf0003d4>] (register_virtio_device [virtio]) from [<bf02bf34>] (rproc_add_virtio_dev+0x44/0xa0 [remoteproc])
  [<bf02bf34>] (rproc_add_virtio_dev [remoteproc]) from [<bf028744>] (rproc_start+0x1c8/0x264 [remoteproc])
  [<bf028744>] (rproc_start [remoteproc]) from [<bf02acac>] (rproc_trigger_recovery+0x28c/0x2dc [remoteproc])
  [<bf02acac>] (rproc_trigger_recovery [remoteproc]) from [<c015d438>] (process_one_work+0x2ac/0x788)
  [<c015d438>] (process_one_work) from [<c015d940>] (worker_thread+0x2c/0x584)
  [<c015d940>] (worker_thread) from [<c0164648>] (kthread+0x140/0x15c)
  [<c0164648>] (kthread) from [<c01010b4>] (ret_from_fork+0x14/0x20)
  ... exception stack dump ...
  virtio_rpmsg_bus virtio1: rpmsg host is online
  driver_bound: device virtio1 already bound
  remoteproc remoteproc1: registered virtio1 (type 7)
  remoteproc remoteproc1: remote processor 55020000.ipu is now up
  Unable to handle kernel NULL pointer dereference at virtual address 00000004

Revert the above commit to fix/restore the error recovery functionality.
This patch is a custom revert of the above commit, accounting for some
additional changes done in commit 2666ca9197e3 ("remoteproc: Add remote
processor coredump support"). The coredump function is invoked in
rproc_stop() now.

Signed-off-by: Suman Anna <s-anna@ti.com>

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index 76fd36413c565e53c52dfb55f776793c67fe5f61..00a6e855784cd7e276646320f7f332d0f54fd113 100644 (file)
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -1206,9 +1206,12 @@ static int rproc_stop(struct rproc *rproc, bool crashed)
 
        rproc_unprepare_subdevices(rproc);
 
-       /* if in crash state, unlock crash handler */
-       if (rproc->state == RPROC_CRASHED)
+       /* if in crash state, perform coredump and unlock crash handler */
+       if (rproc->state == RPROC_CRASHED) {
+               rproc_coredump(rproc);
+
                complete_all(&rproc->crash_comp);
+       }
 
        rproc->state = RPROC_OFFLINE;
 
@@ -1337,43 +1340,23 @@ static void rproc_coredump(struct rproc *rproc)
  */
 int rproc_trigger_recovery(struct rproc *rproc)
 {
-       const struct firmware *firmware_p;
-       struct device *dev = &rproc->dev;
-       int ret;
-
-       dev_err(dev, "recovering %s\n", rproc->name);
+       dev_err(&rproc->dev, "recovering %s\n", rproc->name);
 
        init_completion(&rproc->crash_comp);
 
-       ret = mutex_lock_interruptible(&rproc->lock);
-       if (ret)
-               return ret;
-
-       ret = rproc_stop(rproc, true);
-       if (ret)
-               goto unlock_mutex;
-
-       /* generate coredump */
-       rproc_coredump(rproc);
+       /* shut down the remote */
+       /* TODO: make sure this works with rproc->power > 1 */
+       rproc_shutdown(rproc);
 
        /* wait until there is no more rproc users */
        wait_for_completion(&rproc->crash_comp);
 
-       /* load firmware */
-       ret = request_firmware(&firmware_p, rproc->firmware, dev);
-       if (ret < 0) {
-               dev_err(dev, "request_firmware failed: %d\n", ret);
-               goto unlock_mutex;
-       }
-
-       /* boot the remote processor up again */
-       ret = rproc_start(rproc, firmware_p);
-
-       release_firmware(firmware_p);
+       /*
+        * boot the remote processor up again
+        */
+       rproc_boot(rproc);
 
-unlock_mutex:
-       mutex_unlock(&rproc->lock);
-       return ret;
+       return 0;
 }
 
 /**