On Wed, 2011-01-19 at 11:29 -0500, stigge@xxxxxxxxx wrote: > 2-Channel Temperature Monitor with Dual PWM Fan-Speed Controller > > Signed-off-by: Roland Stigge <stigge@xxxxxxxxx> > Much better. Comments inline. > diff --git a/Documentation/hwmon/max6639 b/Documentation/hwmon/max6639 > new file mode 100644 > index 0000000..debb3f0 > --- /dev/null > +++ b/Documentation/hwmon/max6639 > @@ -0,0 +1,37 @@ > +Kernel driver max6639 > +===================== > + > +Supported chips: > + * Maxim MAX6639 > + Prefix: 'max6639' > + Addresses scanned: I2C 0x2c, 0x2e, 0x2f > + Datasheet: http://pdfserv.maxim-ic.com/en/ds/MAX6639.pdf > + > +Authors: > + He Changqing <hechangqing@xxxxxxxxxxxx> > + Roland Stigge <stigge@xxxxxxxxx> > + > +Description > +----------- > + > +This driver implements support for the Maxim MAX6639. This chip is a 2-channel > +temperature monitor with dual PWM fan speed controller. It can monitor its own > +temperature and one external diode-connected transistor or two external > +diode-connected transistors. > + > +The following device attributes are implemented via sysfs: > + > +Attribute R/W Contents > +---------------------------------------------------------------------------- > +temp1_input R Temperature channel 1 input (0..150 C) > +temp2_input R Temperature channel 2 input (0..150 C) > +temp1_max RW Set THERM temperature for input 1 (in C, see datasheet) > +temp2_max RW Set THERM temperature for input 2 (in C, see datasheet) > +temp1_crit RW Set ALERM temperature for input 1 (in C, see datasheet) > +temp2_crit RW Set ALARM temperature for input 2 (in C, see datasheet) > +temp1_emergency RW Set OT temperature for input 1 (in C, see datasheet) > +temp2_emergency RW Set OT temperature for input 2 (in C, see datasheet) > +pwm1 RW Fan 1 target duty cycle (0..255) > +pwm2 RW Fan 2 target duty cycle (0..255) > +fan1_input R TACH1 fan tachometer input (in RPM) > +fan2_input R TACH2 fan tachometer input (in RPM) > diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig > index a56f6ad..8eb116c 100644 > --- a/drivers/hwmon/Kconfig > +++ b/drivers/hwmon/Kconfig > @@ -674,6 +674,16 @@ config SENSORS_MAX1619 > This driver can also be built as a module. If so, the module > will be called max1619. > > +config SENSORS_MAX6639 > + tristate "Maxim MAX6639 sensor chip" > + depends on I2C && EXPERIMENTAL > + help > + If you say yes here you get support for the MAX6639 > + sensor chips. > + > + This driver can also be built as a module. If so, the module > + will be called max6639. > + > config SENSORS_MAX6650 > tristate "Maxim MAX6650 sensor chip" > depends on I2C && EXPERIMENTAL > diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile > index 2479b3d..a85fa3f 100644 > --- a/drivers/hwmon/Makefile > +++ b/drivers/hwmon/Makefile > @@ -83,6 +83,7 @@ obj-$(CONFIG_SENSORS_LTC4245) += ltc4245.o > obj-$(CONFIG_SENSORS_LTC4261) += ltc4261.o > obj-$(CONFIG_SENSORS_MAX1111) += max1111.o > obj-$(CONFIG_SENSORS_MAX1619) += max1619.o > +obj-$(CONFIG_SENSORS_MAX6639) += max6639.o > obj-$(CONFIG_SENSORS_MAX6650) += max6650.o > obj-$(CONFIG_SENSORS_MC13783_ADC)+= mc13783-adc.o > obj-$(CONFIG_SENSORS_PC87360) += pc87360.o > diff --git a/drivers/hwmon/max6639.c b/drivers/hwmon/max6639.c > new file mode 100644 > index 0000000..64e77f9 > --- /dev/null > +++ b/drivers/hwmon/max6639.c > @@ -0,0 +1,567 @@ > +/* > + * max6639.c - Support for Maxim MAX6639 > + * > + * 2-Channel Temperature Monitor with Dual PWM Fan-Speed Controller > + * > + * Copyright (C) 2010, 2011 Roland Stigge <stigge@xxxxxxxxx> > + * > + * based on the initial MAX6639 support from septian.net semptian.net > + * by He Changqing <hechangqing@xxxxxxxxxxxx> > + * > + * 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. > + * > + * You should have received a copy of the GNU General Public License > + * along with this program; if not, write to the Free Software > + * Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. > + */ > + > +#include <linux/module.h> > +#include <linux/init.h> > +#include <linux/slab.h> > +#include <linux/jiffies.h> > +#include <linux/i2c.h> > +#include <linux/hwmon.h> > +#include <linux/hwmon-sysfs.h> > +#include <linux/err.h> > +#include <linux/mutex.h> > +#include <linux/i2c/max6639.h> > + > +/* Addresses to scan */ > +static unsigned short normal_i2c[] = { 0x2c, 0x2e, 0x2f, I2C_CLIENT_END }; > + > +/* The MAX6639 registers, valid channel numbers: 0, 1 */ > +#define MAX6639_REG_TEMP(ch) (0x00 + ch) > +#define MAX6639_REG_STATUS 0x02 > +#define MAX6639_REG_OUTPUT_MASK 0x03 > +#define MAX6639_REG_GCONFIG 0x04 > +#define MAX6639_REG_TEMP_EXT(ch) (0x05 + ch) > +#define MAX6639_REG_ALERT_LIMIT(ch) (0x08 + ch) > +#define MAX6639_REG_OT_LIMIT(ch) (0x0A + ch) > +#define MAX6639_REG_THERM_LIMIT(ch) (0x0C + ch) > +#define MAX6639_REG_FAN_CONFIG1(ch) (0x10 + ch * 4) > +#define MAX6639_REG_FAN_CONFIG2a(ch) (0x11 + ch * 4) > +#define MAX6639_REG_FAN_CONFIG2b(ch) (0x12 + ch * 4) > +#define MAX6639_REG_FAN_CONFIG3(ch) (0x13 + ch * 4) > +#define MAX6639_REG_FAN_CNT(ch) (0x20 + ch) > +#define MAX6639_REG_TARGET_CNT(ch) (0x22 + ch) > +#define MAX6639_REG_FAN_PPR(ch) (0x24 + ch) > +#define MAX6639_REG_TARGTDUTY(ch) (0x26 + ch) > +#define MAX6639_REG_FAN_START_TEMP(ch) (0x28 + ch) Usually you would want to have "ch" in (), to ensure it is correct if "ch" is an expression. > +#define MAX6639_REG_DEVID 0x3D > +#define MAX6639_REG_MANUID 0x3E > +#define MAX6639_REG_DEVREV 0x3F Not used/needed ? > + > +/* Register bits */ > +#define MAX6639_GCONFIG_STANDBY 0x80 > +#define MAX6639_GCONFIG_POR 0x40 > +#define MAX6639_GCONFIG_DISABLE_TIMEOUT 0x20 > +#define MAX6639_GCONFIG_CH2_LOCAL 0x10 > + > +#define MAX6639_FAN_CONFIG1_PWM 0x80 > + > +static int rpm_ranges[] = { 2000, 4000, 8000, 16000 }; > + > +#define FAN_FROM_REG(val, div) ((val) == 0 ? -1 : \ > + (val) == 255 ? 0 : (4000 * 30) / ((div + 1) * (val))) > +#define TEMP_LIMIT_TO_REG(val) SENSORS_LIMIT((val) / 1000, 0, 255) > + > +/* > + * Client data (each client gets its own) > + */ > +struct max6639_data { > + struct device *hwmon_dev; > + struct mutex update_lock; > + char valid; /* !=0 if following fields are valid */ > + unsigned long last_updated; /* In jiffies */ > + > + /* Register values sampled regularly */ > + u16 temp[2]; /* Temperature, in 1/8 C, 0..255 C */ > + u8 fan[2]; /* Register value: TACH count for fans >=30 */ > + > + /* Register values only written to */ > + u8 pwm[2]; /* Register value: Duty cycle 0..120 */ > + u8 temp_therm[2]; /* THERM Temperature, 0..255 C (->_max) */ > + u8 temp_alert[2]; /* ALERT Temperature, 0..255 C (->_crit) */ > + u8 temp_ot[2]; /* OT Temperature, 0..255 C (->_emergency) */ > + > + /* Register values initialized only once */ > + u8 ppr[2]; /* Pulses per rotation 0..3 for 1..4 ppr */ Don't need ppr[2] - also see below. > +}; > + > +static int max6639_read_value(struct i2c_client *client, u8 reg) > +{ > + return i2c_smbus_read_byte_data(client, reg); > +} > + > +static int max6639_write_value(struct i2c_client *client, u8 reg, u8 value) > +{ > + return i2c_smbus_write_byte_data(client, reg, value); > +} > + Any reason for not using i2c_smbus_read_byte_data() and i2c_smbus_write_byte_data() directly ? > +static struct max6639_data *max6639_update_device(struct device *dev) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct max6639_data *data = i2c_get_clientdata(client); > + struct max6639_data *ret = data; > + int i; > + > + mutex_lock(&data->update_lock); > + > + if (time_after(jiffies, data->last_updated + 2 * HZ) || !data->valid) { > + int res; > + > + dev_dbg(&client->dev, "Starting max6639 update\n"); > + > + for (i = 0; i < 2; i++) { > + res = max6639_read_value(client, > + MAX6639_REG_FAN_CNT(i)); > + if (res < 0) { > + ret = ERR_PTR(res); > + goto abort; > + } > + data->fan[i] = res; > + > + res = (max6639_read_value(client, > + MAX6639_REG_TEMP_EXT(i)) >> 5) > + | (max6639_read_value(client, > + MAX6639_REG_TEMP(i)) << 3); You'll have to rewrite that a bit. Logical or and shifting of error values won't yield a useful result. > + if (res < 0) { > + ret = ERR_PTR(res); > + goto abort; > + } > + data->temp[i] = res; > + } > + > + data->last_updated = jiffies; > + data->valid = 1; > + } > +abort: > + mutex_unlock(&data->update_lock); > + > + return ret; > +} > + > +static ssize_t show_temp_input(struct device *dev, > + struct device_attribute *dev_attr, char *buf) > +{ > + long temp; > + struct max6639_data *data = max6639_update_device(dev); > + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr); > + Add: if (IS_ERR(data)) return PTR_ERR(data); > + temp = data->temp[attr->index] * 125; > + return sprintf(buf, "%ld\n", temp); > +} > + > +static ssize_t show_temp_max(struct device *dev, > + struct device_attribute *dev_attr, char *buf) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct max6639_data *data = i2c_get_clientdata(client); > + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr); > + > + return sprintf(buf, "%d\n", (data->temp_therm[attr->index] * 1000)); > +} > + > +static ssize_t set_temp_max(struct device *dev, > + struct device_attribute *dev_attr, > + const char *buf, size_t count) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct max6639_data *data = i2c_get_clientdata(client); > + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr); > + unsigned long val; > + int res; > + > + res = strict_strtoul(buf, 10, &val); > + if (res) > + return res; > + > + mutex_lock(&data->update_lock); > + data->temp_therm[attr->index] = TEMP_LIMIT_TO_REG(val); > + max6639_write_value(client, MAX6639_REG_THERM_LIMIT(attr->index), > + data->temp_therm[attr->index]); > + mutex_unlock(&data->update_lock); > + return count; > +} > + > +static ssize_t show_temp_crit(struct device *dev, > + struct device_attribute *dev_attr, char *buf) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct max6639_data *data = i2c_get_clientdata(client); > + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr); > + > + return sprintf(buf, "%d\n", (data->temp_alert[attr->index] * 1000)); > +} > + > +static ssize_t set_temp_crit(struct device *dev, > + struct device_attribute *dev_attr, > + const char *buf, size_t count) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct max6639_data *data = i2c_get_clientdata(client); > + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr); > + unsigned long val; > + int res; > + > + res = strict_strtoul(buf, 10, &val); > + if (res) > + return res; > + > + mutex_lock(&data->update_lock); > + data->temp_alert[attr->index] = TEMP_LIMIT_TO_REG(val); > + max6639_write_value(client, MAX6639_REG_THERM_LIMIT(attr->index), Should be MAX6639_REG_ALERT_LIMIT() > + data->temp_alert[attr->index]); > + mutex_unlock(&data->update_lock); > + return count; > +} > + > +static ssize_t show_temp_emergency(struct device *dev, > + struct device_attribute *dev_attr, > + char *buf) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct max6639_data *data = i2c_get_clientdata(client); > + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr); > + > + return sprintf(buf, "%d\n", (data->temp_ot[attr->index] * 1000)); > +} > + > +static ssize_t set_temp_emergency(struct device *dev, > + struct device_attribute *dev_attr, > + const char *buf, size_t count) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct max6639_data *data = i2c_get_clientdata(client); > + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr); > + unsigned long val; > + int res; > + > + res = strict_strtoul(buf, 10, &val); > + if (res) > + return res; > + > + mutex_lock(&data->update_lock); > + data->temp_ot[attr->index] = TEMP_LIMIT_TO_REG(val); > + max6639_write_value(client, MAX6639_REG_THERM_LIMIT(attr->index), > + data->temp_ot[attr->index]); Should be MAX6639_REG_OT_LIMIT() > + mutex_unlock(&data->update_lock); > + return count; > +} > + > +static ssize_t show_pwm(struct device *dev, > + struct device_attribute *dev_attr, char *buf) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct max6639_data *data = i2c_get_clientdata(client); > + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr); > + > + return sprintf(buf, "%d\n", data->pwm[attr->index] * 255 / 120); > +} > + > +static ssize_t set_pwm(struct device *dev, > + struct device_attribute *dev_attr, > + const char *buf, size_t count) > +{ > + struct i2c_client *client = to_i2c_client(dev); > + struct max6639_data *data = i2c_get_clientdata(client); > + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr); > + long val; > + int res; > + > + res = strict_strtoul(buf, 10, &val); > + if (res) > + return res; > + > + mutex_lock(&data->update_lock); > + data->pwm[attr->index] = (u8)(val * 120 / 255); > + max6639_write_value(client, MAX6639_REG_TARGTDUTY(attr->index), > + data->pwm[attr->index]); > + mutex_unlock(&data->update_lock); > + return count; > +} > + > +static ssize_t show_fan_input(struct device *dev, > + struct device_attribute *dev_attr, char *buf) > +{ > + struct max6639_data *data = max6639_update_device(dev); > + struct sensor_device_attribute *attr = to_sensor_dev_attr(dev_attr); > + > + return sprintf(buf, "%d\n", FAN_FROM_REG(data->fan[attr->index], > + data->ppr[attr->index])); > +} > + > +static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, show_temp_input, NULL, 0); > +static SENSOR_DEVICE_ATTR(temp2_input, S_IRUGO, show_temp_input, NULL, 1); > +static SENSOR_DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO, show_temp_max, > + set_temp_max, 0); > +static SENSOR_DEVICE_ATTR(temp2_max, S_IWUSR | S_IRUGO, show_temp_max, > + set_temp_max, 1); > +static SENSOR_DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO, show_temp_crit, > + set_temp_crit, 0); > +static SENSOR_DEVICE_ATTR(temp2_crit, S_IWUSR | S_IRUGO, show_temp_crit, > + set_temp_crit, 1); > +static SENSOR_DEVICE_ATTR(temp1_emergency, S_IWUSR | S_IRUGO, > + show_temp_emergency, set_temp_emergency, 0); > +static SENSOR_DEVICE_ATTR(temp2_emergency, S_IWUSR | S_IRUGO, > + show_temp_emergency, set_temp_emergency, 1); > +static SENSOR_DEVICE_ATTR(pwm1, S_IWUSR | S_IRUGO, show_pwm, set_pwm, 0); > +static SENSOR_DEVICE_ATTR(pwm2, S_IWUSR | S_IRUGO, show_pwm, set_pwm, 1); > +static SENSOR_DEVICE_ATTR(fan1_input, S_IRUGO, show_fan_input, NULL, 0); > +static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, show_fan_input, NULL, 1); > + The chip also supports reporting FAULT (extended temp register bit 0). Please add temp1_fault and temp2_fault to report it. > +static struct attribute *max6639_attributes[] = { > + &sensor_dev_attr_temp1_input.dev_attr.attr, > + &sensor_dev_attr_temp2_input.dev_attr.attr, > + &sensor_dev_attr_temp1_max.dev_attr.attr, > + &sensor_dev_attr_temp2_max.dev_attr.attr, > + &sensor_dev_attr_temp1_crit.dev_attr.attr, > + &sensor_dev_attr_temp2_crit.dev_attr.attr, > + &sensor_dev_attr_temp1_emergency.dev_attr.attr, > + &sensor_dev_attr_temp2_emergency.dev_attr.attr, > + &sensor_dev_attr_pwm1.dev_attr.attr, > + &sensor_dev_attr_pwm2.dev_attr.attr, > + &sensor_dev_attr_fan1_input.dev_attr.attr, > + &sensor_dev_attr_fan2_input.dev_attr.attr, > + NULL > +}; > + > +static const struct attribute_group max6639_group = { > + .attrs = max6639_attributes, > +}; > + > +/* returns respective index in rpm_ranges table, -1 on invalid range */ > +static int rpm_range_to_reg(int range) > +{ > + int i; > + int result = -1; > + > + for (i = 0; i < ARRAY_SIZE(rpm_ranges); i++) { > + if (rpm_ranges[i] == range) > + result = i; break; ? or return i; ? > + } > + > + return result; If you return i above, you would not need the result variable and could simply return -1 here (or 1 as suggested below). > +} > + > +static int max6639_init_client(struct i2c_client *client) > +{ > + struct max6639_data *data = i2c_get_clientdata(client); > + struct max6639_platform_data *max6639_info = > + client->dev.platform_data; > + int i; > + int rpm_range = -1; > + int err = 0; > + > + /* Reset chip to default values */ > + err = max6639_write_value(client, MAX6639_REG_GCONFIG, > + MAX6639_GCONFIG_POR); > + if (err) > + goto exit; > + > + /* RPM range, 4000 by default */ > + if (max6639_info) > + rpm_range = rpm_range_to_reg(max6639_info->rpm_range); > + if (rpm_range < 0) > + rpm_range = 1; > + If you have rpm_range_to_reg() return 1 if it doesn't find the value, you could initialize rpm_range with 1 and would not need the additional error check and update. > + for (i = 0; i < 2; i++) { > + > + /* Fans config PWM, RPM */ > + err = max6639_write_value(client, MAX6639_REG_FAN_CONFIG1(i), > + MAX6639_FAN_CONFIG1_PWM | rpm_range); > + if (err) > + goto exit; > + > + /* Fans pulse per revolution is 2 by default */ > + if (max6639_info) > + data->ppr[i] = max6639_info->ppr; > + else > + data->ppr[i] = 2; Please validate max6639_info->ppr, such as if (max6639_info && max6639_info->ppr > 0 && max6639_info->ppr < 5) Since ppr is the same for both fans, you don't really need data->ppr[2], and data->ppr would be sufficient (ie move data->ppr initialization out of the loop and only initialize the register here). > + data->ppr[i] -= 1; > + err = max6639_write_value(client, MAX6639_REG_FAN_PPR(i), > + (data->ppr[i]) << 5); > + if (err) > + goto exit; > + > + /* Fans PWM polarity high by default */ > + if (max6639_info && max6639_info->pwm_polarity == 0) > + err = max6639_write_value(client, > + MAX6639_REG_FAN_CONFIG2a(i), 0x00); > + else > + err = max6639_write_value(client, > + MAX6639_REG_FAN_CONFIG2a(i), 0x02); > + if (err) > + goto exit; > + > + /* Max. temp. 80C/90C/100C */ > + data->temp_therm[i] = 80; > + data->temp_alert[i] = 90; > + data->temp_ot[i] = 100; > + err = max6639_write_value(client, MAX6639_REG_THERM_LIMIT(i), > + data->temp_therm[i]); > + if (err) > + goto exit; > + err = max6639_write_value(client, MAX6639_REG_ALERT_LIMIT(i), > + data->temp_alert[i]); > + if (err) > + goto exit; > + err = max6639_write_value(client, MAX6639_REG_OT_LIMIT(i), > + data->temp_ot[i]); > + if (err) > + goto exit; > + > + /* PWM 120/120 (i.e. 100%) */ > + data->pwm[i] = 120; > + err = max6639_write_value(client, MAX6639_REG_TARGTDUTY(i), > + data->pwm[i]); > + if (err) > + goto exit; > + } > + /* Start monitoring */ > + err = max6639_write_value(client, MAX6639_REG_GCONFIG, > + MAX6639_GCONFIG_DISABLE_TIMEOUT | MAX6639_GCONFIG_CH2_LOCAL); > +exit: > + return err; > +} > + > +/* Return 0 if detection is successful, -ENODEV otherwise */ > +static int max6639_detect(struct i2c_client *client, > + struct i2c_board_info *info) > +{ > + struct i2c_adapter *adapter = client->adapter; > + int dev_id, manu_id; > + > + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) > + return -ENODEV; > + > + /* Actual detection via device and manufacturer ID */ > + dev_id = max6639_read_value(client, MAX6639_REG_DEVID); > + manu_id = max6639_read_value(client, MAX6639_REG_MANUID); > + if (dev_id != 0x58 || manu_id != 0x4D) > + return -ENODEV; > + > + strlcpy(info->type, "max6639", I2C_NAME_SIZE); > + > + return 0; > +} > + > +static int max6639_probe(struct i2c_client *client, > + const struct i2c_device_id *id) > +{ > + struct max6639_data *data; > + int err; > + > + data = kzalloc(sizeof(struct max6639_data), GFP_KERNEL); > + if (!data) { > + err = -ENOMEM; > + goto exit; > + } > + > + i2c_set_clientdata(client, data); > + mutex_init(&data->update_lock); > + > + /* Initialize the max6639 chip */ > + err = max6639_init_client(client); > + if (err < 0) > + goto error_free; > + > + /* Register sysfs hooks */ > + err = sysfs_create_group(&client->dev.kobj, &max6639_group); > + if (err) > + goto error_free; > + > + data->hwmon_dev = hwmon_device_register(&client->dev); > + if (IS_ERR(data->hwmon_dev)) { > + err = PTR_ERR(data->hwmon_dev); > + goto error_remove; > + } > + > + dev_info(&client->dev, "temperature sensor and fan control found\n"); > + > + return 0; > + > +error_remove: > + sysfs_remove_group(&client->dev.kobj, &max6639_group); > +error_free: > + kfree(data); > +exit: > + return err; > +} > + > +static int max6639_remove(struct i2c_client *client) > +{ > + struct max6639_data *data = i2c_get_clientdata(client); > + > + hwmon_device_unregister(data->hwmon_dev); > + sysfs_remove_group(&client->dev.kobj, &max6639_group); > + > + kfree(data); > + return 0; > +} > + > +static int max6639_suspend(struct i2c_client *client, pm_message_t mesg) > +{ > + int data = max6639_read_value(client, MAX6639_REG_GCONFIG); > + if (data < 0) > + return data; > + > + return max6639_write_value(client, > + MAX6639_REG_GCONFIG, data | MAX6639_GCONFIG_STANDBY); > +} > + > +static int max6639_resume(struct i2c_client *client) > +{ > + int data = max6639_read_value(client, MAX6639_REG_GCONFIG); > + if (data < 0) > + return data; > + > + return max6639_write_value(client, > + MAX6639_REG_GCONFIG, data & ~MAX6639_GCONFIG_STANDBY); > +} > + > +static const struct i2c_device_id max6639_id[] = { > + {"max6639", 0}, > + {} > +}; > + > +MODULE_DEVICE_TABLE(i2c, max6639_id); > + > +static struct i2c_driver max6639_driver = { > + .class = I2C_CLASS_HWMON, > + .driver = { > + .name = "max6639", > + }, > + .probe = max6639_probe, > + .remove = max6639_remove, > + .suspend = max6639_suspend, > + .resume = max6639_resume, > + .id_table = max6639_id, > + .detect = max6639_detect, > + .address_list = normal_i2c, > +}; > + > +static int __init max6639_init(void) > +{ > + return i2c_add_driver(&max6639_driver); > +} > + > +static void __exit max6639_exit(void) > +{ > + i2c_del_driver(&max6639_driver); > +} > + > +MODULE_AUTHOR("Roland Stigge <stigge@xxxxxxxxx>"); > +MODULE_DESCRIPTION("max6639 driver"); > +MODULE_LICENSE("GPL"); > + > +module_init(max6639_init); > +module_exit(max6639_exit); > diff --git a/include/linux/i2c/max6639.h b/include/linux/i2c/max6639.h > new file mode 100644 > index 0000000..f7323c5 > --- /dev/null > +++ b/include/linux/i2c/max6639.h > @@ -0,0 +1,15 @@ > +#ifndef _LINUX_MAX6639_H > +#define _LINUX_MAX6639_H > + > +#include <linux/types.h> > +#include <linux/i2c.h> > + Do you need to include i2c.h here ? > +/* platform data for the MAX6639 temperature sensor and fan control */ > + > +struct max6639_platform_data { > + bool pwm_polarity; /* Polarity low (0) or high (1, default) */ > + int ppr; /* Pulses per rotation 1..4 (default == 1) */ You initialize ppr with 2 above if not configured, so the default is not correct. Please either change default or initialization code. > + int rpm_range; /* 2000, 4000 (default), 8000 or 16000 */ > +}; > + > +#endif /* _LINUX_MAX6639_H */ > > _______________________________________________ > lm-sensors mailing list > lm-sensors@xxxxxxxxxxxxxx > http://lists.lm-sensors.org/mailman/listinfo/lm-sensors _______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
