On Wed, Mar 06, 2013 at 09:17:18AM -0800, Andrew Chew wrote:
[...]
> diff --git a/drivers/video/backlight/pwm_bl.c
> b/drivers/video/backlight/pwm_bl.c
> index 069983c..ff98cdd 100644
> --- a/drivers/video/backlight/pwm_bl.c
> +++ b/drivers/video/backlight/pwm_bl.c
> @@ -20,10 +20,13 @@
> #include <linux/pwm.h>
> #include <linux/pwm_backlight.h>
> #include <linux/slab.h>
> +#include <linux/regulator/consumer.h>
>
> struct pwm_bl_data {
> struct pwm_device *pwm;
> struct device *dev;
> + struct regulator *en_supply;
Can this be renamed to enable_supply to match the DT property name?
> + bool en_supply_enabled;
This boolean can be dropped. As discussed in a previous thread, the
pwm-backlight driver shouldn't need to know about any other uses of the
regulator.
> +static void pwm_backlight_enable(struct backlight_device *bl)
> +{
> + struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
> +
> + pwm_enable(pb->pwm);
> +
> + if (pb->en_supply && !pb->en_supply_enabled) {
> + if (regulator_enable(pb->en_supply) != 0)
> + dev_warn(&bl->dev, "Failed to enable regulator");
> + else
> + pb->en_supply_enabled = true;
> + }
> +}
> +
> +static void pwm_backlight_disable(struct backlight_device *bl)
> +{
> + struct pwm_bl_data *pb = dev_get_drvdata(&bl->dev);
> +
> + if (pb->en_supply && pb->en_supply_enabled) {
> + if (regulator_disable(pb->en_supply) != 0)
> + dev_warn(&bl->dev, "Failed to disable regulator");
> + else
> + pb->en_supply_enabled = false;
> + }
> +
> + pwm_disable(pb->pwm);
> +}
Alex already brought this up: shouldn't the sequences be reversed. That
is, when enabling the backlight, turn on the regulator first, then
enable the PWM. When disabling, disable the PWM first, then turn off the
regulator?
> @@ -213,6 +238,13 @@ static int pwm_backlight_probe(struct platform_device
> *pdev)
> pb->exit = data->exit;
> pb->dev = &pdev->dev;
>
> + pb->en_supply = devm_regulator_get(&pdev->dev, "enable");
> + if (IS_ERR(pb->en_supply)) {
> + ret = PTR_ERR(pb->en_supply);
> + pb->en_supply = NULL;
> + goto err_alloc;
> + }
This effectively makes the regulator mandatory, so the board files that
use pwm-backlight need to be updated or otherwise will break.
Thierry
pgpoh9e9w6I_4.pgp
Description: PGP signature
