Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

backlight: pwm_bl: Improve bootloader/kernel device handover

Currently there are (at least) two problems in the way pwm_bl starts
managing the enable_gpio pin. Both occur when the backlight is initially
off and the driver finds the pin not already in output mode and, as a
result, unconditionally switches it to output-mode and asserts the signal.

Problem 1: This could cause the backlight to flicker since, at this stage
in driver initialisation, we have no idea what the PWM and regulator are
doing (an unconfigured PWM could easily "rest" at 100% duty cycle).

Problem 2: This will cause us not to correctly honour the
post_pwm_on_delay (which also risks flickers).

Fix this by moving the code to configure the GPIO output mode until after
we have examines the handover state. That allows us to initialize
enable_gpio to off if the backlight is currently off and on if the
backlight is on.

Cc: stable@vger.kernel.org
Reported-by: Marek Vasut <marex@denx.de>
Signed-off-by: Daniel Thompson <daniel.thompson@linaro.org>
Acked-by: Marek Vasut <marex@denx.de>
Tested-by: Marek Vasut <marex@denx.de>
Signed-off-by: Lee Jones <lee.jones@linaro.org>

authored by

Daniel Thompson and committed by
Lee Jones
79fad92f daa37361

+28 -26
+28 -26
drivers/video/backlight/pwm_bl.c
··· 409 409 static int pwm_backlight_initial_power_state(const struct pwm_bl_data *pb) 410 410 { 411 411 struct device_node *node = pb->dev->of_node; 412 + bool active = true; 413 + 414 + /* 415 + * If the enable GPIO is present, observable (either as input 416 + * or output) and off then the backlight is not currently active. 417 + * */ 418 + if (pb->enable_gpio && gpiod_get_value_cansleep(pb->enable_gpio) == 0) 419 + active = false; 420 + 421 + if (!regulator_is_enabled(pb->power_supply)) 422 + active = false; 423 + 424 + if (!pwm_is_enabled(pb->pwm)) 425 + active = false; 426 + 427 + /* 428 + * Synchronize the enable_gpio with the observed state of the 429 + * hardware. 430 + */ 431 + if (pb->enable_gpio) 432 + gpiod_direction_output(pb->enable_gpio, active); 433 + 434 + /* 435 + * Do not change pb->enabled here! pb->enabled essentially 436 + * tells us if we own one of the regulator's use counts and 437 + * right now we do not. 438 + */ 412 439 413 440 /* Not booted with device tree or no phandle link to the node */ 414 441 if (!node || !node->phandle) ··· 447 420 * assume that another driver will enable the backlight at the 448 421 * appropriate time. Therefore, if it is disabled, keep it so. 449 422 */ 450 - 451 - /* if the enable GPIO is disabled, do not enable the backlight */ 452 - if (pb->enable_gpio && gpiod_get_value_cansleep(pb->enable_gpio) == 0) 453 - return FB_BLANK_POWERDOWN; 454 - 455 - /* The regulator is disabled, do not enable the backlight */ 456 - if (!regulator_is_enabled(pb->power_supply)) 457 - return FB_BLANK_POWERDOWN; 458 - 459 - /* The PWM is disabled, keep it like this */ 460 - if (!pwm_is_enabled(pb->pwm)) 461 - return FB_BLANK_POWERDOWN; 462 - 463 - return FB_BLANK_UNBLANK; 423 + return active ? FB_BLANK_UNBLANK: FB_BLANK_POWERDOWN; 464 424 } 465 425 466 426 static int pwm_backlight_probe(struct platform_device *pdev) ··· 499 485 ret = PTR_ERR(pb->enable_gpio); 500 486 goto err_alloc; 501 487 } 502 - 503 - /* 504 - * If the GPIO is not known to be already configured as output, that 505 - * is, if gpiod_get_direction returns either 1 or -EINVAL, change the 506 - * direction to output and set the GPIO as active. 507 - * Do not force the GPIO to active when it was already output as it 508 - * could cause backlight flickering or we would enable the backlight too 509 - * early. Leave the decision of the initial backlight state for later. 510 - */ 511 - if (pb->enable_gpio && 512 - gpiod_get_direction(pb->enable_gpio) != 0) 513 - gpiod_direction_output(pb->enable_gpio, 1); 514 488 515 489 pb->power_supply = devm_regulator_get(&pdev->dev, "power"); 516 490 if (IS_ERR(pb->power_supply)) {