On Wed, Feb 22, 2012 at 05:17:13PM -0800, Guenter Roeck wrote: > On Wed, Feb 22, 2012 at 05:18:45PM -0500, Nikolaus Schulz wrote: > > For the F75387, the register holding the current PWM duty cycle value is > > r/o; changing it requires writing to the fan expect register instead. > > > > Signed-off-by: Nikolaus Schulz <mail@xxxxxxxxxxxxxx> > > --- > > drivers/hwmon/f75375s.c | 27 +++++++++++++++++++++------ > > 1 files changed, 21 insertions(+), 6 deletions(-) > > > > diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c > > index 5dae122..fc6444d 100644 > > --- a/drivers/hwmon/f75375s.c > > +++ b/drivers/hwmon/f75375s.c > > @@ -308,8 +308,14 @@ static ssize_t set_pwm(struct device *dev, struct device_attribute *attr, > > return err; > > > > mutex_lock(&data->update_lock); > > + /* TODO: Improve representation in the register cache for the F75387 */ > > Is this comment needed ? If not, I would prefer not to add it as comment in a bug fix > at this point in the release cycle. Yeah, in fact didn't feel good about that myself. :) I'll drop that. > > data->pwm[nr] = SENSORS_LIMIT(val, 0, 255); > > - f75375_write8(client, F75375_REG_FAN_PWM_DUTY(nr), data->pwm[nr]); > > + if (data->kind == f75387) > > + f75375_write16(client, F75375_REG_FAN_EXP(nr), > > + data->pwm[nr]); > > + else > > + f75375_write8(client, F75375_REG_FAN_PWM_DUTY(nr), > > + data->pwm[nr]); > > This is three times the same code. Can you move it into a separate function ? Right, I thought that making this a function involves passing so many variables around that it's not worth it, but it's indeed cleaner. I'll post an updated patch. Thanks for the review! Nikolaus _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
