lkml.org 
[lkml]   [2011]   [Jan]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCHv4 3/4] OMAP3: Devkit8000: Check return value of gpio_request
From
Date
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@corscience.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



\
 
 \ /
  Last update: 2011-01-19 13:31    [W:1.232 / U:0.088 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site