Hello,
On Thu, Nov 30, 2023 at 10:19:42AM -0700, Mathieu Poirier wrote:
> Hari and Nishanth,
>
> On Thu, 23 Nov 2023 at 14:17, Uwe Kleine-König
> <u.kleine-koenig@xxxxxxxxxxxxxx> wrote:
> >
> > When the remove callback returns non-zero, the driver core emits an
> > error message about the error value being ignored. As the driver already
> > emits an error message already, return zero. This has no effect apart
> > from suppressing the core's message. The platform device gets unbound
> > irrespective of the return value.
> >
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@xxxxxxxxxxxxxx>
> > ---
> > drivers/remoteproc/ti_k3_dsp_remoteproc.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
> > index ef8415a7cd54..40a5fd8763fa 100644
> > --- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c
> > +++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
> > @@ -835,8 +835,9 @@ static int k3_dsp_rproc_remove(struct platform_device *pdev)
> > if (rproc->state == RPROC_ATTACHED) {
> > ret = rproc_detach(rproc);
> > if (ret) {
> > + /* Note this error path leaks resources */
> > dev_err(dev, "failed to detach proc, ret = %d\n", ret);
> > - return ret;
> > + return 0;
>
> Please have a look at this error path. As with the scenario where the
> remote processor is controlled by the remoteproc core, nothing can be
> done in .remove() to prevent the driver from going away. As such and
> even if rproc_detach() fails, other resources associated with this
> remote processor should be cleaned-up.
Without having done a deep dive into the driver and the remoteproc core
I think the remoteproc core should provide a function that deregisters
the software representation of a rproc device and returns void.
If you look at rproc_detach, that can even fail if it doesn't get the
mutex.
So I'm convinced there is something to do on the framework level before
removing the ti_k3_dsp_remoteproc driver can be done without leaking
stuff.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | https://www.pengutronix.de/ |
Attachment:
signature.asc
Description: PGP signature
