On Tue, Jan 25, 2011 at 10:50, Hiremath, Vaibhav <hvaibhav@xxxxxx> wrote: > >> -----Original Message----- >> From: Varadarajan, Charulatha [mailto:charu@xxxxxx] >> Sent: Tuesday, January 25, 2011 10:24 AM >> To: Hiremath, Vaibhav >> Cc: Semwal, Sumit; linux-omap@xxxxxxxxxxxxxxx; tony@xxxxxxxxxxx; >> tomi.valkeinen@xxxxxxxxx >> Subject: Re: [PATCH-V2] OMAP3EVM: Made backlight GPIO default state to off >> >> On Tue, Jan 25, 2011 at 10:18, Hiremath, Vaibhav <hvaibhav@xxxxxx> wrote: >> > >> >> -----Original Message----- >> >> From: Semwal, Sumit >> >> >> >> Vaibhav, >> >> >> >> On Tue, Jan 25, 2011 at 12:52 AM, Vaibhav Hiremath <hvaibhav@xxxxxx> >> >> wrote: >> >> > If you choose default output to DVI, the LCD backlight used to >> >> > stay on, since panel->disable function never gets called. >> >> > >> >> > So, during init put backlight GPIO to off state and the driver >> >> > code will decide which output to enable. >> >> > >> >> > Signed-off-by: Vaibhav Hiremath <hvaibhav@xxxxxx> >> >> > --- >> >> > Changes from V1 - >> >> > - Added check for return value >> >> > >> >> > arch/arm/mach-omap2/board-omap3evm.c | 15 +++++++++++++-- >> >> > 1 files changed, 13 insertions(+), 2 deletions(-) >> >> > >> >> > diff --git a/arch/arm/mach-omap2/board-omap3evm.c b/arch/arm/mach- >> >> omap2/board-omap3evm.c >> >> > index 7119380..a888a7d 100644 >> >> > --- a/arch/arm/mach-omap2/board-omap3evm.c >> >> > +++ b/arch/arm/mach-omap2/board-omap3evm.c >> >> > @@ -411,6 +411,8 @@ static struct platform_device leds_gpio = { >> >> > static int omap3evm_twl_gpio_setup(struct device *dev, >> >> > unsigned gpio, unsigned ngpio) >> >> > { >> >> > + int r; >> >> > + >> >> > /* gpio + 0 is "mmc0_cd" (input/IRQ) */ >> >> > omap_mux_init_gpio(63, OMAP_PIN_INPUT); >> >> > mmc[0].gpio_cd = gpio + 0; >> >> > @@ -426,8 +428,17 @@ static int omap3evm_twl_gpio_setup(struct device >> >> *dev, >> >> > */ >> >> > >> >> > /* TWL4030_GPIO_MAX + 0 == ledA, LCD Backlight control */ >> >> > - gpio_request(gpio + TWL4030_GPIO_MAX, "EN_LCD_BKL"); >> >> > - gpio_direction_output(gpio + TWL4030_GPIO_MAX, 0); >> >> > + r = gpio_request(gpio + TWL4030_GPIO_MAX, "EN_LCD_BKL"); >> >> > + if (r) >> >> > + printk(KERN_ERR "failed to get lcd_bkl gpio\n"); >> >> So even if the gpio_request fails, this code prints an error and >> >> continues? >> > [Hiremath, Vaibhav] yes, that's correct and intended. >> >> As pointed out by Sumit, you should not continue modifying the >> direction of a gpio whose request failed (may be some other module >> is using this gpio)? Please mention why this is intentionally done so. >> > [Hiremath, Vaibhav] First of all, if you look at implementation of gpio_xxx, it does handle all these scenarios gracefully. I guess you are talking about gpio_ensure_requested() used in gpio_xxx calls. That would throw warnings if the gpio is not requested but being used (eg., set output direction). > > And second point is, the request is happening for backlight gpio (which is not something MUST required), so even if we fail to get handle here I think we should not return an error, just flag the warning and continue. If this is the case, I guess, we need do a gpio_request() at all. Instead, we shall directly do a gpio_direction_output(). But I would like to differ. > > Let me understand, why do you want to return from middle of function, for backlight gpio request, which means we will not call "platform_device_register" for leds_gpio and many other things. And also if you look at the twl4030-gpio.c, we do same thing - Well, I am not saying that you should return from the function, if gpio_request fails. I am suggesting that you should not continue using the gpio whose request failed. One reason why gpio_request() might fail is in case some other request has already happened for the same gpio. > > > } else if (pdata->setup) { > int status; > > status = pdata->setup(&pdev->dev, > pdata->gpio_base, TWL4030_GPIO_MAX); > if (status) > dev_dbg(&pdev->dev, "setup --> %d\n", status); > } > return ret; > > I hope this clarifies your doubts. > > Thanks, > Vaibhav >> > >> > Thanks, >> > Vaibhav >> >> > + >> >> > + if (get_omap3_evm_rev() >= OMAP3EVM_BOARD_GEN_2) >> >> > + r = gpio_direction_output(gpio + TWL4030_GPIO_MAX, >> 1); >> >> > + else >> >> > + r = gpio_direction_output(gpio + TWL4030_GPIO_MAX, >> 0); >> >> > + >> >> > + if (r) >> >> > + printk(KERN_ERR "failed to set direction of lcd_bkl >> >> gpio\n"); >> >> > >> >> > /* gpio + 7 == DVI Enable */ >> >> > gpio_request(gpio + 7, "EN_DVI"); >> >> > -- >> >> > 1.6.2.4 >> >> > -- To unsubscribe from this list: send the line "unsubscribe linux-omap" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
