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/ |
