[PATCH] fbdev: da8xx-fb: Fix error handling in .remove()

[PATCH] fbdev: da8xx-fb: Fix error handling in .remove()

[Uwe Kleine-König]

Even in the presence of problems (here: regulator_disable() might fail),
it's important to unregister all resources acquired during .probe() and
disable the device (i.e. DMA activity) because even if .remove() returns
an error code, the device is removed and the .remove() callback is never
called again later to catch up.

This is a preparation for making platform remove callbacks return void.

Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
---
 drivers/video/fbdev/da8xx-fb.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/da8xx-fb.c b/drivers/video/fbdev/da8xx-fb.c
index ae76a2111c77..11922b009ed7 100644
--- a/drivers/video/fbdev/da8xx-fb.c
+++ b/drivers/video/fbdev/da8xx-fb.c
@@ -1076,7 +1076,8 @@ static int fb_remove(struct platform_device *dev)
 	if (par->lcd_supply) {
 		ret = regulator_disable(par->lcd_supply);
 		if (ret)
-			return ret;
+			dev_warn(&dev->dev, "Failed to disable regulator (%pe)\n",
+				 ERR_PTR(ret));
 	}
 
 	lcd_disable_raster(DA8XX_FRAME_WAIT);

base-commit: 4fe89d07dcc2804c8b562f6c7896a45643d34b2f
-- 
2.37.2

Re: [PATCH] fbdev: da8xx-fb: Fix error handling in .remove()

[Helge Deller]

On 10/17/22 21:52, Uwe Kleine-König wrote:
> Even in the presence of problems (here: regulator_disable() might fail),
> it's important to unregister all resources acquired during .probe() and
> disable the device (i.e. DMA activity) because even if .remove() returns
> an error code, the device is removed and the .remove() callback is never
> called again later to catch up.
>
> This is a preparation for making platform remove callbacks return void.
>
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>

applied.

Thanks!
Helge

> ---
>   drivers/video/fbdev/da8xx-fb.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/da8xx-fb.c b/drivers/video/fbdev/da8xx-fb.c
> index ae76a2111c77..11922b009ed7 100644
> --- a/drivers/video/fbdev/da8xx-fb.c
> +++ b/drivers/video/fbdev/da8xx-fb.c
> @@ -1076,7 +1076,8 @@ static int fb_remove(struct platform_device *dev)
>   	if (par->lcd_supply) {
>   		ret = regulator_disable(par->lcd_supply);
>   		if (ret)
> -			return ret;
> +			dev_warn(&dev->dev, "Failed to disable regulator (%pe)\n",
> +				 ERR_PTR(ret));
>   	}
>
>   	lcd_disable_raster(DA8XX_FRAME_WAIT);
>
> base-commit: 4fe89d07dcc2804c8b562f6c7896a45643d34b2f

Re: [PATCH] fbdev: da8xx-fb: Fix error handling in .remove()

[Uwe Kleine-König]

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

On Tue, Oct 18, 2022 at 08:08:49AM +0200, Helge Deller wrote:
> On 10/17/22 21:52, Uwe Kleine-König wrote:
> > Even in the presence of problems (here: regulator_disable() might fail),
> > it's important to unregister all resources acquired during .probe() and
> > disable the device (i.e. DMA activity) because even if .remove() returns
> > an error code, the device is removed and the .remove() callback is never
> > called again later to catch up.
> > 
> > This is a preparation for making platform remove callbacks return void.
> > 
> > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> applied.

Great. If you want a Fixes: line, that's:

Fixes: 611097d5daea ("fbdev: da8xx: add support for a regulator")

(expanded Cc: a bit with the people involved there.)

Best regards
Uwe

> > ---
> >   drivers/video/fbdev/da8xx-fb.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/video/fbdev/da8xx-fb.c b/drivers/video/fbdev/da8xx-fb.c
> > index ae76a2111c77..11922b009ed7 100644
> > --- a/drivers/video/fbdev/da8xx-fb.c
> > +++ b/drivers/video/fbdev/da8xx-fb.c
> > @@ -1076,7 +1076,8 @@ static int fb_remove(struct platform_device *dev)
> >   	if (par->lcd_supply) {
> >   		ret = regulator_disable(par->lcd_supply);
> >   		if (ret)
> > -			return ret;
> > +			dev_warn(&dev->dev, "Failed to disable regulator (%pe)\n",
> > +				 ERR_PTR(ret));
> >   	}
> > 
> >   	lcd_disable_raster(DA8XX_FRAME_WAIT);
> > 
> > base-commit: 4fe89d07dcc2804c8b562f6c7896a45643d34b2f
> 
> 
> 

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