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/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ