[PATCH 0/3] remoteproc: k3-dsp: Some cleanups

[PATCH 0/3] remoteproc: k3-dsp: Some cleanups

[Uwe Kleine-König]

Hello,

my initial plan was to just convert the k3-dsp remoteproc driver to
.remove_new() (i.e. the third patch). The other two patches are
improvements I noticed while working on that driver.

Best regards
Uwe

Uwe Kleine-König (3):
  remoteproc: k3-dsp: Suppress duplicate error message in .remove()
  remoteproc: k3-dsp: Use symbolic error codes in error messages
  remoteproc: k3-dsp: Convert to platform remove callback returning void

 drivers/remoteproc/ti_k3_dsp_remoteproc.c | 87 ++++++++++-------------
 1 file changed, 38 insertions(+), 49 deletions(-)

base-commit: 4e87148f80d198ba5febcbcc969c6b9471099a09
-- 
2.42.0

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

[Uwe Kleine-König]

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@pengutronix.de>
---
 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;
 		}
 	}
 
-- 
2.42.0

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

[Mathieu Poirier]

Hi Uwe,

On Thu, Nov 23, 2023 at 10:16:59PM +0100, Uwe Kleine-König 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@pengutronix.de>
> ---
>  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 */

I'm not sure why this comment has been added...

>  			dev_err(dev, "failed to detach proc, ret = %d\n", ret);

And why this isn't refactored in the next patch.

> -			return ret;
> +			return 0;

Appart from the above I'm good with this patchset.

Thanks,
Mathieu

>  		}
>  	}
>  
> -- 
> 2.42.0
> 

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 1450 bytes --]

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.

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Mathieu Poirier]

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

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

[Mathieu Poirier]

Hari and Nishanth,

On Thu, 23 Nov 2023 at 14:17, Uwe Kleine-König
<u.kleine-koenig@pengutronix.de> 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@pengutronix.de>
> ---
>  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.

Thanks,
Mathieu

>                 }
>         }
>
> --
> 2.42.0
>

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

[Uwe Kleine-König]

[-- Attachment #1: Type: text/plain, Size: 2409 bytes --]

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@pengutronix.de> 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@pengutronix.de>
> > ---
> >  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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

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

[Hari Nagalla]

Hi Mathieu, Uwe,


On 11/30/23 11:19, Mathieu Poirier wrote:
>> --- 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 */the
>>                          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.

Since, anyway the driver goes away we probably need a force cleanup of 
the resources even if 'rproc_detach' fails. Looking a bit into the 
remote core driver, the detach can fail if it fails to get mutex lock or 
unable to reset resource table. It appears to me failure of reset 
resource table is remote
IMO, we can throw an error message when 'rproc_detach' fails in 
'dsp_rproc_remove' and proceed with the rest of the resource clean 
up,i.e call 'rproc_del()', followed by calls to ti device manager to 
relinquish the remote dsp processor.

Best,
Hari Nagalla

[PATCH 2/3] remoteproc: k3-dsp: Use symbolic error codes in error messages

[Uwe Kleine-König]

The error message

	failed to send mailbox message (-EINVAL)

is (for a human) more useful than

	failed to send mailbox message, status = -22

Adapt all error messages to use the symbolic names instead of the
numeric constants. The error paths in .probe() make use of
dev_err_probe() which automatically handles EPROBE_DEFER.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/remoteproc/ti_k3_dsp_remoteproc.c | 78 ++++++++++-------------
 1 file changed, 34 insertions(+), 44 deletions(-)

diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
index 40a5fd8763fa..f048ec1bb00f 100644
--- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
@@ -158,8 +158,8 @@ static void k3_dsp_rproc_kick(struct rproc *rproc, int vqid)
 	/* send the index of the triggered virtqueue in the mailbox payload */
 	ret = mbox_send_message(kproc->mbox, (void *)msg);
 	if (ret < 0)
-		dev_err(dev, "failed to send mailbox message, status = %d\n",
-			ret);
+		dev_err(dev, "failed to send mailbox message (%pe)\n",
+			ERR_PTR(ret));
 }
 
 /* Put the DSP processor into reset */
@@ -170,7 +170,7 @@ static int k3_dsp_rproc_reset(struct k3_dsp_rproc *kproc)
 
 	ret = reset_control_assert(kproc->reset);
 	if (ret) {
-		dev_err(dev, "local-reset assert failed, ret = %d\n", ret);
+		dev_err(dev, "local-reset assert failed (%pe)\n", ERR_PTR(ret));
 		return ret;
 	}
 
@@ -180,7 +180,7 @@ static int k3_dsp_rproc_reset(struct k3_dsp_rproc *kproc)
 	ret = kproc->ti_sci->ops.dev_ops.put_device(kproc->ti_sci,
 						    kproc->ti_sci_id);
 	if (ret) {
-		dev_err(dev, "module-reset assert failed, ret = %d\n", ret);
+		dev_err(dev, "module-reset assert failed (%pe)\n", ERR_PTR(ret));
 		if (reset_control_deassert(kproc->reset))
 			dev_warn(dev, "local-reset deassert back failed\n");
 	}
@@ -200,14 +200,14 @@ static int k3_dsp_rproc_release(struct k3_dsp_rproc *kproc)
 	ret = kproc->ti_sci->ops.dev_ops.get_device(kproc->ti_sci,
 						    kproc->ti_sci_id);
 	if (ret) {
-		dev_err(dev, "module-reset deassert failed, ret = %d\n", ret);
+		dev_err(dev, "module-reset deassert failed (%pe)\n", ERR_PTR(ret));
 		return ret;
 	}
 
 lreset:
 	ret = reset_control_deassert(kproc->reset);
 	if (ret) {
-		dev_err(dev, "local-reset deassert failed, ret = %d\n", ret);
+		dev_err(dev, "local-reset deassert failed, (%pe)\n", ERR_PTR(ret));
 		if (kproc->ti_sci->ops.dev_ops.put_device(kproc->ti_sci,
 							  kproc->ti_sci_id))
 			dev_warn(dev, "module-reset assert back failed\n");
@@ -246,7 +246,7 @@ static int k3_dsp_rproc_request_mbox(struct rproc *rproc)
 	 */
 	ret = mbox_send_message(kproc->mbox, (void *)RP_MBOX_ECHO_REQUEST);
 	if (ret < 0) {
-		dev_err(dev, "mbox_send_message failed: %d\n", ret);
+		dev_err(dev, "mbox_send_message failed (%pe)\n", ERR_PTR(ret));
 		mbox_free_channel(kproc->mbox);
 		return ret;
 	}
@@ -272,8 +272,8 @@ static int k3_dsp_rproc_prepare(struct rproc *rproc)
 	ret = kproc->ti_sci->ops.dev_ops.get_device(kproc->ti_sci,
 						    kproc->ti_sci_id);
 	if (ret)
-		dev_err(dev, "module-reset deassert failed, cannot enable internal RAM loading, ret = %d\n",
-			ret);
+		dev_err(dev, "module-reset deassert failed, cannot enable internal RAM loading (%pe)\n",
+			ERR_PTR(ret));
 
 	return ret;
 }
@@ -296,7 +296,7 @@ static int k3_dsp_rproc_unprepare(struct rproc *rproc)
 	ret = kproc->ti_sci->ops.dev_ops.put_device(kproc->ti_sci,
 						    kproc->ti_sci_id);
 	if (ret)
-		dev_err(dev, "module-reset assert failed, ret = %d\n", ret);
+		dev_err(dev, "module-reset assert failed (%pe)\n", ERR_PTR(ret));
 
 	return ret;
 }
@@ -561,9 +561,9 @@ static int k3_dsp_reserved_mem_init(struct k3_dsp_rproc *kproc)
 
 	num_rmems = of_property_count_elems_of_size(np, "memory-region",
 						    sizeof(phandle));
-	if (num_rmems <= 0) {
-		dev_err(dev, "device does not reserved memory regions, ret = %d\n",
-			num_rmems);
+	if (num_rmems < 0) {
+		dev_err(dev, "device does not reserved memory regions (%pe)\n",
+			ERR_PTR(num_rmems));
 		return -EINVAL;
 	}
 	if (num_rmems < 2) {
@@ -575,8 +575,8 @@ static int k3_dsp_reserved_mem_init(struct k3_dsp_rproc *kproc)
 	/* use reserved memory region 0 for vring DMA allocations */
 	ret = of_reserved_mem_device_init_by_idx(dev, np, 0);
 	if (ret) {
-		dev_err(dev, "device cannot initialize DMA pool, ret = %d\n",
-			ret);
+		dev_err(dev, "device cannot initialize DMA pool (%pe)\n",
+			ERR_PTR(ret));
 		return ret;
 	}
 
@@ -687,11 +687,8 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev)
 		return -ENODEV;
 
 	ret = rproc_of_parse_firmware(dev, 0, &fw_name);
-	if (ret) {
-		dev_err(dev, "failed to parse firmware-name property, ret = %d\n",
-			ret);
-		return ret;
-	}
+	if (ret)
+		return dev_err_probe(dev, ret, "failed to parse firmware-name property\n");
 
 	rproc = rproc_alloc(dev, dev_name(dev), &k3_dsp_rproc_ops, fw_name,
 			    sizeof(*kproc));
@@ -711,39 +708,35 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev)
 
 	kproc->ti_sci = ti_sci_get_by_phandle(np, "ti,sci");
 	if (IS_ERR(kproc->ti_sci)) {
-		ret = PTR_ERR(kproc->ti_sci);
-		if (ret != -EPROBE_DEFER) {
-			dev_err(dev, "failed to get ti-sci handle, ret = %d\n",
-				ret);
-		}
+		ret = dev_err_probe(dev, PTR_ERR(kproc->ti_sci),
+				    "failed to get ti-sci handle\n");
 		kproc->ti_sci = NULL;
 		goto free_rproc;
 	}
 
 	ret = of_property_read_u32(np, "ti,sci-dev-id", &kproc->ti_sci_id);
 	if (ret) {
-		dev_err(dev, "missing 'ti,sci-dev-id' property\n");
+		dev_err_probe(dev, ret, "missing 'ti,sci-dev-id' property\n");
 		goto put_sci;
 	}
 
 	kproc->reset = devm_reset_control_get_exclusive(dev, NULL);
 	if (IS_ERR(kproc->reset)) {
-		ret = PTR_ERR(kproc->reset);
-		dev_err(dev, "failed to get reset, status = %d\n", ret);
+		ret = dev_err_probe(dev, PTR_ERR(kproc->reset),
+				    "failed to get reset\n");
 		goto put_sci;
 	}
 
 	kproc->tsp = k3_dsp_rproc_of_get_tsp(dev, kproc->ti_sci);
 	if (IS_ERR(kproc->tsp)) {
-		dev_err(dev, "failed to construct ti-sci proc control, ret = %d\n",
-			ret);
-		ret = PTR_ERR(kproc->tsp);
+		ret = dev_err_probe(dev, PTR_ERR(kproc->tsp),
+				    "failed to construct ti-sci proc control\n");
 		goto put_sci;
 	}
 
 	ret = ti_sci_proc_request(kproc->tsp);
 	if (ret < 0) {
-		dev_err(dev, "ti_sci_proc_request failed, ret = %d\n", ret);
+		dev_err_probe(dev, ret, "ti_sci_proc_request failed\n");
 		goto free_tsp;
 	}
 
@@ -753,15 +746,14 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev)
 
 	ret = k3_dsp_reserved_mem_init(kproc);
 	if (ret) {
-		dev_err(dev, "reserved memory init failed, ret = %d\n", ret);
+		dev_err_probe(dev, ret, "reserved memory init failed\n");
 		goto release_tsp;
 	}
 
 	ret = kproc->ti_sci->ops.dev_ops.is_on(kproc->ti_sci, kproc->ti_sci_id,
 					       NULL, &p_state);
 	if (ret) {
-		dev_err(dev, "failed to get initial state, mode cannot be determined, ret = %d\n",
-			ret);
+		dev_err_probe(dev, ret, "failed to get initial state, mode cannot be determined\n");
 		goto release_mem;
 	}
 
@@ -787,8 +779,7 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev)
 		if (data->uses_lreset) {
 			ret = reset_control_status(kproc->reset);
 			if (ret < 0) {
-				dev_err(dev, "failed to get reset status, status = %d\n",
-					ret);
+				dev_err_probe(dev, ret, "failed to get reset status\n");
 				goto release_mem;
 			} else if (ret == 0) {
 				dev_warn(dev, "local reset is deasserted for device\n");
@@ -799,8 +790,7 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev)
 
 	ret = rproc_add(rproc);
 	if (ret) {
-		dev_err(dev, "failed to add register device with remoteproc core, status = %d\n",
-			ret);
+		dev_err_probe(dev, ret, "failed to add register device with remoteproc core\n");
 		goto release_mem;
 	}
 
@@ -813,13 +803,13 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev)
 release_tsp:
 	ret1 = ti_sci_proc_release(kproc->tsp);
 	if (ret1)
-		dev_err(dev, "failed to release proc, ret = %d\n", ret1);
+		dev_err(dev, "failed to release proc (%pe)\n", ERR_PTR(ret1));
 free_tsp:
 	kfree(kproc->tsp);
 put_sci:
 	ret1 = ti_sci_put_handle(kproc->ti_sci);
 	if (ret1)
-		dev_err(dev, "failed to put ti_sci handle, ret = %d\n", ret1);
+		dev_err(dev, "failed to put ti_sci handle (%pe)\n", ERR_PTR(ret1));
 free_rproc:
 	rproc_free(rproc);
 	return ret;
@@ -836,7 +826,7 @@ static int k3_dsp_rproc_remove(struct platform_device *pdev)
 		ret = rproc_detach(rproc);
 		if (ret) {
 			/* Note this error path leaks resources */
-			dev_err(dev, "failed to detach proc, ret = %d\n", ret);
+			dev_err(dev, "failed to detach proc (%pe)\n", ERR_PTR(ret));
 			return 0;
 		}
 	}
@@ -845,13 +835,13 @@ static int k3_dsp_rproc_remove(struct platform_device *pdev)
 
 	ret = ti_sci_proc_release(kproc->tsp);
 	if (ret)
-		dev_err(dev, "failed to release proc, ret = %d\n", ret);
+		dev_err(dev, "failed to release proc (%pe)\n", ERR_PTR(ret));
 
 	kfree(kproc->tsp);
 
 	ret = ti_sci_put_handle(kproc->ti_sci);
 	if (ret)
-		dev_err(dev, "failed to put ti_sci handle, ret = %d\n", ret);
+		dev_err(dev, "failed to put ti_sci handle (%pe)\n", ERR_PTR(ret));
 
 	k3_dsp_reserved_mem_exit(kproc);
 	rproc_free(kproc->rproc);
-- 
2.42.0

[PATCH 3/3] remoteproc: k3-dsp: Convert to platform remove callback returning void

[Uwe Kleine-König]

The .remove() callback for a platform driver returns an int which makes
many driver authors wrongly assume it's possible to do error handling by
returning an error code. However the value returned is ignored (apart
from emitting a warning) and this typically results in resource leaks.

To improve here there is a quest to make the remove callback return
void. In the first step of this quest all drivers are converted to
.remove_new(), which already returns void. Eventually after all drivers
are converted, .remove_new() will be renamed to .remove().

There is no change in behaviour as .remove() already returned zero
unconditionally.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/remoteproc/ti_k3_dsp_remoteproc.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/remoteproc/ti_k3_dsp_remoteproc.c b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
index f048ec1bb00f..ab882e3b7130 100644
--- a/drivers/remoteproc/ti_k3_dsp_remoteproc.c
+++ b/drivers/remoteproc/ti_k3_dsp_remoteproc.c
@@ -815,7 +815,7 @@ static int k3_dsp_rproc_probe(struct platform_device *pdev)
 	return ret;
 }
 
-static int k3_dsp_rproc_remove(struct platform_device *pdev)
+static void k3_dsp_rproc_remove(struct platform_device *pdev)
 {
 	struct k3_dsp_rproc *kproc = platform_get_drvdata(pdev);
 	struct rproc *rproc = kproc->rproc;
@@ -827,7 +827,7 @@ static int k3_dsp_rproc_remove(struct platform_device *pdev)
 		if (ret) {
 			/* Note this error path leaks resources */
 			dev_err(dev, "failed to detach proc (%pe)\n", ERR_PTR(ret));
-			return 0;
+			return;
 		}
 	}
 
@@ -845,8 +845,6 @@ static int k3_dsp_rproc_remove(struct platform_device *pdev)
 
 	k3_dsp_reserved_mem_exit(kproc);
 	rproc_free(kproc->rproc);
-
-	return 0;
 }
 
 static const struct k3_dsp_mem_data c66_mems[] = {
@@ -897,7 +895,7 @@ MODULE_DEVICE_TABLE(of, k3_dsp_of_match);
 
 static struct platform_driver k3_dsp_rproc_driver = {
 	.probe	= k3_dsp_rproc_probe,
-	.remove	= k3_dsp_rproc_remove,
+	.remove_new = k3_dsp_rproc_remove,
 	.driver	= {
 		.name = "k3-dsp-rproc",
 		.of_match_table = k3_dsp_of_match,
-- 
2.42.0