On 09/23/2013 03:41 PM, Thierry Reding wrote:
> Many backlights require a power supply to work properly. This commit
> uses a power-supply regulator, if available, to power up and power down
> the panel.
I think that all backlights require a power supply, albeit the supply
may not be SW-controllable. Hence, shouldn't the regulator be mandatory
in the binding, yet the driver be defensively coded such that if one
isn't specified, the driver continues to work?
> diff --git a/drivers/video/backlight/pwm_bl.c b/drivers/video/backlight/pwm_bl.c
> @@ -253,6 +264,16 @@ static int pwm_backlight_probe(struct platform_device *pdev)
> }
> }
>
> + pb->power_supply = devm_regulator_get_optional(&pdev->dev, "power");
... so I think that should be devm_regulator_get(), since the regulator
isn't really optional.
> + if (IS_ERR(pb->power_supply)) {
> + if (PTR_ERR(pb->power_supply) != -ENODEV) {
> + ret = PTR_ERR(pb->power_supply);
> + goto err_gpio;
> + }
> +
> + pb->power_supply = NULL;
If devm_regulator_get_optional() returns an error value or a valid
value, then I don't think that this driver should transmute error values
into NULL; NULL might be a perfectly valid regulator value. Related, I
think the if (pb->power_supply) tests should be replaced with if
(!IS_ERR(pb->power_supply)) instead.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
