* [PATCH-V2] OMAP3EVM: Made backlight GPIO default state to off
@ 2011-01-24 19:22 Vaibhav Hiremath
2011-01-25 2:34 ` Semwal, Sumit
0 siblings, 1 reply; 6+ messages in thread
From: Vaibhav Hiremath @ 2011-01-24 19:22 UTC (permalink / raw)
To: linux-omap; +Cc: tony, tomi.valkeinen, Vaibhav Hiremath
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@ti.com>
---
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");
+
+ 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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH-V2] OMAP3EVM: Made backlight GPIO default state to off
2011-01-24 19:22 [PATCH-V2] OMAP3EVM: Made backlight GPIO default state to off Vaibhav Hiremath
@ 2011-01-25 2:34 ` Semwal, Sumit
2011-01-25 4:48 ` Hiremath, Vaibhav
0 siblings, 1 reply; 6+ messages in thread
From: Semwal, Sumit @ 2011-01-25 2:34 UTC (permalink / raw)
To: Vaibhav Hiremath; +Cc: linux-omap, tony, tomi.valkeinen
Vaibhav,
On Tue, Jan 25, 2011 at 12:52 AM, Vaibhav Hiremath <hvaibhav@ti.com> 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@ti.com>
> ---
> 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?
> +
> + 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@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH-V2] OMAP3EVM: Made backlight GPIO default state to off
2011-01-25 2:34 ` Semwal, Sumit
@ 2011-01-25 4:48 ` Hiremath, Vaibhav
2011-01-25 4:53 ` Varadarajan, Charulatha
0 siblings, 1 reply; 6+ messages in thread
From: Hiremath, Vaibhav @ 2011-01-25 4:48 UTC (permalink / raw)
To: Semwal, Sumit; +Cc: linux-omap, tony, tomi.valkeinen
> -----Original Message-----
> From: Semwal, Sumit
> Sent: Tuesday, January 25, 2011 8:05 AM
> To: Hiremath, Vaibhav
> Cc: linux-omap@vger.kernel.org; tony@atomide.com; tomi.valkeinen@nokia.com
> Subject: Re: [PATCH-V2] OMAP3EVM: Made backlight GPIO default state to off
>
> Vaibhav,
>
> On Tue, Jan 25, 2011 at 12:52 AM, Vaibhav Hiremath <hvaibhav@ti.com>
> 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@ti.com>
> > ---
> > 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.
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@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH-V2] OMAP3EVM: Made backlight GPIO default state to off
2011-01-25 4:48 ` Hiremath, Vaibhav
@ 2011-01-25 4:53 ` Varadarajan, Charulatha
2011-01-25 5:20 ` Hiremath, Vaibhav
0 siblings, 1 reply; 6+ messages in thread
From: Varadarajan, Charulatha @ 2011-01-25 4:53 UTC (permalink / raw)
To: Hiremath, Vaibhav; +Cc: Semwal, Sumit, linux-omap, tony, tomi.valkeinen
On Tue, Jan 25, 2011 at 10:18, Hiremath, Vaibhav <hvaibhav@ti.com> wrote:
>
>> -----Original Message-----
>> From: Semwal, Sumit
>> Sent: Tuesday, January 25, 2011 8:05 AM
>> To: Hiremath, Vaibhav
>> Cc: linux-omap@vger.kernel.org; tony@atomide.com; tomi.valkeinen@nokia.com
>> Subject: Re: [PATCH-V2] OMAP3EVM: Made backlight GPIO default state to off
>>
>> Vaibhav,
>>
>> On Tue, Jan 25, 2011 at 12:52 AM, Vaibhav Hiremath <hvaibhav@ti.com>
>> 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@ti.com>
>> > ---
>> > 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.
>
> 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@vger.kernel.org
>> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>> >
> --
> To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH-V2] OMAP3EVM: Made backlight GPIO default state to off
2011-01-25 4:53 ` Varadarajan, Charulatha
@ 2011-01-25 5:20 ` Hiremath, Vaibhav
2011-01-25 6:26 ` Varadarajan, Charulatha
0 siblings, 1 reply; 6+ messages in thread
From: Hiremath, Vaibhav @ 2011-01-25 5:20 UTC (permalink / raw)
To: Varadarajan, Charulatha; +Cc: Semwal, Sumit, linux-omap, tony, tomi.valkeinen
> -----Original Message-----
> From: Varadarajan, Charulatha [mailto:charu@ti.com]
> Sent: Tuesday, January 25, 2011 10:24 AM
> To: Hiremath, Vaibhav
> Cc: Semwal, Sumit; linux-omap@vger.kernel.org; tony@atomide.com;
> tomi.valkeinen@nokia.com
> Subject: Re: [PATCH-V2] OMAP3EVM: Made backlight GPIO default state to off
>
> On Tue, Jan 25, 2011 at 10:18, Hiremath, Vaibhav <hvaibhav@ti.com> wrote:
> >
> >> -----Original Message-----
> >> From: Semwal, Sumit
> >> Sent: Tuesday, January 25, 2011 8:05 AM
> >> To: Hiremath, Vaibhav
> >> Cc: linux-omap@vger.kernel.org; tony@atomide.com;
> tomi.valkeinen@nokia.com
> >> Subject: Re: [PATCH-V2] OMAP3EVM: Made backlight GPIO default state to
> off
> >>
> >> Vaibhav,
> >>
> >> On Tue, Jan 25, 2011 at 12:52 AM, Vaibhav Hiremath <hvaibhav@ti.com>
> >> 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@ti.com>
> >> > ---
> >> > 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.
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.
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 -
} 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@vger.kernel.org
> >> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-omap" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH-V2] OMAP3EVM: Made backlight GPIO default state to off
2011-01-25 5:20 ` Hiremath, Vaibhav
@ 2011-01-25 6:26 ` Varadarajan, Charulatha
0 siblings, 0 replies; 6+ messages in thread
From: Varadarajan, Charulatha @ 2011-01-25 6:26 UTC (permalink / raw)
To: Hiremath, Vaibhav; +Cc: Semwal, Sumit, linux-omap, tony, tomi.valkeinen
On Tue, Jan 25, 2011 at 10:50, Hiremath, Vaibhav <hvaibhav@ti.com> wrote:
>
>> -----Original Message-----
>> From: Varadarajan, Charulatha [mailto:charu@ti.com]
>> Sent: Tuesday, January 25, 2011 10:24 AM
>> To: Hiremath, Vaibhav
>> Cc: Semwal, Sumit; linux-omap@vger.kernel.org; tony@atomide.com;
>> tomi.valkeinen@nokia.com
>> Subject: Re: [PATCH-V2] OMAP3EVM: Made backlight GPIO default state to off
>>
>> On Tue, Jan 25, 2011 at 10:18, Hiremath, Vaibhav <hvaibhav@ti.com> wrote:
>> >
>> >> -----Original Message-----
>> >> From: Semwal, Sumit
>> >>
>> >> Vaibhav,
>> >>
>> >> On Tue, Jan 25, 2011 at 12:52 AM, Vaibhav Hiremath <hvaibhav@ti.com>
>> >> 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@ti.com>
>> >> > ---
>> >> > 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@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-01-25 6:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-24 19:22 [PATCH-V2] OMAP3EVM: Made backlight GPIO default state to off Vaibhav Hiremath
2011-01-25 2:34 ` Semwal, Sumit
2011-01-25 4:48 ` Hiremath, Vaibhav
2011-01-25 4:53 ` Varadarajan, Charulatha
2011-01-25 5:20 ` Hiremath, Vaibhav
2011-01-25 6:26 ` Varadarajan, Charulatha
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.