Re: [PATCH] ACPI: AGDI: Improve error reporting for problems during .remove()

[Lorenzo Pieralisi <lpieralisi@kernel.org>]

[+James]

On Fri, Oct 14, 2022 at 06:06:23PM +0200, Uwe Kleine-König wrote:
> Returning an error value in a platform driver's remove callback results in
> a generic error message being emitted by the driver core, but otherwise it
> doesn't make a difference. The device goes away anyhow.
> 
> So instead of triggering the generic platform error message, emit a more
> helpful message if a problem occurs and return 0 to suppress the generic
> message.
> 
> This patch is a preparation for making platform remove callbacks return
> void.

If that's the plan - I don't have anything against this patch.

> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
> Hello,
> 
> note that in the situations where the driver returned an error before
> and now emits a message, there is a resource leak. Someone who knows
> more about this driver and maybe even can test stuff, might want to
> address this. This might not only be about non-freed memory, the device
> disappears but it is kept in sdei_list and so might be used after being
> gone.

I'd need James' input on this. I guess we may ignore
sdei_event_disable() return value and continue anyway in agdi_remove(),
whether that's the right thing to do it is a different question.

Lorenzo

> Best regards
> Uwe
> 
>  drivers/acpi/arm64/agdi.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/acpi/arm64/agdi.c b/drivers/acpi/arm64/agdi.c
> index cf31abd0ed1b..f605302395c3 100644
> --- a/drivers/acpi/arm64/agdi.c
> +++ b/drivers/acpi/arm64/agdi.c
> @@ -64,8 +64,11 @@ static int agdi_remove(struct platform_device *pdev)
>  	int err, i;
>  
>  	err = sdei_event_disable(adata->sdei_event);
> -	if (err)
> -		return err;
> +	if (err) {
> +		dev_err(&pdev->dev, "Failed to disable sdei-event #%d (%pe)\n",
> +			adata->sdei_event, ERR_PTR(err));
> +		return 0;
> +	}
>  
>  	for (i = 0; i < 3; i++) {
>  		err = sdei_event_unregister(adata->sdei_event);
> @@ -75,7 +78,11 @@ static int agdi_remove(struct platform_device *pdev)
>  		schedule();
>  	}
>  
> -	return err;
> +	if (err)
> +		dev_err(&pdev->dev, "Failed to unregister sdei-event #%d (%pe)\n",
> +			adata->sdei_event, ERR_PTR(err));
> +
> +	return 0;
>  }
>  
>  static struct platform_driver agdi_driver = {
> 
> base-commit: 4fe89d07dcc2804c8b562f6c7896a45643d34b2f
> -- 
> 2.37.2
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel