Re: [PATCH 9/9] backlight: atmel-pwm-bl: use gpio_request_one

On 14:20 Tue 29 Oct     , Johan Hovold wrote:

On Fri, Oct 25, 2013 at 01:15:43PM +0200, Jean-Christophe PLAGNIOL-VILLARD wrote:

quoted

On 11:55 Wed 23 Oct     , Johan Hovold wrote:

quoted

Use devm_gpio_request_one rather than requesting and setting direction
in two calls.

this is the same I do not see any advantage

It removes one error path.

quoted

and as I said for ather backligth It's wrong to enable or disable it at probe
as the bootloader might have already enable it for splash screen

I agree with you on that, and it's actually the reason for the below
clean up. I use a second patch:

        if (gpio_is_valid(pwmbl->gpio_on)) {
-               /* Turn display off by default. */
-               if (pdata->on_active_low)
+               /* Turn display off unless already enabled. */
+               if (pdata->gpio_on_enabled ^ pdata->on_active_low)
                        flags = GPIOF_OUT_INIT_HIGH;
                else
                        flags = GPIOF_OUT_INIT_LOW;

to make sure the driver does not temporarily disable the backlight at
probe.

Decided not to submit it as part of this series when I realised that
several other backlight drivers did the same, but perhaps I should?

I'm not happy with this idea to play with enable at probe time

we need to detect the current status if possible and only enable or disable
when requiered

Best Regards,
J.

Thanks,
Johan

quoted

quoted

Acked-by: Jingoo Han <redacted>:w
Signed-off-by: Johan Hovold <redacted>
---
 drivers/video/backlight/atmel-pwm-bl.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/video/backlight/atmel-pwm-bl.c b/drivers/video/backlight/atmel-pwm-bl.c
index 1277e0c..5ea2517 100644
--- a/drivers/video/backlight/atmel-pwm-bl.c
+++ b/drivers/video/backlight/atmel-pwm-bl.c

@@ -124,6 +124,7 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 	const struct atmel_pwm_bl_platform_data *pdata;
 	struct backlight_device *bldev;
 	struct atmel_pwm_bl *pwmbl;
+	int flags;
 	int retval;
 
 	pdata = dev_get_platdata(&pdev->dev);

@@ -149,14 +150,14 @@ static int atmel_pwm_bl_probe(struct platform_device *pdev)
 		return retval;
 
 	if (gpio_is_valid(pwmbl->gpio_on)) {
-		retval = devm_gpio_request(&pdev->dev, pwmbl->gpio_on,
-					"gpio_atmel_pwm_bl");
-		if (retval)
-			goto err_free_pwm;
-
 		/* Turn display off by default. */
-		retval = gpio_direction_output(pwmbl->gpio_on,
-				0 ^ pdata->on_active_low);
+		if (pdata->on_active_low)
+			flags = GPIOF_OUT_INIT_HIGH;
+		else
+			flags = GPIOF_OUT_INIT_LOW;
+
+		retval = devm_gpio_request_one(&pdev->dev, pwmbl->gpio_on,
+						flags, "gpio_atmel_pwm_bl");
 		if (retval)
 			goto err_free_pwm;
 	}

-- 
1.8.4