Re: [PATCH] leds-lm3530: replace pwm platform functions with generic pwm functions

[Thierry Reding <thierry.reding@avionic-design.de>]

[-- Attachment #1: Type: text/plain, Size: 1939 bytes --]

On Mon, Aug 20, 2012 at 04:02:05AM +0000, Kim, Milo wrote:
> use generic pwm functions for changing the duty rather than the platform data

pwm -> PWM, duty -> duty-cycle

There are more occurrences of "pwm"scattered through the file, please
fix those as well.

> @@ -153,6 +158,64 @@ static int lm3530_get_mode_from_str(const char *str)
>  	return -1;
>  }
>  
> +#ifdef CONFIG_PWM
> +static int lm3530_pwm_request(struct lm3530_data *drvdata)
> +{
> +	int pwm_id;
> +
> +	/* if the pwm device exists, skip requesting the device */
> +	if (drvdata->pwm)
> +		return 0;
> +
> +	pwm_id = drvdata->pdata ? drvdata->pdata->pwm_id : 0;
> +
> +	drvdata->pwm = pwm_request(pwm_id, "lm3530-pwm");

Please don't use pwm_id and pwm_request() for new code. You should be
using pwm_get() along with a corresponding PWM_LOOKUP() entry.

> +	drvdata->period_ns = drvdata->pdata ? drvdata->pdata->period_ns : 0;
> +
> +	return IS_ERR(drvdata->pwm) ? PTR_ERR(drvdata->pwm) : 0;

This is "PTR_RET(drvdata->pwm)".

> +}
> +
> +static void lm3530_pwm_set(struct lm3530_data *drvdata,
> +			int brightness, int max_brightness)
> +{
> +	struct pwm_device *pwm = drvdata->pwm;
> +
> +	if (pwm) {

Maybe this check should be:

	if (pwm && drvdata->period_ns > 0)

> @@ -363,9 +418,15 @@ static ssize_t lm3530_mode_set(struct device *dev, struct device_attribute
>  
>  	drvdata->mode = mode;
>  
> -	/* set pwm to low if unnecessary */
> -	if (mode != LM3530_BL_MODE_PWM && pwm->pwm_set_intensity)
> -		pwm->pwm_set_intensity(0, max_brightness);
> +	/* if new mode is pwm, then request a pwm device.
> +	   otherwise, set pwm to low */
> +	if (mode == LM3530_BL_MODE_PWM) {
> +		err = lm3530_pwm_request(drvdata);
> +		if (err)
> +			return err;
> +	} else {
> +		lm3530_pwm_set(drvdata, 0, max_brightness);
> +	}

Don't you need to free the PWM if mode is set to !LM3530_BL_MODE_PWM?

Thierry

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]