Messages in this thread Patch in this message | | | Date | Mon, 20 Aug 2012 07:04:52 +0200 | From | Thierry Reding <> | Subject | Re: [PATCH] leds-lm3530: replace pwm platform functions with generic pwm functions |
| |
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 [unhandled content-type:application/pgp-signature] | |