Messages in this thread | | | Subject | Re: The default value of enable_gpio in pwm-backlight driver? | From | Yingjoe Chen <> | Date | Mon, 14 Sep 2015 11:24:23 +0800 |
| |
Hi Alexandre, Thierry,
On Thu, 2015-09-03 at 21:08 +0800, YH Huang wrote: <...> > From Alexandre Courbot <> > Subject [PATCH 2/2] pwm-backlight: switch to gpiod interface > Date Thu, 27 Feb 2014 14:53:34 +0900 > > Switch to the new gpiod interface, which allows to handle GPIO > properties such as active low transparently and removes a whole bunch of > code. > > There are still a couple of users of this driver that rely on passing > the enable GPIO number through platform data, so a fallback mechanism > using a GPIO number is still available to avoid breaking them. It will > be removed once current users have switched to the GPIO lookup tables > provided by the gpiod interface. > > --- a/drivers/video/backlight/pwm_bl.c > +++ b/drivers/video/backlight/pwm_bl.c > > @@ -265,26 +245,39 @@ static int pwm_backlight_probe(struct > platform_device *pdev) > pb->dev = &pdev->dev; > pb->enabled = false; > > - if (gpio_is_valid(pb->enable_gpio)) { > - unsigned long flags; > - > - if (pb->enable_gpio_flags & PWM_BACKLIGHT_GPIO_ACTIVE_LOW) > - flags = GPIOF_OUT_INIT_HIGH; > - else > - flags = GPIOF_OUT_INIT_LOW;
In commit 257462dbf3ed ("pwm-backlight: switch to gpiod interface"). It seems to me the original code is trying to make it default disable...
> + pb->enable_gpio = devm_gpiod_get(&pdev->dev, "enable"); > + if (IS_ERR(pb->enable_gpio)) { > + ret = PTR_ERR(pb->enable_gpio); > + if (ret == -ENOENT) { > + pb->enable_gpio = NULL; > + ret = 0; > + } else { > + goto err_alloc; > + } > + } > > - ret = gpio_request_one(pb->enable_gpio, flags, "enable"); > + /* > + * Compatibility fallback for drivers still using the integer GPIO > + * platform data. Must go away soon. > + */ > + if (pb->enable_gpio == NULL && gpio_is_valid(data->enable_gpio)) { > + ret = devm_gpio_request_one(&pdev->dev, data->enable_gpio, > + GPIOF_OUT_INIT_HIGH, "enable"); > if (ret < 0) { > dev_err(&pdev->dev, "failed to request GPIO#%d: %d\n", > - pb->enable_gpio, ret); > + data->enable_gpio, ret); > goto err_alloc; > } > + pb->enable_gpio = gpio_to_desc(data->enable_gpio); > } > > + if (pb->enable_gpio) > + gpiod_direction_output(pb->enable_gpio, 1);
But the new code here set it to enable. Is this on purpose, or am I miss read anything?
Joe.C
| |