On Thu, 2012-02-23 at 14:40 -0500, Nikolaus Schulz wrote: > On Wed, Feb 22, 2012 at 05:35:12PM -0800, Guenter Roeck wrote: > > On Wed, Feb 22, 2012 at 05:18:49PM -0500, Nikolaus Schulz wrote: > > > It makes no sense to attempt to manually configure the fan in auto mode, > > > or set the duty cycle directly in closed loop mode. The corresponding > > > registers are then read-only. If the user tries it nonetheless, error out > > > with EBUSY instead of silently doing nothing. > > > > > > Signed-off-by: Nikolaus Schulz <mail@xxxxxxxxxxxxxx> > > > --- > > > drivers/hwmon/f75375s.c | 32 ++++++++++++++++++++++++++++++++ > > > 1 files changed, 32 insertions(+), 0 deletions(-) > > > > > > diff --git a/drivers/hwmon/f75375s.c b/drivers/hwmon/f75375s.c > > > index 29b11c6..0e7fe3a 100644 > > > --- a/drivers/hwmon/f75375s.c > > > +++ b/drivers/hwmon/f75375s.c > > > @@ -281,6 +281,21 @@ static bool duty_mode_enabled(u8 pwm_enable) > > > } > > > } > > > > > > +static bool auto_mode_enabled(u8 pwm_enable) > > > +{ > > > + switch (pwm_enable) { > > > + case 0: /* Manual, duty mode (full speed) */ > > > + case 1: /* Manual, duty mode */ > > > + case 3: /* Manual, speed mode */ > > > + return false; > > > + case 2: /* Auto, speed mode */ > > > + case 4: /* Auto, duty mode */ > > > + return true; > > > + default: > > > + BUG(); > > > + } > > > +} > > > + > > > static ssize_t set_fan_min(struct device *dev, struct device_attribute *attr, > > > const char *buf, size_t count) > > > { > > > @@ -314,6 +329,11 @@ static ssize_t set_fan_target(struct device *dev, struct device_attribute *attr, > > > if (err < 0) > > > return err; > > > > > > + if (auto_mode_enabled(data->pwm_enable[nr])) > > > + return -EBUSY; > > > + if (data->kind == f75387 && duty_mode_enabled(data->pwm_enable[nr])) > > > + return -EBUSY; > > > + > > -EBUSY is really not appropriate here. -EINVAL would be better, though I am open to suggestions. > > I am not very happy about using EINVAL here, but if EBUSY is > inappropriate, I don't have a better idea. > EBUSY - device or resource busy EINVAL - Invalid argument So EBUSY is definitely wrong, and EINVAL is a better match. It would be nice if there was an "Invalid in this context", or "Invalid with the current configuration", but I am not aware of such an error code. One could think of EOPNOTSUPP, but that means "Operation not supported on transport endpoint", which doesn't seem to be appropriate either. Thanks, Guenter _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors