From mboxrd@z Thu Jan 1 00:00:00 1970 From: Guenter Roeck Date: Thu, 30 Jul 2015 02:41:26 +0000 Subject: Re: [lm-sensors] [PATCH] hwmon: Driver for Maxim MAX31790 Message-Id: <55B98ED6.7010506@roeck-us.net> List-Id: References: <1438195783-26978-1-git-send-email-corone.il.han@gmail.com> In-Reply-To: <1438195783-26978-1-git-send-email-corone.il.han@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: lm-sensors@vger.kernel.org Hi, On 07/29/2015 11:49 AM, Il Han wrote: > The driver supports the Maxim MAX31790. > > Signed-off-by: Il Han Can you provide the output of i2cdump for this chip ? It will help module testing the driver. Further comments below. Thanks, Guenter > --- > drivers/hwmon/Kconfig | 9 + > drivers/hwmon/Makefile | 1 + > drivers/hwmon/max31790.c | 806 +++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 816 insertions(+) > create mode 100644 drivers/hwmon/max31790.c > > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index 7c65b73..fcd1c93 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -840,6 +840,15 @@ config SENSORS_MAX6697 > This driver can also be built as a module. If so, the module > will be called max6697. > > +config SENSORS_MAX31790 > + tristate "Maxim MAX31790 sensor chip" > + depends on I2C > + help > + If you say yes here you get support for the MAX31790 sensor chips. > + > + This driver can also be built as a module. If so, the module > + will be called max31790. > + > config SENSORS_HTU21 > tristate "Measurement Specialties HTU21D humidity/temperature sensors" > depends on I2C > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index 9e0f3dd..01855ee 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -115,6 +115,7 @@ obj-$(CONFIG_SENSORS_MAX6639) += max6639.o > obj-$(CONFIG_SENSORS_MAX6642) += max6642.o > obj-$(CONFIG_SENSORS_MAX6650) += max6650.o > obj-$(CONFIG_SENSORS_MAX6697) += max6697.o > +obj-$(CONFIG_SENSORS_MAX31790) += max31790.o > obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o > obj-$(CONFIG_SENSORS_MCP3021) += mcp3021.o > obj-$(CONFIG_SENSORS_MENF21BMC_HWMON) += menf21bmc_hwmon.o > diff --git a/drivers/hwmon/max31790.c b/drivers/hwmon/max31790.c > new file mode 100644 > index 0000000..8c4ff08 > --- /dev/null > +++ b/drivers/hwmon/max31790.c > @@ -0,0 +1,806 @@ > +/* > + * max31790.c - Part of lm_sensors, Linux kernel modules for hardware > + * monitoring. > + * > + * (C) 2015 by Il Han > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + * > + * This program is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + Please list include files in alphabetic order. Not strictly required, but reduces potential conflicts later on. > +enum chips { max31790 }; Not needed; the driver only supports a single chip. > + > +/* MAX31790 registers */ > +#define MAX31790_REG_GLOBAL_CONFIG 0x00 > +#define MAX31790_REG_PWM_FREQ 0x01 Not used. Please drop or implement pwmX_freq. > +#define MAX31790_REG_FAN_CONFIG(ch) (0x02 + (ch)) > +#define MAX31790_REG_FAN_DYNAMICS(ch) (0x08 + (ch)) > +#define MAX31790_REG_FAN_FAULT_STATUS2 0x10 > +#define MAX31790_REG_FAN_FAULT_STATUS1 0x11 > +#define MAX31790_REG_FAN_FAULT_MASK2 0x12 > +#define MAX31790_REG_FAN_FAULT_MASK1 0x13 > +#define MAX31790_REG_TACH_COUNT(ch) (0x18 + (ch) * 2) > +#define MAX31790_REG_PWM_DUTY_CYCLE(ch) (0x30 + (ch) * 2) > +#define MAX31790_REG_PWMOUT(ch) (0x40 + (ch) * 2) > +#define MAX31790_REG_TARGET_COUNT(ch) (0x50 + (ch) * 2) > +#define MAX31790_REG_WINDOW(ch) (0x60 + (ch)) > + > +/* Global Config register bits */ > +#define MAX31790_GL_CFG_STANDBY 0x80 > +#define MAX31790_GL_CFG_RESET 0x40 > +#define MAX31790_GL_CFG_BUS_TIMEOUT 0x20 > +#define MAX31790_GL_CFG_OSC 0x08 > +#define MAX31790_GL_CFG_WD_SHIFT 1 > +#define MAX31790_GL_CFG_WD_MASK 0x06 > +#define MAX31790_GL_CFG_WD_DISABLE 0x00 > +#define MAX31790_GL_CFG_WD_5S 0x02 > +#define MAX31790_GL_CFG_WD_10S 0x04 > +#define MAX31790_GL_CFG_WD_30S 0x06 > +#define MAX31790_GL_CFG_WD_STATUS 0x01 > + > +/* Fan Config register bits */ > +#define MAX31790_FAN_CFG_MODE_MASK 0x80 > +#define MAX31790_FAN_CFG_PWM_MODE 0x00 > +#define MAX31790_FAN_CFG_RPM_MODE 0x80 > +#define MAX31790_FAN_CFG_MONITOR_ONLY 0x10 > +#define MAX31790_FAN_CFG_TACH_INPUT_EN 0x08 > +#define MAX31790_FAN_CFG_LOCKED_ROTOR 0x04 > +#define MAX31790_FAN_CFG_TACH_INPUT 0x01 > + > +/* Fan Dynamics register bits */ > +#define MAX31790_FAN_DYNAMICS_SR(reg) (((reg) >> 5) & 0x7) > + > +/* number of tachometer pulses per revolution */ > +#define MAX31790_TACHOMETER_PULSE 2 > + > +#define NR_CHANNEL 6 > + > +#define FAN_RPM_MIN 480 > +#define FAN_RPM_MAX 30000 Where do those values come from ? > + > +/* > + * Client data (each client gets its own) > + */ > +struct max31790_data { > + struct i2c_client *client; > + struct mutex update_lock; > + char valid; /* zero until following fields are valid */ Please use bool. > + unsigned long last_updated; /* in jiffies */ > + > + /* register values */ > + u8 global_config; > + u8 fan_config[NR_CHANNEL]; > + u8 fan_dynamics[NR_CHANNEL]; > + u8 fault_status2; > + u8 fault_status1; > + u8 fault_mask2; > + u8 fault_mask1; > + u16 tach[NR_CHANNEL]; > + u16 pwm[NR_CHANNEL]; > + u16 target_count[NR_CHANNEL]; > +}; > + > +static struct max31790_data *max31790_update_device(struct device *dev) > +{ > + struct max31790_data *data = dev_get_drvdata(dev); > + struct i2c_client *client = data->client; > + int i; > + > + mutex_lock(&data->update_lock); > + > + if (time_after(jiffies, data->last_updated + HZ) || !data->valid) { > + data->global_config = i2c_smbus_read_byte_data(client, > + MAX31790_REG_GLOBAL_CONFIG); > + data->fault_status1 = i2c_smbus_read_byte_data(client, > + MAX31790_REG_FAN_FAULT_STATUS1); > + data->fault_mask1 = i2c_smbus_read_byte_data(client, > + MAX31790_REG_FAN_FAULT_MASK1); > + for (i = 0; i < NR_CHANNEL; i++) { > + data->fan_config[i] = i2c_smbus_read_byte_data(client, > + MAX31790_REG_FAN_CONFIG(i)); > + data->fan_dynamics[i] = i2c_smbus_read_byte_data(client, > + MAX31790_REG_FAN_DYNAMICS(i)); > + data->tach[i] = swab16(i2c_smbus_read_word_data(client, > + MAX31790_REG_TACH_COUNT(i))); Consider using i2c_smbus_read_word_data_swapped(). > + data->pwm[i] = swab16(i2c_smbus_read_word_data(client, > + MAX31790_REG_PWMOUT(i))); > + data->target_count[i] > + swab16(i2c_smbus_read_word_data(client, > + MAX31790_REG_TARGET_COUNT(i))); Several of those attributes do not change values and can be cached continuously, ie only have to be read once. > + } > + > + data->last_updated = jiffies; > + data->valid = 1; > + } > + > + mutex_unlock(&data->update_lock); > + > + return data; > +} > + > +int get_speed_range(unsigned char fan_dynamics) > +{ > + int sr; > + > + switch (MAX31790_FAN_DYNAMICS_SR(fan_dynamics)) { > + case 0x0: > + sr = 1; > + break; > + case 0x1: > + sr = 2; > + break; > + case 0x2: > + sr = 4; > + break; > + case 0x3: > + sr = 8; > + break; > + case 0x4: > + sr = 16; > + break; > + case 0x5: > + case 0x6: > + case 0x7: > + sr = 32; > + break; > + default: > + return -EINVAL; > + } > + Given that the sr range is 3 bit, an array would be simpler. static const u8 sr[8] = { 1, 2, 4, 8, 16, 32, 32, 32 }; u8 get_speed_range(u8 fan_dynamics) { return sr[MAX31790_FAN_DYNAMICS_SR(fan_dynamics)]; } It might be useful to report the configured speed range as fanX_div, and make it configurable. > + return sr; > +} > + > +static ssize_t get_fan(struct device *dev, > + struct device_attribute *devattr, char *buf) Please use standard continuation line alignment (align with '(' on the previous line). > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct max31790_data *data = max31790_update_device(dev); > + int np = MAX31790_TACHOMETER_PULSE; > + int sr = get_speed_range(data->fan_dynamics[attr->index]); > + int rpm; > + > + rpm = (60 * sr * 8192) / (np * (data->tach[attr->index] >> 5)); You use direct values for everything but the pulse count. Might as well just use '2' for that as well. It is quite useless, though, since (2 * (data->tach[attr->index] >> 5)) is identical to (data->tach[attr->index] >> 4) which you might as well use directly. I would suggest to perform the calculation in a separate function, such as rmp_from_reg() and rpm_to_reg(), to avoid repetition in get_fan() and {get,set}_fan_target(). > + return sprintf(buf, "%d\n", rpm); > +} > + > +static ssize_t get_fan_target(struct device *dev, > + struct device_attribute *devattr, char *buf) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct max31790_data *data = max31790_update_device(dev); > + int np = MAX31790_TACHOMETER_PULSE; > + int sr = get_speed_range(data->fan_dynamics[attr->index]); > + int rpm; > + > + rpm = (60 * sr * 8192) / (np * (data->target_count[attr->index] >> 5)); > + > + return sprintf(buf, "%d\n", rpm); > +} > + > +static ssize_t set_fan_target(struct device *dev, > + struct device_attribute *devattr, const char *buf, size_t count) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct i2c_client *client = to_i2c_client(dev); > + struct max31790_data *data = i2c_get_clientdata(client); > + int np = MAX31790_TACHOMETER_PULSE; > + int sr; > + unsigned long rpm; > + int target_count; > + int err; > + > + err = kstrtoul(buf, 10, &rpm); > + if (err) > + return err; > + > + mutex_lock(&data->update_lock); > + > + rpm = clamp_val(rpm, FAN_RPM_MIN, FAN_RPM_MAX); > + sr = get_speed_range(data->fan_dynamics[attr->index]); > + target_count = (60 * sr * 8192) / (np * rpm); > + target_count = clamp_val(target_count, 0x0, 0x7FF); > + > + data->target_count[attr->index] = (u16)target_count << 5; > + > + i2c_smbus_write_word_data(client, > + MAX31790_REG_TARGET_COUNT(attr->index), > + swab16(data->target_count[attr->index])); > + > + mutex_unlock(&data->update_lock); > + > + return count; > +} > + > +static ssize_t get_pwm(struct device *dev, > + struct device_attribute *devattr, char *buf) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct max31790_data *data = max31790_update_device(dev); > + int pwm; > + > + pwm = data->pwm[attr->index] >> 7; > + > + return sprintf(buf, "%d\n", pwm); > +} > + > +static ssize_t set_pwm(struct device *dev, > + struct device_attribute *devattr, const char *buf, size_t count) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct i2c_client *client = to_i2c_client(dev); > + struct max31790_data *data = i2c_get_clientdata(client); > + unsigned long pwm; > + int err; > + > + err = kstrtoul(buf, 10, &pwm); > + if (err) > + return err; > + > + pwm = clamp_val(pwm, 0x0, 0x1FF); > + > + mutex_lock(&data->update_lock); > + data->pwm[attr->index] = (u16)pwm << 7; > + i2c_smbus_write_word_data(client, > + MAX31790_REG_PWMOUT(attr->index), > + swab16(data->pwm[attr->index])); > + > + mutex_unlock(&data->update_lock); > + > + return count; > +} > + Valid range for pwmX is 0..255. Please report this range and adjust reported / configured values accordingly. > +static ssize_t get_reset(struct device *dev, > + struct device_attribute *devattr, char *buf) > +{ > + struct max31790_data *data = max31790_update_device(dev); > + int reset; > + > + if (data->global_config & MAX31790_GL_CFG_RESET) > + reset = 1; > + else > + reset = 0; > + > + return sprintf(buf, "%d\n", reset); > +} > + > +static ssize_t set_reset(struct device *dev, > + struct device_attribute *devattr, const char *buf, size_t count) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct max31790_data *data = i2c_get_clientdata(client); > + unsigned long reset; > + int err; > + > + err = kstrtoul(buf, 10, &reset); > + if (err) > + return err; > + > + mutex_lock(&data->update_lock); > + > + if (reset) > + data->global_config |= MAX31790_GL_CFG_RESET; > + else > + data->global_config &= ~MAX31790_GL_CFG_RESET; > + > + i2c_smbus_write_byte_data(client, > + MAX31790_REG_GLOBAL_CONFIG, data->global_config); > + > + mutex_unlock(&data->update_lock); > + > + return count; > +} There should be no reason to expose this configuration bit. It is not common to implement chip resets this way. Please drop. > + > +static ssize_t get_standby(struct device *dev, > + struct device_attribute *devattr, char *buf) > +{ > + struct max31790_data *data = max31790_update_device(dev); > + int standby; > + > + if (data->global_config & MAX31790_GL_CFG_STANDBY) > + standby = 1; > + else > + standby = 0; > + > + return sprintf(buf, "%d\n", standby); > +} > + > +static ssize_t set_standby(struct device *dev, > + struct device_attribute *devattr, const char *buf, size_t count) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct max31790_data *data = i2c_get_clientdata(client); > + unsigned long standby; > + int err; > + > + err = kstrtoul(buf, 10, &standby); > + if (err) > + return err; > + > + mutex_lock(&data->update_lock); > + > + if (standby) > + data->global_config |= MAX31790_GL_CFG_STANDBY; > + else > + data->global_config &= ~MAX31790_GL_CFG_STANDBY; > + > + i2c_smbus_write_byte_data(client, > + MAX31790_REG_GLOBAL_CONFIG, data->global_config); > + > + mutex_unlock(&data->update_lock); > + > + return count; > +} Same here. If you want to support standby more, please use PM functions. > + > +static ssize_t get_bus_timeout(struct device *dev, > + struct device_attribute *devattr, char *buf) > +{ > + struct max31790_data *data = max31790_update_device(dev); > + int bus_timeout; > + > + if (data->global_config & MAX31790_GL_CFG_BUS_TIMEOUT) > + bus_timeout = 0; > + else > + bus_timeout = 1; > + > + return sprintf(buf, "%d\n", bus_timeout); > +} > + > +static ssize_t set_bus_timeout(struct device *dev, > + struct device_attribute *devattr, const char *buf, size_t count) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct max31790_data *data = i2c_get_clientdata(client); > + unsigned long bus_timeout; > + int err; > + > + err = kstrtoul(buf, 10, &bus_timeout); > + if (err) > + return err; > + > + mutex_lock(&data->update_lock); > + > + if (bus_timeout) > + data->global_config &= ~MAX31790_GL_CFG_BUS_TIMEOUT; > + else > + data->global_config |= MAX31790_GL_CFG_BUS_TIMEOUT; > + > + i2c_smbus_write_byte_data(client, > + MAX31790_REG_GLOBAL_CONFIG, data->global_config); > + > + mutex_unlock(&data->update_lock); > + > + return count; > +} This is, if at all, a driver configuration flag. Either use platform data, devicetree data, or a module parameter, but not a sysfs entry. > + > +static ssize_t get_watchdog(struct device *dev, > + struct device_attribute *devattr, char *buf) > +{ > + struct max31790_data *data = max31790_update_device(dev); > + int watchdog; > + > + watchdog = (data->global_config & MAX31790_GL_CFG_WD_MASK) > + >> MAX31790_GL_CFG_WD_SHIFT; > + > + return sprintf(buf, "%d\n", watchdog); > +} > + > +static ssize_t set_watchdog(struct device *dev, > + struct device_attribute *devattr, const char *buf, size_t count) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct max31790_data *data = i2c_get_clientdata(client); > + unsigned long watchdog; > + int err; > + > + err = kstrtoul(buf, 10, &watchdog); > + if (err) > + return err; > + > + mutex_lock(&data->update_lock); > + > + if (watchdog < 0x0 || watchdog > 0x3) > + return -EINVAL; > + > + data->global_config > + ((data->global_config & ~MAX31790_GL_CFG_WD_MASK) > + | ((watchdog << MAX31790_GL_CFG_WD_SHIFT) > + & MAX31790_GL_CFG_WD_MASK)); > + > + i2c_smbus_write_byte_data(client, > + MAX31790_REG_GLOBAL_CONFIG, data->global_config); > + > + mutex_unlock(&data->update_lock); > + > + return count; > +} Same here. I would sugegst to not make this (and the bus timeout bit) configurable at all, though, unless you have a good reason to do so. If some special configuration is needed, it is supposed to be set by the BIOS or ROM monitor. > + > +static ssize_t get_fan_mode(struct device *dev, > + struct device_attribute *devattr, char *buf) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct max31790_data *data = max31790_update_device(dev); > + int mode; > + > + if (data->fan_config[attr->index] & MAX31790_FAN_CFG_RPM_MODE) > + mode = 1; > + else > + mode = 0; > + > + return sprintf(buf, "%d\n", mode); > +} > + > +static ssize_t set_fan_mode(struct device *dev, > + struct device_attribute *devattr, const char *buf, size_t count) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct i2c_client *client = to_i2c_client(dev); > + struct max31790_data *data = i2c_get_clientdata(client); > + unsigned long mode; > + int err; > + > + err = kstrtoul(buf, 10, &mode); > + if (err) > + return err; > + > + mutex_lock(&data->update_lock); > + > + if (mode) > + data->fan_config[attr->index] > + (data->fan_config[attr->index] > + & ~MAX31790_FAN_CFG_MODE_MASK) > + | MAX31790_FAN_CFG_RPM_MODE; > + else > + data->fan_config[attr->index] > + (data->fan_config[attr->index] > + & ~MAX31790_FAN_CFG_MODE_MASK) > + | MAX31790_FAN_CFG_PWM_MODE; > + > + i2c_smbus_write_byte_data(client, > + MAX31790_REG_FAN_CONFIG(attr->index), > + data->fan_config[attr->index]); > + > + mutex_unlock(&data->update_lock); > + > + return count; > +} > + This is expected to be controlled with the pwmX_enable attribute. pwmX_enable = 0: disabled. In this case, I would assume set bit 4 to 1. pwmX_enable = 1: manual mode. Set bit 7 to 1. pwmX_enable = 2: rpm mode. Select desired fan speed with the fanX_target attribute. > +static ssize_t get_tach_enable(struct device *dev, > + struct device_attribute *devattr, char *buf) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct max31790_data *data = max31790_update_device(dev); > + int tach_enable; > + > + if (data->fan_config[attr->index] & MAX31790_FAN_CFG_TACH_INPUT_EN) > + tach_enable = 1; > + else > + tach_enable = 0; > + > + return sprintf(buf, "%d\n", tach_enable); > +} > + This is again a configuration flag and should not be exposed as sysfs attribute. > +static ssize_t set_tach_enable(struct device *dev, > + struct device_attribute *devattr, const char *buf, size_t count) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct i2c_client *client = to_i2c_client(dev); > + struct max31790_data *data = i2c_get_clientdata(client); > + unsigned long tach_enable; > + int err; > + > + err = kstrtoul(buf, 10, &tach_enable); > + if (err) > + return err; > + > + mutex_lock(&data->update_lock); > + > + if (tach_enable) > + data->fan_config[attr->index] > + |= MAX31790_FAN_CFG_TACH_INPUT_EN; > + else > + data->fan_config[attr->index] > + &= ~MAX31790_FAN_CFG_TACH_INPUT_EN; > + > + i2c_smbus_write_byte_data(client, > + MAX31790_REG_FAN_CONFIG(attr->index), > + data->fan_config[attr->index]); > + > + mutex_unlock(&data->update_lock); > + > + return count; > +} > + > +static ssize_t get_fan_fault(struct device *dev, > + struct device_attribute *devattr, char *buf) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct max31790_data *data = max31790_update_device(dev); > + int fault; > + > + if (data->fault_status1 & (1 << attr->index)) > + fault = 1; > + else > + fault = 0; > + fault = !!(data->fault_status1 & (1 << attr->index)); > + return sprintf(buf, "%d\n", fault); > +} > + > +static ssize_t set_fan_fault(struct device *dev, > + struct device_attribute *devattr, const char *buf, size_t count) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct i2c_client *client = to_i2c_client(dev); > + struct max31790_data *data = i2c_get_clientdata(client); > + unsigned long fault; > + int err; > + > + err = kstrtoul(buf, 10, &fault); > + if (err) > + return err; > + > + mutex_lock(&data->update_lock); > + > + if (fault) > + data->fault_status1 > + (data->fault_status1 | (1 << attr->index)); > + else > + data->fault_status1 > + (data->fault_status1 & ~(1 << attr->index)); > + > + i2c_smbus_write_byte_data(client, > + MAX31790_REG_FAN_FAULT_STATUS1, data->fault_status1); > + > + mutex_unlock(&data->update_lock); > + fanX_fault is suppsed to be read-only. There is no manual fault reset. The idea is that the flag is cleared on read; if the fault condition is still present, the fault bit will be set again. > + return count; > +} > + > +static ssize_t get_fan_mask(struct device *dev, > + struct device_attribute *devattr, char *buf) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct max31790_data *data = max31790_update_device(dev); > + int mask; > + > + if (data->fault_mask1 & (1 << attr->index)) > + mask = 1; > + else > + mask = 0; > + > + return sprintf(buf, "%d\n", mask); > +} > + > +static ssize_t set_fan_mask(struct device *dev, > + struct device_attribute *devattr, const char *buf, size_t count) > +{ > + struct sensor_device_attribute *attr = to_sensor_dev_attr(devattr); > + struct i2c_client *client = to_i2c_client(dev); > + struct max31790_data *data = i2c_get_clientdata(client); > + unsigned long mask; > + int err; > + > + err = kstrtoul(buf, 10, &mask); > + if (err) > + return err; > + > + mutex_lock(&data->update_lock); > + > + if (mask) > + data->fault_mask1 > + (data->fault_mask1 | (1 << attr->index)); > + else > + data->fault_mask1 > + (data->fault_mask1 & ~(1 << attr->index)); > + > + i2c_smbus_write_byte_data(client, > + MAX31790_REG_FAN_FAULT_MASK1, data->fault_mask1); > + > + mutex_unlock(&data->update_lock); > + > + return count; > +} Yet another set of configuration flags. > + > +static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, get_fan, NULL, 0); > +static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, get_fan, NULL, 1); > +static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, get_fan, NULL, 2); > +static SENSOR_DEVICE_ATTR(fan4_input, S_IRUGO, get_fan, NULL, 3); > +static SENSOR_DEVICE_ATTR(fan5_input, S_IRUGO, get_fan, NULL, 4); > +static SENSOR_DEVICE_ATTR(fan6_input, S_IRUGO, get_fan, NULL, 5); > + > +static SENSOR_DEVICE_ATTR(fan1_target, S_IWUSR | S_IRUGO, > + get_fan_target, set_fan_target, 0); > +static SENSOR_DEVICE_ATTR(fan2_target, S_IWUSR | S_IRUGO, > + get_fan_target, set_fan_target, 1); > +static SENSOR_DEVICE_ATTR(fan3_target, S_IWUSR | S_IRUGO, > + get_fan_target, set_fan_target, 2); > +static SENSOR_DEVICE_ATTR(fan4_target, S_IWUSR | S_IRUGO, > + get_fan_target, set_fan_target, 3); > +static SENSOR_DEVICE_ATTR(fan5_target, S_IWUSR | S_IRUGO, > + get_fan_target, set_fan_target, 4); > +static SENSOR_DEVICE_ATTR(fan6_target, S_IWUSR | S_IRUGO, > + get_fan_target, set_fan_target, 5); > + > +static SENSOR_DEVICE_ATTR(pwm1, S_IWUSR | S_IRUGO, get_pwm, set_pwm, 0); > +static SENSOR_DEVICE_ATTR(pwm2, S_IWUSR | S_IRUGO, get_pwm, set_pwm, 1); > +static SENSOR_DEVICE_ATTR(pwm3, S_IWUSR | S_IRUGO, get_pwm, set_pwm, 2); > +static SENSOR_DEVICE_ATTR(pwm4, S_IWUSR | S_IRUGO, get_pwm, set_pwm, 3); > +static SENSOR_DEVICE_ATTR(pwm5, S_IWUSR | S_IRUGO, get_pwm, set_pwm, 4); > +static SENSOR_DEVICE_ATTR(pwm6, S_IWUSR | S_IRUGO, get_pwm, set_pwm, 5); > + > +static DEVICE_ATTR(reset, S_IWUSR | S_IRUGO, get_reset, set_reset); > +static DEVICE_ATTR(standby, S_IWUSR | S_IRUGO, get_standby, set_standby); > +static DEVICE_ATTR(bus_timeout, S_IWUSR | S_IRUGO, > + get_bus_timeout, set_bus_timeout); > +static DEVICE_ATTR(watchdog, S_IWUSR | S_IRUGO, get_watchdog, set_watchdog); > + > +static SENSOR_DEVICE_ATTR(fan1_mode, S_IWUSR | S_IRUGO, > + get_fan_mode, set_fan_mode, 0); > +static SENSOR_DEVICE_ATTR(fan2_mode, S_IWUSR | S_IRUGO, > + get_fan_mode, set_fan_mode, 1); > +static SENSOR_DEVICE_ATTR(fan3_mode, S_IWUSR | S_IRUGO, > + get_fan_mode, set_fan_mode, 2); > +static SENSOR_DEVICE_ATTR(fan4_mode, S_IWUSR | S_IRUGO, > + get_fan_mode, set_fan_mode, 3); > +static SENSOR_DEVICE_ATTR(fan5_mode, S_IWUSR | S_IRUGO, > + get_fan_mode, set_fan_mode, 4); > +static SENSOR_DEVICE_ATTR(fan6_mode, S_IWUSR | S_IRUGO, > + get_fan_mode, set_fan_mode, 5); > + > +static SENSOR_DEVICE_ATTR(fan1_tach_enable, S_IWUSR | S_IRUGO, > + get_tach_enable, set_tach_enable, 0); > +static SENSOR_DEVICE_ATTR(fan2_tach_enable, S_IWUSR | S_IRUGO, > + get_tach_enable, set_tach_enable, 1); > +static SENSOR_DEVICE_ATTR(fan3_tach_enable, S_IWUSR | S_IRUGO, > + get_tach_enable, set_tach_enable, 2); > +static SENSOR_DEVICE_ATTR(fan4_tach_enable, S_IWUSR | S_IRUGO, > + get_tach_enable, set_tach_enable, 3); > +static SENSOR_DEVICE_ATTR(fan5_tach_enable, S_IWUSR | S_IRUGO, > + get_tach_enable, set_tach_enable, 4); > +static SENSOR_DEVICE_ATTR(fan6_tach_enable, S_IWUSR | S_IRUGO, > + get_tach_enable, set_tach_enable, 5); > + > +static SENSOR_DEVICE_ATTR(fan1_fault, S_IWUSR | S_IRUGO, > + get_fan_fault, set_fan_fault, 0); > +static SENSOR_DEVICE_ATTR(fan2_fault, S_IWUSR | S_IRUGO, > + get_fan_fault, set_fan_fault, 1); > +static SENSOR_DEVICE_ATTR(fan3_fault, S_IWUSR | S_IRUGO, > + get_fan_fault, set_fan_fault, 2); > +static SENSOR_DEVICE_ATTR(fan4_fault, S_IWUSR | S_IRUGO, > + get_fan_fault, set_fan_fault, 3); > +static SENSOR_DEVICE_ATTR(fan5_fault, S_IWUSR | S_IRUGO, > + get_fan_fault, set_fan_fault, 4); > +static SENSOR_DEVICE_ATTR(fan6_fault, S_IWUSR | S_IRUGO, > + get_fan_fault, set_fan_fault, 5); > + > +static SENSOR_DEVICE_ATTR(fan1_mask, S_IWUSR | S_IRUGO, > + get_fan_mask, set_fan_mask, 0); > +static SENSOR_DEVICE_ATTR(fan2_mask, S_IWUSR | S_IRUGO, > + get_fan_mask, set_fan_mask, 1); > +static SENSOR_DEVICE_ATTR(fan3_mask, S_IWUSR | S_IRUGO, > + get_fan_mask, set_fan_mask, 2); > +static SENSOR_DEVICE_ATTR(fan4_mask, S_IWUSR | S_IRUGO, > + get_fan_mask, set_fan_mask, 3); > +static SENSOR_DEVICE_ATTR(fan5_mask, S_IWUSR | S_IRUGO, > + get_fan_mask, set_fan_mask, 4); > +static SENSOR_DEVICE_ATTR(fan6_mask, S_IWUSR | S_IRUGO, > + get_fan_mask, set_fan_mask, 5); > + > +static struct attribute *max31790_attrs[] = { > + &sensor_dev_attr_fan1_input.dev_attr.attr, > + &sensor_dev_attr_fan2_input.dev_attr.attr, > + &sensor_dev_attr_fan3_input.dev_attr.attr, > + &sensor_dev_attr_fan4_input.dev_attr.attr, > + &sensor_dev_attr_fan5_input.dev_attr.attr, > + &sensor_dev_attr_fan6_input.dev_attr.attr, > + > + &sensor_dev_attr_fan1_target.dev_attr.attr, > + &sensor_dev_attr_fan2_target.dev_attr.attr, > + &sensor_dev_attr_fan3_target.dev_attr.attr, > + &sensor_dev_attr_fan4_target.dev_attr.attr, > + &sensor_dev_attr_fan5_target.dev_attr.attr, > + &sensor_dev_attr_fan6_target.dev_attr.attr, > + > + &sensor_dev_attr_pwm1.dev_attr.attr, > + &sensor_dev_attr_pwm2.dev_attr.attr, > + &sensor_dev_attr_pwm3.dev_attr.attr, > + &sensor_dev_attr_pwm4.dev_attr.attr, > + &sensor_dev_attr_pwm5.dev_attr.attr, > + &sensor_dev_attr_pwm6.dev_attr.attr, > + > + &dev_attr_reset.attr, > + &dev_attr_standby.attr, > + &dev_attr_bus_timeout.attr, > + &dev_attr_watchdog.attr, > + > + &sensor_dev_attr_fan1_mode.dev_attr.attr, > + &sensor_dev_attr_fan2_mode.dev_attr.attr, > + &sensor_dev_attr_fan3_mode.dev_attr.attr, > + &sensor_dev_attr_fan4_mode.dev_attr.attr, > + &sensor_dev_attr_fan5_mode.dev_attr.attr, > + &sensor_dev_attr_fan6_mode.dev_attr.attr, > + > + &sensor_dev_attr_fan1_tach_enable.dev_attr.attr, > + &sensor_dev_attr_fan2_tach_enable.dev_attr.attr, > + &sensor_dev_attr_fan3_tach_enable.dev_attr.attr, > + &sensor_dev_attr_fan4_tach_enable.dev_attr.attr, > + &sensor_dev_attr_fan5_tach_enable.dev_attr.attr, > + &sensor_dev_attr_fan6_tach_enable.dev_attr.attr, > + > + &sensor_dev_attr_fan1_fault.dev_attr.attr, > + &sensor_dev_attr_fan2_fault.dev_attr.attr, > + &sensor_dev_attr_fan3_fault.dev_attr.attr, > + &sensor_dev_attr_fan4_fault.dev_attr.attr, > + &sensor_dev_attr_fan5_fault.dev_attr.attr, > + &sensor_dev_attr_fan6_fault.dev_attr.attr, > + > + &sensor_dev_attr_fan1_mask.dev_attr.attr, > + &sensor_dev_attr_fan2_mask.dev_attr.attr, > + &sensor_dev_attr_fan3_mask.dev_attr.attr, > + &sensor_dev_attr_fan4_mask.dev_attr.attr, > + &sensor_dev_attr_fan5_mask.dev_attr.attr, > + &sensor_dev_attr_fan6_mask.dev_attr.attr, > + NULL > +}; > +ATTRIBUTE_GROUPS(max31790); > + > +static int max31790_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct i2c_adapter *adapter = client->adapter; > + struct device *dev = &client->dev; > + struct max31790_data *data; > + struct device *hwmon_dev; > + > + if (!i2c_check_functionality(adapter, > + I2C_FUNC_SMBUS_BYTE_DATA | I2C_FUNC_SMBUS_WORD_DATA)) > + return -ENODEV; > + > + data = devm_kzalloc(dev, sizeof(struct max31790_data), GFP_KERNEL); > + if (!data) > + return -ENOMEM; > + > + data->client = client; > + mutex_init(&data->update_lock); > + > + hwmon_dev = devm_hwmon_device_register_with_groups(dev, > + client->name, data, max31790_groups); > + > + return PTR_ERR_OR_ZERO(hwmon_dev); > +} > + > +static const struct i2c_device_id max31790_id[] = { > + { "max31790", max31790 }, You don't use the 'max31790' in the probe function (nor would there be a reason to do so). > + { } > +}; > +MODULE_DEVICE_TABLE(i2c, max31790_id); > + > +static struct i2c_driver max31790_driver = { > + .class = I2C_CLASS_HWMON, > + .probe = max31790_probe, > + .driver = { > + .name = "max31790", > + }, > + .id_table = max31790_id, > +}; > + > +module_i2c_driver(max31790_driver); > + > +MODULE_AUTHOR("Il Han "); > +MODULE_DESCRIPTION("MAX31790 sensor driver"); > +MODULE_LICENSE("GPL"); > _______________________________________________ lm-sensors mailing list lm-sensors@lm-sensors.org http://lists.lm-sensors.org/mailman/listinfo/lm-sensors