Hello Dan,
On 03/05/24 20:54, Dan Carpenter wrote:
Hello Beleswar Padhi,
Commit 3c8a9066d584 ("remoteproc: k3-r5: Do not allow core1 to power
up before core0 via sysfs") from Apr 30, 2024 (linux-next), leads to
the following Smatch static checker warning:
drivers/remoteproc/ti_k3_r5_remoteproc.c:583 k3_r5_rproc_start()
warn: missing unwind goto?
drivers/remoteproc/ti_k3_r5_remoteproc.c:651 k3_r5_rproc_stop()
warn: missing unwind goto?
drivers/remoteproc/ti_k3_r5_remoteproc.c
546 static int k3_r5_rproc_start(struct rproc *rproc)
547 {
548 struct k3_r5_rproc *kproc = rproc->priv;
549 struct k3_r5_cluster *cluster = kproc->cluster;
550 struct device *dev = kproc->dev;
551 struct k3_r5_core *core0, *core;
552 u32 boot_addr;
553 int ret;
554
555 ret = k3_r5_rproc_request_mbox(rproc);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
556 if (ret)
557 return ret;
558
559 boot_addr = rproc->bootaddr;
560 /* TODO: add boot_addr sanity checking */
561 dev_dbg(dev, "booting R5F core using boot addr = 0x%x\n", boot_addr);
562
563 /* boot vector need not be programmed for Core1 in LockStep mode */
564 core = kproc->core;
565 ret = ti_sci_proc_set_config(core->tsp, boot_addr, 0, 0);
566 if (ret)
567 goto put_mbox;
568
569 /* unhalt/run all applicable cores */
570 if (cluster->mode == CLUSTER_MODE_LOCKSTEP) {
571 list_for_each_entry_reverse(core, &cluster->cores, elem) {
572 ret = k3_r5_core_run(core);
573 if (ret)
574 goto unroll_core_run;
575 }
576 } else {
577 /* do not allow core 1 to start before core 0 */
578 core0 = list_first_entry(&cluster->cores, struct k3_r5_core,
579 elem);
580 if (core != core0 && core0->rproc->state == RPROC_OFFLINE) {
581 dev_err(dev, "%s: can not start core 1 before core 0\n",
582 __func__);
--> 583 return -EPERM;
Is there no clean up on this error path? I think we need to do a
goto put_mbox at least.
Thank you for pointing out. Apologies for the oversight. I have sent a
PATCH addressing this bug.
Link to PATCH:
https://lore.kernel.org/all/20240506141849.1735679-1-b-padhi@xxxxxx/
Regards,
Beleswar
584 }
585
586 ret = k3_r5_core_run(core);
587 if (ret)
588 goto put_mbox;
589 }
590
591 return 0;
592
593 unroll_core_run:
594 list_for_each_entry_continue(core, &cluster->cores, elem) {
595 if (k3_r5_core_halt(core))
596 dev_warn(core->dev, "core halt back failed\n");
597 }
598 put_mbox:
599 mbox_free_channel(kproc->mbox);
600 return ret;
601 }
regards,
dan carpenter
