| Message ID | 1444959454-1516-1-git-send-email-yh.huang@mediatek.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi YH,
[auto build test ERROR on pwm/for-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base]
url: https://github.com/0day-ci/linux/commits/YH-Huang/pwm-backlight-fix-the-panel-power-sequence/20151016-093957
config: i386-allmodconfig (attached as .config)
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
drivers/video/backlight/pwm_bl.c: In function 'pwm_backlight_probe':
>> drivers/video/backlight/pwm_bl.c:245:20: error: too few arguments to function 'devm_gpiod_get_optional'
pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable");
^
In file included from drivers/video/backlight/pwm_bl.c:13:0:
include/linux/gpio/consumer.h:80:32: note: declared here
struct gpio_desc *__must_check devm_gpiod_get_optional(struct device *dev,
^
vim +/devm_gpiod_get_optional +245 drivers/video/backlight/pwm_bl.c
239 pb->notify_after = data->notify_after;
240 pb->check_fb = data->check_fb;
241 pb->exit = data->exit;
242 pb->dev = &pdev->dev;
243 pb->enabled = false;
244
> 245 pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable");
246 if (IS_ERR(pb->enable_gpio)) {
247 ret = PTR_ERR(pb->enable_gpio);
248 goto err_alloc;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
Am Freitag, den 16.10.2015, 09:37 +0800 schrieb YH Huang: > In order to match the panel power sequence, disable the enable_gpio > in the probe function. Also, reorder the code in the power_on and > power_off function to match the timing. > You aren't specifying which panels power sequence you are matching here. Are you sure you aren't breaking other panels with this patch? Regards, Lucas > Signed-off-by: YH Huang <yh.huang@mediatek.com> > --- > drivers/video/backlight/pwm_bl.c | 15 +++++++++------ > 1 file changed, 9 insertions(+), 6 deletions(-) > > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c > index eff379b..99eca1e 100644 > --- a/drivers/video/backlight/pwm_bl.c > +++ b/drivers/video/backlight/pwm_bl.c > @@ -54,10 +54,11 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness) > if (err < 0) > dev_err(pb->dev, "failed to enable power supply\n"); > > + pwm_enable(pb->pwm); > + > if (pb->enable_gpio) > gpiod_set_value(pb->enable_gpio, 1); > > - pwm_enable(pb->pwm); > pb->enabled = true; > } > > @@ -66,12 +67,12 @@ static void pwm_backlight_power_off(struct pwm_bl_data *pb) > if (!pb->enabled) > return; > > - pwm_config(pb->pwm, 0, pb->period); > - pwm_disable(pb->pwm); > - > if (pb->enable_gpio) > gpiod_set_value(pb->enable_gpio, 0); > > + pwm_config(pb->pwm, 0, pb->period); > + pwm_disable(pb->pwm); > + > regulator_disable(pb->power_supply); > pb->enabled = false; > } > @@ -241,8 +242,7 @@ static int pwm_backlight_probe(struct platform_device *pdev) > pb->dev = &pdev->dev; > pb->enabled = false; > > - pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable", > - GPIOD_OUT_HIGH); > + pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable"); > if (IS_ERR(pb->enable_gpio)) { > ret = PTR_ERR(pb->enable_gpio); > goto err_alloc; > @@ -264,6 +264,9 @@ static int pwm_backlight_probe(struct platform_device *pdev) > pb->enable_gpio = gpio_to_desc(data->enable_gpio); > } > > + if (pb->enable_gpio) > + gpiod_direction_output(pb->enable_gpio, 0); > + > pb->power_supply = devm_regulator_get(&pdev->dev, "power"); > if (IS_ERR(pb->power_supply)) { > ret = PTR_ERR(pb->power_supply);
On Fri, Oct 16, 2015 at 09:37:34AM +0800, YH Huang wrote: > In order to match the panel power sequence, disable the enable_gpio > in the probe function. Also, reorder the code in the power_on and > power_off function to match the timing. > @@ -241,8 +242,7 @@ static int pwm_backlight_probe(struct platform_device *pdev) > pb->dev = &pdev->dev; > pb->enabled = false; > > - pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable", > - GPIOD_OUT_HIGH); > + pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable"); Please actually test your patches. This change here won't compile. Sascha
On Fri, 2015-10-16 at 10:42 +0800, kbuild test robot wrote: > Hi YH, > > [auto build test ERROR on pwm/for-next -- if it's inappropriate base, please suggest rules for selecting the more suitable base] > > url: https://github.com/0day-ci/linux/commits/YH-Huang/pwm-backlight-fix-the-panel-power-sequence/20151016-093957 > config: i386-allmodconfig (attached as .config) > reproduce: > # save the attached .config to linux build tree > make ARCH=i386 > > All errors (new ones prefixed by >>): > > drivers/video/backlight/pwm_bl.c: In function 'pwm_backlight_probe': > >> drivers/video/backlight/pwm_bl.c:245:20: error: too few arguments to function 'devm_gpiod_get_optional' > pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable"); > ^ > In file included from drivers/video/backlight/pwm_bl.c:13:0: > include/linux/gpio/consumer.h:80:32: note: declared here > struct gpio_desc *__must_check devm_gpiod_get_optional(struct device *dev, > ^ > > vim +/devm_gpiod_get_optional +245 drivers/video/backlight/pwm_bl.c > > 239 pb->notify_after = data->notify_after; > 240 pb->check_fb = data->check_fb; > 241 pb->exit = data->exit; > 242 pb->dev = &pdev->dev; > 243 pb->enabled = false; > 244 > > 245 pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable"); > 246 if (IS_ERR(pb->enable_gpio)) { > 247 ret = PTR_ERR(pb->enable_gpio); > 248 goto err_alloc; > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation Sorry, I made a mistake. I will send patch v2 to fix it. YH Huang
On Fri, 2015-10-16 at 10:36 +0200, Sascha Hauer wrote: > On Fri, Oct 16, 2015 at 09:37:34AM +0800, YH Huang wrote: > > In order to match the panel power sequence, disable the enable_gpio > > in the probe function. Also, reorder the code in the power_on and > > power_off function to match the timing. > > @@ -241,8 +242,7 @@ static int pwm_backlight_probe(struct platform_device *pdev) > > pb->dev = &pdev->dev; > > pb->enabled = false; > > > > - pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable", > > - GPIOD_OUT_HIGH); > > + pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable"); > > Please actually test your patches. This change here won't compile. > > Sascha > I will send patch v2 to fix it. YH Huang
On Fri, 2015-10-16 at 10:31 +0200, Lucas Stach wrote: > Am Freitag, den 16.10.2015, 09:37 +0800 schrieb YH Huang: > > In order to match the panel power sequence, disable the enable_gpio > > in the probe function. Also, reorder the code in the power_on and > > power_off function to match the timing. > > > You aren't specifying which panels power sequence you are matching here. > Are you sure you aren't breaking other panels with this patch? > > Regards, > Lucas The panel sequence is: When powering on the panel, generate pwm signals fist and then enable it to show the backlight. When powering off the panel, do it opposite. In probe function, we keep the panel status from bootloader and don't enable or disable it by default. Do these changes break other panels? Regards, YH Huang > > > Signed-off-by: YH Huang <yh.huang@mediatek.com> > > --- > > drivers/video/backlight/pwm_bl.c | 15 +++++++++------ > > 1 file changed, 9 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c > > index eff379b..99eca1e 100644 > > --- a/drivers/video/backlight/pwm_bl.c > > +++ b/drivers/video/backlight/pwm_bl.c > > @@ -54,10 +54,11 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness) > > if (err < 0) > > dev_err(pb->dev, "failed to enable power supply\n"); > > > > + pwm_enable(pb->pwm); > > + > > if (pb->enable_gpio) > > gpiod_set_value(pb->enable_gpio, 1); > > > > - pwm_enable(pb->pwm); > > pb->enabled = true; > > } > > > > @@ -66,12 +67,12 @@ static void pwm_backlight_power_off(struct pwm_bl_data *pb) > > if (!pb->enabled) > > return; > > > > - pwm_config(pb->pwm, 0, pb->period); > > - pwm_disable(pb->pwm); > > - > > if (pb->enable_gpio) > > gpiod_set_value(pb->enable_gpio, 0); > > > > + pwm_config(pb->pwm, 0, pb->period); > > + pwm_disable(pb->pwm); > > + > > regulator_disable(pb->power_supply); > > pb->enabled = false; > > } > > @@ -241,8 +242,7 @@ static int pwm_backlight_probe(struct platform_device *pdev) > > pb->dev = &pdev->dev; > > pb->enabled = false; > > > > - pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable", > > - GPIOD_OUT_HIGH); > > + pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable"); > > if (IS_ERR(pb->enable_gpio)) { > > ret = PTR_ERR(pb->enable_gpio); > > goto err_alloc; > > @@ -264,6 +264,9 @@ static int pwm_backlight_probe(struct platform_device *pdev) > > pb->enable_gpio = gpio_to_desc(data->enable_gpio); > > } > > > > + if (pb->enable_gpio) > > + gpiod_direction_output(pb->enable_gpio, 0); > > + > > pb->power_supply = devm_regulator_get(&pdev->dev, "power"); > > if (IS_ERR(pb->power_supply)) { > > ret = PTR_ERR(pb->power_supply); >
diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c index eff379b..99eca1e 100644 --- a/drivers/video/backlight/pwm_bl.c +++ b/drivers/video/backlight/pwm_bl.c @@ -54,10 +54,11 @@ static void pwm_backlight_power_on(struct pwm_bl_data *pb, int brightness) if (err < 0) dev_err(pb->dev, "failed to enable power supply\n"); + pwm_enable(pb->pwm); + if (pb->enable_gpio) gpiod_set_value(pb->enable_gpio, 1); - pwm_enable(pb->pwm); pb->enabled = true; } @@ -66,12 +67,12 @@ static void pwm_backlight_power_off(struct pwm_bl_data *pb) if (!pb->enabled) return; - pwm_config(pb->pwm, 0, pb->period); - pwm_disable(pb->pwm); - if (pb->enable_gpio) gpiod_set_value(pb->enable_gpio, 0); + pwm_config(pb->pwm, 0, pb->period); + pwm_disable(pb->pwm); + regulator_disable(pb->power_supply); pb->enabled = false; } @@ -241,8 +242,7 @@ static int pwm_backlight_probe(struct platform_device *pdev) pb->dev = &pdev->dev; pb->enabled = false; - pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable", - GPIOD_OUT_HIGH); + pb->enable_gpio = devm_gpiod_get_optional(&pdev->dev, "enable"); if (IS_ERR(pb->enable_gpio)) { ret = PTR_ERR(pb->enable_gpio); goto err_alloc; @@ -264,6 +264,9 @@ static int pwm_backlight_probe(struct platform_device *pdev) pb->enable_gpio = gpio_to_desc(data->enable_gpio); } + if (pb->enable_gpio) + gpiod_direction_output(pb->enable_gpio, 0); + pb->power_supply = devm_regulator_get(&pdev->dev, "power"); if (IS_ERR(pb->power_supply)) { ret = PTR_ERR(pb->power_supply);
In order to match the panel power sequence, disable the enable_gpio in the probe function. Also, reorder the code in the power_on and power_off function to match the timing. Signed-off-by: YH Huang <yh.huang@mediatek.com> --- drivers/video/backlight/pwm_bl.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)