linux-kernel - Re: [PATCHv4 3/4] OMAP3: Devkit8000: Check return value of gpio

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1295440190.2635.18.camel@morsing>
Date:	Wed, 19 Jan 2011 13:29:50 +0100
From:	Daniel Morsing <daniel.morsing@...il.com>
To:	Thomas Weber <weber@...science.de>
Cc:	linux-omap@...r.kernel.org, Tony Lindgren <tony@...mide.com>,
	Russell King <linux@....linux.org.uk>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	charu@...com, sshtylyov@...sta.com
Subject: Re: [PATCHv4 3/4] OMAP3: Devkit8000: Check return value of
 gpio_request

Hey Thomas

On Wed, 2011-01-19 at 09:19 +0100, Thomas Weber wrote:
> The return value of gpio_request is ignored.
> This patch adds the check of the return value of gpio_request.
> 
> Signed-off-by: Thomas Weber <weber@...science.de>
> ---
>  arch/arm/mach-omap2/board-devkit8000.c |   16 ++++++++++++++--
>  1 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/arm/mach-omap2/board-devkit8000.c b/arch/arm/mach-omap2/board-devkit8000.c
> index 9fb416b..4ddd81c 100644
> --- a/arch/arm/mach-omap2/board-devkit8000.c
> +++ b/arch/arm/mach-omap2/board-devkit8000.c
> @@ -234,6 +234,8 @@ static struct gpio_led gpio_leds[];
>  static int devkit8000_twl_gpio_setup(struct device *dev,
>  		unsigned gpio, unsigned ngpio)
>  {
> +	int ret;
> +
>  	omap_mux_init_gpio(29, OMAP_PIN_INPUT);
>  	/* gpio + 0 is "mmc0_cd" (input/IRQ) */
>  	mmc[0].gpio_cd = gpio + 0;
> @@ -244,13 +246,23 @@ static int devkit8000_twl_gpio_setup(struct device *dev,
>  
>  	/* TWL4030_GPIO_MAX + 0 is "LCD_PWREN" (out, active high) */
>  	devkit8000_lcd_device.reset_gpio = gpio + TWL4030_GPIO_MAX + 0;
> -	gpio_request(devkit8000_lcd_device.reset_gpio, "LCD_PWREN");
> +	ret = gpio_request(devkit8000_lcd_device.reset_gpio, "LCD_PWREN");
> +	if (ret < 0) {
> +		printk(KERN_ERR "Failed to request GPIO for LCD_PWRN\n");
> +		return ret;
> +	}
> +

If we fail here, reset_gpio will be set to the gpio the was requested.
The main reason for this would be that the gpio has already been
requested, so any subsequent lcd operations could potentially mess up
some other code.

>  	/* Disable until needed */
>  	gpio_direction_output(devkit8000_lcd_device.reset_gpio, 0);
>  
>  	/* gpio + 7 is "DVI_PD" (out, active low) */
>  	devkit8000_dvi_device.reset_gpio = gpio + 7;
> -	gpio_request(devkit8000_dvi_device.reset_gpio, "DVI PowerDown");
> +	ret = gpio_request(devkit8000_dvi_device.reset_gpio, "DVI PowerDown");
> +	if (ret < 0) {
> +		printk(KERN_ERR "Failed to request GPIO for DVI PowerDown\n");
> +		return ret;
> +	}
> +

Same as above.

>  	/* Disable until needed */
>  	gpio_direction_output(devkit8000_dvi_device.reset_gpio, 0);
>  

Consider switching to gpio_request_{one, array}. Besides making
everything cleaner, it would also provide error checking for the
unlikely case that the request succeeded, but the direction setting
failed.

Also agreeing with Manjunath. This patch should be merged with the
adjust lcd gpio patch. The patches depend on each other and the
resulting patch is not big enough to warrant a split.

Regards,
Daniel

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/