Re: [PATCH 3/5] fbdev: omap2: panel-dpi: drop assignment to local variable

[Tomi Valkeinen <tomi.valkeinen@ti.com>]

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



On 10/12/15 15:11, Uwe Kleine-König wrote:
> From: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> 
> The variable gpio is only used to store the return value of
> devm_gpiod_get_optional just to assign it to a member of the driver
> data.
> 
> Get rid of this local variable and assign to driver data directly.
> 
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de>
> ---
>  drivers/video/fbdev/omap2/displays-new/panel-dpi.c | 10 ++++------
>  1 file changed, 4 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/video/fbdev/omap2/displays-new/panel-dpi.c b/drivers/video/fbdev/omap2/displays-new/panel-dpi.c
> index e780fd4f8b46..1216341a0d19 100644
> --- a/drivers/video/fbdev/omap2/displays-new/panel-dpi.c
> +++ b/drivers/video/fbdev/omap2/displays-new/panel-dpi.c
> @@ -205,13 +205,11 @@ static int panel_dpi_probe_of(struct platform_device *pdev)
>  	int r;
>  	struct display_timing timing;
>  	struct videomode vm;
> -	struct gpio_desc *gpio;
>  
> -	gpio = devm_gpiod_get_optional(&pdev->dev, "enable", GPIOD_OUT_LOW);
> -	if (IS_ERR(gpio))
> -		return PTR_ERR(gpio);
> -
> -	ddata->enable_gpio = gpio;
> +	ddata->enable_gpio = devm_gpiod_get_optional(&pdev->dev,
> +						     "enable", GPIOD_OUT_LOW);
> +	if (IS_ERR(ddata->enable_gpio))
> +		return PTR_ERR(ddata->enable_gpio);
>  
>  	ddata->backlight_gpio = -ENOENT;

I usually try to avoid writing bad values to fields. Here
ddata->enable_gpio may get an error ptr. It probably doesn't matter as
we bail out right away, but still. If devm_gpiod_get_optional's return
value would be NULL or valid gpio_desc*, then it'd be fine.

And the code is shorter (more readable) when using just "gpio" instead
of "ddata->enable_gpio".

So I'll leave this one out.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]