On Mon, Dec 10, 2012 at 11:00:37AM +0100, Peter Ujfalusi wrote:
[...]
> +LED sub-node properties:
> +- pwms : PWM property, please refer to:
> + Documentation/devicetree/bindings/pwm/pwm.txt
Instead of only referring to the generic PWM binding document, this
should probably explain what the PWM device is used for.
> +err:
> + if (priv->num_leds > 0) {
> + for (count = priv->num_leds - 1; count >= 0; count--) {
> + led_classdev_unregister(&priv->leds[count].cdev);
> + pwm_put(priv->leds[count].pwm);
> + }
> + }
Can this not be written more simply as follows?
while (priv->num_leds--) {
...
}
> static int led_pwm_remove(struct platform_device *pdev)
> {
> + struct led_pwm_platform_data *pdata = pdev->dev.platform_data;
> struct led_pwm_priv *priv = platform_get_drvdata(pdev);
> int i;
>
> - for (i = 0; i < priv->num_leds; i++)
> + for (i = 0; i < priv->num_leds; i++) {
> led_classdev_unregister(&priv->leds[i].cdev);
> + if (!pdata)
> + pwm_put(priv->leds[i].pwm);
> + }
Perhaps while at it we can add devm_of_pwm_get() along with exporting
of_pwm_get() so that you don't have to special-case this?
> +static const struct of_device_id of_pwm_leds_match[] = {
> + { .compatible = "pwm-leds", },
> + {},
> +};
Doesn't this cause a compiler warning for !OF builds?
Thierry
Attachment:
pgpIFrEbEb9hH.pgp
Description: PGP signature
