Re: [PATCH 1/3] remoteproc: k3-dsp: Suppress duplicate error message in .remove()

[Mathieu Poirier <mathieu.poirier@linaro.org>]

On Wed, Nov 29, 2023 at 11:50:10PM +0100, Uwe Kleine-König wrote:
> Helo Mathieu,
> 
> On Wed, Nov 29, 2023 at 10:35:32AM -0700, Mathieu Poirier wrote:
> > On Thu, Nov 23, 2023 at 10:16:59PM +0100, Uwe Kleine-König wrote:
> > > 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 */
> > 
> > I'm not sure why this comment has been added...
> 
> The comment was added because there is a real problem and I didn't try
> to fix it as doing that without the hardware is hard.
>
 
I've looked at this again and as it turns out, you are correct on both front.  I
will apply your patches as-is and ask people at TI to look at this code again.

Thanks,
Mathieu

> > >  			dev_err(dev, "failed to detach proc, ret = %d\n", ret);
> > 
> > And why this isn't refactored in the next patch.
> 
> the next patch has:
> 
> -                       dev_err(dev, "failed to detach proc, ret = %d\n", ret);
> +                       dev_err(dev, "failed to detach proc (%pe)\n", ERR_PTR(ret));
> 
> so this is refactored?!
> 
> > > -			return ret;
> > > +			return 0;
> > 
> > Appart from the above I'm good with this patchset.
> 
> Best regards
> Uwe
> 
> -- 
> Pengutronix e.K.                           | Uwe Kleine-König            |
> Industrial Linux Solutions                 | https://www.pengutronix.de/ |