Re: [PATCH v12 04/36] remoteproc: k3-m4: Don't assert reset in detach routine

[Beleswar Prasad Padhi]

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.welch@xxxxxxxxxxxxx

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.

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-padhi@xxxxxx>
> > ---
> > 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