> -----Original Message----- > From: Jean Delvare [mailto:khali@xxxxxxxxxxxx] > Sent: Monday, September 13, 2010 7:58 PM > To: Datta, Shubhrajyoti > Cc: lm-sensors@xxxxxxxxxxxxxx > Subject: Re: [RFC PATCH] Allow the configuration register to > be written > > On Mon, 13 Sep 2010 19:37:47 +0530, Shubhrajyoti D wrote: > > The patch intends to allow the configuration of resolution > > polarity etc. > > > > Signed-off-by: Shubhrajyoti D <shubhrajyoti@xxxxxx> > > --- > > drivers/hwmon/lm75.c | 35 +++++++++++++++++++++++++++++++++++ > > 1 files changed, 35 insertions(+), 0 deletions(-) > > > > diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c > > index 2a5b12a..abf3135 100644 > > --- a/drivers/hwmon/lm75.c > > +++ b/drivers/hwmon/lm75.c > > @@ -111,7 +111,41 @@ static ssize_t set_temp(struct device *dev, struct > device_attribute *da, > > mutex_unlock(&data->update_lock); > > return count; > > } > > +static ssize_t set_configuration(struct device *dev, > > + struct device_attribute *attr, > > + const char *buf, size_t count) > > +{ > > + s32 status; > > + struct i2c_client *client = to_i2c_client(dev); > > + struct lm75_data *data = i2c_get_clientdata(client); > > + unsigned long temp; > > + int error; > > + error = strict_strtol(buf, 10, &temp); > > + if (error) > > + return error; > > + > > + data->orig_conf = (u8)temp; > > + /* I2C write to the configuration register */ > > + status = lm75_write_value(client, LM75_REG_CONF, > > + data->orig_conf); > > + return count; > > +} > > + > > +static ssize_t show_configuration(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + struct i2c_client *client = to_i2c_client(dev); > > + int status = lm75_read_value(client, LM75_REG_CONF); > > + if (status < 0) { > > + dev_dbg(&client->dev, "Can't read config? %d\n", status); > > + return status; > > + } > > + > > + return sprintf(buf, "%u\n", status); > > +} > > > > +static SENSOR_DEVICE_ATTR(configuration, S_IWUSR | S_IRUGO, > > + show_configuration, set_configuration, 0); > > static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, > > show_temp, set_temp, 1); > > static SENSOR_DEVICE_ATTR(temp1_max_hyst, S_IWUSR | S_IRUGO, > > @@ -122,6 +156,7 @@ static struct attribute *lm75_attributes[] = { > > &sensor_dev_attr_temp1_input.dev_attr.attr, > > &sensor_dev_attr_temp1_max.dev_attr.attr, > > &sensor_dev_attr_temp1_max_hyst.dev_attr.attr, > > + &sensor_dev_attr_configuration.dev_attr.attr, > > > > NULL > > }; > > Massive NACK. You are exposing a raw register interface to user-space, > in a way which can't be standardized. This goes against the philosophy > of our standard sysfs interface. > > If you have an LM75-style device in an embedded device, configuration > should be provided as platform data. For other cases, either the > BIOS/firmware should configure the device properly, or you can do it > with i2c-dev + i2cset from user-space. Ok makes sense. > > If any properly really needs to be exposed though sysfs in your > opinion, it must be standardized first and stick to the one file, one > feature philosophy. I chose it as a sysfs because ideally the resolution/time is a tradeoff applications are better suited and not board dependent. However agree to your "one file, one feature" comment. Was wondering if any other driver has similar interface or sysfs entry(resolution). > > > -- > Jean Delvare _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
