On 5/20/25 00:06, Beleswar Prasad Padhi wrote:
Hi Mathieu,

On 19/05/25 20:07, Mathieu Poirier wrote:
On Sat, May 17, 2025 at 06:53:29PM +0530, 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.

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.

I suggest you send this patch on its own and then the series (without this
patch) with a note in the cover letter that it depends on the fix.  That way we
get the best of both worlds.


Sure. If I get any comments/reviews on this version, I will re-spin this patch 
separately than the series.

Thanks,
Beleswar

Reviewed-by: Hari Nagalla <hnaga...@ti.com>

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