On 17/05/2025 14:23, Beleswar Prasad Padhi wrote:

On 5/16/2025 9:15 PM, Mathieu Poirier wrote:
On Tue, May 13, 2025 at 11:14:38AM +0530, Beleswar Padhi wrote:
The rproc_detach() function invokes __rproc_detach() before
rproc_unprepare_device(). The __rproc_detach() function sets the
rproc->state to "RPROC_DETACHED".

However, the TI K3 M4 driver erroneously looks for "RPROC_ATTACHED"
state in its .unprepare ops to identify IPC-only mode; which leads to
resetting the rproc in detach routine.

Therefore, correct the IPC-only mode detection logic to look for
"RPROC_DETACHED" in k3_m4_rproc_unprepare() function.

This driver has been upstream for 9 whole months, it is hard for me to believe this but was just noticed.  Martyn from Collabora should be CC'ed on this, and I
will also need the required R-b/T-b tags.


Cc: Martyn Welch martyn.we...@collabora.com

Requesting Andrew/Judith for review and test too.


It's been a while since I used this, IIRC the project I thought we needed this for went in a different direction almost as soon as I'd managed to get the driver upstream... ...But I've spent some time to review as best as I can:

Reviewed-by: Martyn Welch <martyn.we...@collabora.com>


Typically bug fixes are not part of refactoring exercises.


Typically, yes. But the refactor depends on this fix. This k3_m4_rproc_unprepare() function is entirely refactored to common driver in [PATCH v12 26/36].

So, If the refactor is picked without this patch fix, the mainline M4 driver would be fixed, but the older stable kernels would always have this bug. Let me know what you think.

Thanks,
Beleswar

  I suggest to apply
this set without this patch - you can then work on fixing this bug.

Thanks,
Mathieu

Fixes: ebcf9008a895 ("remoteproc: k3-m4: Add a remoteproc driver for M4F subsystem")
Signed-off-by: Beleswar Padhi <b-pa...@ti.com>
---
v12: Changelog:
1. New patch. Fixup a state detection logic.

  drivers/remoteproc/ti_k3_m4_remoteproc.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/remoteproc/ti_k3_m4_remoteproc.c b/drivers/ remoteproc/ti_k3_m4_remoteproc.c
index a16fb165fcedd..6cd50b16a8e82 100644
--- a/drivers/remoteproc/ti_k3_m4_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_m4_remoteproc.c
@@ -228,7 +228,7 @@ static int k3_m4_rproc_unprepare(struct rproc *rproc)
      int ret;
      /* If the core is going to be detached do not assert the module reset */
-    if (rproc->state == RPROC_ATTACHED)
+    if (rproc->state == RPROC_DETACHED)
          return 0;
      ret = kproc->ti_sci->ops.dev_ops.put_device(kproc->ti_sci,
--
2.34.1



Reply via email to