Hans-J?rgen Koch a ?crit :
> Am Sonntag, 11. Februar 2007 17:22 schrieb Hans-J?rgen Koch:
>> Am Sonntag 11 Februar 2007 16:44 schrieb corentin.labbe:
>>> Hello
>>>
>>> i saw some problems in your code:
>>>
>>> On line 141 you excess the 80 characters width
>>> + static ssize_t get_fan(struct device *dev, struct
>>> device_attribute *devattr, char *buf, int nr)
>>>
>>>
>>> On the comment you have written "MAX6650 also" instead of "MAX6551
>>> also" * This module has only been tested with the MAX6650 chip. It
>>> should * work with the MAX6650 also, though with reduced
>>> functionality. It * does not distinguish max6650 and max6651 chips.
>>>
>>>
>>> You must use dev_dbg instead of #ifdef debug
>>>
>>>
>>> On line 535 the while(0) is useless
>>> erase the max6650_init_client function
>>> +static void max6650_init_client(struct i2c_client *client)
>>> +{
>>> + /* Nothing to do here - assume the BIOS has initialized the chip
>>> */ + while(0)
>>> + {
>>> + ;
>>> + }
>>> +}
>>>
>>>
>>> Don't use down() up()
>>> replace with mutex
>>>
>>> On line 553
>>> MAX6650_REG_TACH0, MAX6650_REG_TACH1,
>>> MAX6650_REG_TACH2, MAX6650_REG_TACH3
>>> one per line
>>> "," at the end
>>> like
>>> static const u8 tach_reg[] =
>>> {
>>> MAX6650_REG_TACH0,
>>> MAX6650_REG_TACH1,
>>> MAX6650_REG_TACH2,
>>> MAX6650_REG_TACH3,
>>> };
>>>
>>>
>>> Why use max6650_read_value and max6650_write_value?
>>> replace with i2c_smbus_write_byte_data and i2c_smbus_read_byte_data
>>> directly
>>>
>>> Check error return code of i2c_detach_client(client);
>>> like
>>> if ((err = i2c_detach_client(client)))
>>> return err;
>>>
>>>
>>> Don't use many device_create_file
>>> use sysfs_create_group
>> Thanks for your review, I'll look at the code again and post an
>> updated patch soon.
>>
>
> Hi,
> here's the promised patch with these issues hopefully solved. I tested the
> module on a Kontron CPX board, with a vanilla 2.6.20 kernel, it seems to work
> fine.
>
> Thanks again for the review,
>
> Hans
>
>
>
> ------------------------------------------------------------------------
>
> Index: linux-2.6.20/Documentation/hwmon/max6650
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6.20/Documentation/hwmon/max6650 2007-02-12 14:40:26.000000000 +0100
> @@ -0,0 +1,33 @@
> +Kernel driver max6650
> +=====================
> +
> +Supported chips:
> + * Maxim 6650 / 6651
> + Prefix: 'w83792d'
> + Addresses scanned: I2C 0x1b, 0x1f, 0x48, 0x4b
> + Datasheet: http://pdfserv.maxim-ic.com/en/ds/MAX6650-MAX6651.pdf
> +
> +Authors:
> + John Morris <john.morris at spirentcom.com>
> + Claus Gindhart <claus.gindhart at kontron.com>
> + Hans J. Koch <hjk at linutronix.de>
> +
> +Description
> +-----------
> +
> +This driver implements support for the Maxim 6650/6651
> +
> +The 2 devices are very similar, bit the Maxim 6550 has a reduced feature
> +set, e.g. only one fan-input, instead of 4 for the 6651.
> +
> +The driver is not able to distinguish between the 2 devices.
> +
> +The driver provides the following sensor accesses
> +
> +fan0 ro fan tachometer speed in RPM
> +fan1 ro "
> +fan2 ro "
> +fan3 ro "
> +config ro contents of the config register (for debugging)
> +count ro tachometer count time in milliseconds
> +speed rw fan speed adjustment value
> Index: linux-2.6.20/drivers/hwmon/Kconfig
> ===================================================================
> --- linux-2.6.20.orig/drivers/hwmon/Kconfig 2007-02-04 19:44:54.000000000 +0100
> +++ linux-2.6.20/drivers/hwmon/Kconfig 2007-02-12 09:01:25.000000000 +0100
> @@ -364,6 +364,16 @@
> This driver can also be built as a module. If so, the module
> will be called max1619.
>
> +config SENSORS_MAX6650
> + tristate "Maxim MAX6650 sensor chip"
> + depends on HWMON && I2C && EXPERIMENTAL
> + help
> + If you say yes here you get support for the MAX6650 / MAX6651
> + sensor chips.
> +
> + This driver can also be built as a module. If so, the module
> + will be called max6650.
> +
> config SENSORS_PC87360
> tristate "National Semiconductor PC87360 family"
> depends on HWMON && I2C && EXPERIMENTAL
> Index: linux-2.6.20/drivers/hwmon/Makefile
> ===================================================================
> --- linux-2.6.20.orig/drivers/hwmon/Makefile 2007-02-04 19:44:54.000000000 +0100
> +++ linux-2.6.20/drivers/hwmon/Makefile 2007-02-12 09:01:25.000000000 +0100
> @@ -42,6 +42,7 @@
> obj-$(CONFIG_SENSORS_LM90) += lm90.o
> obj-$(CONFIG_SENSORS_LM92) += lm92.o
> obj-$(CONFIG_SENSORS_MAX1619) += max1619.o
> +obj-$(CONFIG_SENSORS_MAX6650) += max6650.o
> obj-$(CONFIG_SENSORS_PC87360) += pc87360.o
> obj-$(CONFIG_SENSORS_PC87427) += pc87427.o
> obj-$(CONFIG_SENSORS_SIS5595) += sis5595.o
> Index: linux-2.6.20/drivers/hwmon/max6650.c
> ===================================================================
> --- /dev/null 1970-01-01 00:00:00.000000000 +0000
> +++ linux-2.6.20/drivers/hwmon/max6650.c 2007-02-12 14:34:50.000000000 +0100
> @@ -0,0 +1,558 @@
> +/*
> + * max6650.c - Part of lm_sensors, Linux kernel modules for hardware
> + * monitoring.
> + *
> + * Author: John Morris <john.morris at spirentcom.com>
> + * Copyright (c) 2003 Spirent Communications
> + *
> + * This module has only been tested with the MAX6650 chip. It should
> + * also work with the MAX6651, though with reduced functionality. It
> + * does not distinguish max6650 and max6651 chips.
> + *
> + * Tha datasheet was last seen at:
> + *
> + * http://pdfserv.maxim-ic.com/en/ds/MAX6650-MAX6651.pdf
> + *
> + * Ported to Linux 2.6 by Claus Gindhart <claus.gindhart at kontron.com>
> + * Some cleanup done by Hans J. Koch <hjk at linutronix.de>
> + *
> + * 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>
> +
> +#ifndef I2C_DRIVERID_MAX6650
> +#define I2C_DRIVERID_MAX6650 1044
> +#endif
> +
> +/* HW-related: Set this to 1 for 12V and 0 for 5V configuration */
> +static const int voltage_12V=0;
> +
> +/*
> + * Addresses to scan. There are four disjoint possibilities, by pin config.
> + */
> +
> +static unsigned short normal_i2c[] = {0x1b, 0x1f, 0x48, 0x4b, I2C_CLIENT_END};
> +
> +/*
> + * Insmod parameters
> + */
> +
> +I2C_CLIENT_INSMOD_1(max6650);
> +
> +/*
> + * MAX 6650/6651 registers
> + */
> +
> +#define MAX6650_REG_SPEED 0x00
> +#define MAX6650_REG_CONFIG 0x02
> +#define MAX6650_REG_GPIO_DEF 0x04
> +#define MAX6650_REG_DAC 0x06
> +#define MAX6650_REG_ALARM_EN 0x08
> +#define MAX6650_REG_ALARM 0x0A
> +#define MAX6650_REG_TACH0 0x0C
> +#define MAX6650_REG_TACH1 0x0E
> +#define MAX6650_REG_TACH2 0x10
> +#define MAX6650_REG_TACH3 0x12
> +#define MAX6650_REG_GPIO_STAT 0x14
> +#define MAX6650_REG_COUNT 0x16
> +
> +
> +/*
> + * Config register bits
> + */
> +
> +#define MAX6650_CFG_V12 0x08
> +#define MAX6650_CFG_MODE_MASK 0x30
> +#define MAX6650_CFG_MODE_ON 0x00
> +#define MAX6650_CFG_MODE_OFF 0x10
> +#define MAX6650_CFG_MODE_CLOSED_LOOP 0x20
> +#define MAX6650_CFG_MODE_OPEN_LOOP 0x30
> +
> +#define MAX6650_INT_CLK 254000 /* Default clock speed - 254 kHz */
> +
> +/* Minimum and maximum values of the FAN-RPM */
> +#define FAN_RPM_MIN 240
> +#define FAN_RPM_MAX 30000
> +
> +#define DIV_FROM_REG(reg) (1 << (reg & 7))
> +
> +static int max6650_attach_adapter(struct i2c_adapter *adapter);
> +static int max6650_detect(struct i2c_adapter *adapter, int address, int kind);
> +static void max6650_init_client(struct i2c_client *client);
> +static int max6650_detach_client(struct i2c_client *client);
> +static struct max6650_data *max6650_update_device(struct device *dev);
> +
> +/*
> + * Driver data (common to all clients)
> + */
> +
> +static struct i2c_driver max6650_driver = {
> + .driver = {
> + .name = "max6650",
> + },
> + .id = I2C_DRIVERID_MAX6650,
> + .attach_adapter = max6650_attach_adapter,
> + .detach_client = max6650_detach_client
> +};
> +
> +/*
> + * Client data (each client gets its own)
> + */
> +
> +struct max6650_data
> +{
> + struct i2c_client client;
> + struct class_device *class_dev;
> + struct mutex update_lock;
> + char valid; /* zero until following fields are valid */
> + unsigned long last_updated; /* in jiffies */
> +
> + /* register values */
> + u8 speed;
> + u8 config;
> + u8 tach[4];
> + u8 count;
> +};
> +
> +static ssize_t get_fan(struct device *dev, struct device_attribute *devattr,
> + char *buf, int nr)
> +{
> + struct max6650_data *data = max6650_update_device(dev);
> +
> + /*
> + * Calculation details:
> + *
> + * Each tachometer counts over an interval given by the "count"
> + * register (0.25, 0.5, 1 or 2 seconds). This module assumes
> + * that the fans produce two pulses per revolution (this seems
> + * to be the most common).
> + */
> +
> + int rpm = ((data->tach[nr] * 120) / DIV_FROM_REG(data->count));
> + return sprintf(buf, "%d\n", rpm);
> +}
> +
> +static ssize_t get_fan0(struct device *dev, struct device_attribute *devattr,
> + char *buf)
> +{
> + return get_fan(dev,devattr,buf,0);
> +}
> +
> +static ssize_t get_fan1(struct device *dev, struct device_attribute *devattr,
> + char *buf)
> +{
> + return get_fan(dev,devattr,buf,1);
> +}
> +
> +static ssize_t get_fan2(struct device *dev, struct device_attribute *devattr,
> + char *buf)
> +{
> + return get_fan(dev,devattr,buf,2);
> +}
> +
> +static ssize_t get_fan3(struct device *dev, struct device_attribute *devattr,
> + char *buf)
> +{
> + return get_fan(dev,devattr,buf,3);
> +}
> +
> +/* Show values of the config register for debugging purposes
> + */
> +
> +static ssize_t get_config(struct device *dev, struct device_attribute *devattr,
> + char *buf)
> +{
> + struct max6650_data *data = max6650_update_device(dev);
> +
> + return sprintf(buf, "mode: %s%s%s%s, voltage:%s prescale: %d\n",
> + ((data->config & MAX6650_CFG_MODE_MASK) ==
> + MAX6650_CFG_MODE_ON) ? "On" : "",
> + ((data->config & MAX6650_CFG_MODE_MASK) ==
> + MAX6650_CFG_MODE_OFF) ? "Off" : "",
> + ((data->config & MAX6650_CFG_MODE_MASK) ==
> + MAX6650_CFG_MODE_CLOSED_LOOP) ? "closed loop" : "",
> + ((data->config & MAX6650_CFG_MODE_MASK) ==
> + MAX6650_CFG_MODE_OPEN_LOOP) ? "open Loop" : "",
> + (data->config & MAX6650_CFG_V12) ? "12V" : "5V",
> + DIV_FROM_REG(data->config)
> + );
> +}
> +
> +/* Returns count time in ms */
> +static ssize_t get_count(struct device *dev, struct device_attribute *devattr,
> + char *buf)
> +{
> + struct max6650_data *data = max6650_update_device(dev);
> +
> + return sprintf(buf, "%d\n", DIV_FROM_REG(data->count) * 250 );
> +}
> +
> +/*
> + * Set the fan speed to the specified RPM (or read back the RPM setting).
> + *
> + * The MAX6650/1 will automatically control fan speed when in closed loop
> + * mode.
> + *
> + * Assumptions:
> + *
> + * 1) The MAX6650/1 is running from its internal 254kHz clock (perhaps
> + * this should be made a module parameter).
> + *
> + * 2) The prescaler (low three bits of the config register) has already
> + * been set to an appropriate value.
> + *
> + * The relevant equations are given on pages 21 and 22 of the datasheet.
> + *
> + * From the datasheet, the relevant equation when in regulation is:
> + *
> + * [fCLK / (128 x (KTACH + 1))] = 2 x FanSpeed / KSCALE
> + *
> + * where:
> + *
> + * fCLK is the oscillator frequency (either the 254kHz internal
> + * oscillator or the externally applied clock)
> + *
> + * KTACH is the value in the speed register
> + *
> + * FanSpeed is the speed of the fan in rps
> + *
> + * KSCALE is the prescaler value (1, 2, 4, 8, or 16)
> + *
> + * When reading, we need to solve for FanSpeed. When writing, we need to
> + * solve for KTACH.
> + *
> + * Note: this tachometer is completely separate from the tachometers
> + * used to measure the fan speeds. Only one fan's speed (fan1) is
> + * controlled.
> + */
> +
> +static ssize_t get_speed(struct device *dev, struct device_attribute *devattr,
> + char *buf)
> +{
> + struct max6650_data *data = max6650_update_device(dev);
> + int kscale, ktach, fclk, rpm;
> +
> + /*
> + * Use the datasheet equation:
> + *
> + * FanSpeed = KSCALE x fCLK / [256 x (KTACH + 1)]
> + *
> + * then multiply by 60 to give rpm.
> + */
> +
> + kscale = DIV_FROM_REG(data->config);
> + ktach = data->speed;
> + fclk = MAX6650_INT_CLK;
> + rpm = 60 * kscale * fclk / (256 * (ktach + 1));
> + return sprintf(buf, "%d\n", rpm);
> +}
> +
> +static ssize_t set_speed(struct device *dev, struct device_attribute *devattr,
> + const char *buf, size_t count)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct max6650_data *data = i2c_get_clientdata(client);
> + int rpm = simple_strtoul(buf, NULL, 10);
> + int kscale, ktach, fclk;
> +
> + dev_dbg(dev, "%s called, rpm=%d\n", __FUNCTION__, rpm);
> +
> + mutex_lock(&data->update_lock);
> + if (rpm == 0)
> + {
> + /* Switch totally off */
> + data->speed = 0;
> + data->config = (data->config & ~MAX6650_CFG_MODE_MASK)
> + | MAX6650_CFG_MODE_OFF;
> + }
> + else
> + {
> + rpm = SENSORS_LIMIT(rpm, FAN_RPM_MIN, FAN_RPM_MAX);
> +
> + /*
> + * Divide the required speed by 60 to get from rpm to rps, then
> + * use the datasheet equation:
> + *
> + * KTACH = [(fCLK x KSCALE) / (256 x FanSpeed)] - 1
> + */
> +
> + kscale = DIV_FROM_REG(data->config);
> + fclk = MAX6650_INT_CLK;
> + ktach = ((fclk * kscale) / (256 * rpm / 60)) - 1;
> +
> + data->speed = ktach;
> + data->config = (data->config & ~MAX6650_CFG_MODE_MASK)
> + | MAX6650_CFG_MODE_CLOSED_LOOP;
> + }
> + i2c_smbus_write_byte_data(client, MAX6650_REG_CONFIG, data->config);
> + i2c_smbus_write_byte_data(client, MAX6650_REG_SPEED, data->speed);
> +
> + mutex_unlock(&data->update_lock);
> +
> + return count;
> +}
> +
> +static DEVICE_ATTR(fan0, S_IRUGO, get_fan0, NULL);
> +static DEVICE_ATTR(fan1, S_IRUGO, get_fan1, NULL);
> +static DEVICE_ATTR(fan2, S_IRUGO, get_fan2, NULL);
> +static DEVICE_ATTR(fan3, S_IRUGO, get_fan3, NULL);
> +static DEVICE_ATTR(count, S_IRUGO, get_count, NULL);
> +static DEVICE_ATTR(config, S_IRUGO, get_config, NULL);
> +static DEVICE_ATTR(speed, S_IWUSR | S_IRUGO, get_speed, set_speed);
> +
> +static struct attribute *max6650_attrs[] = {
> + &dev_attr_fan0.attr,
> + &dev_attr_fan1.attr,
> + &dev_attr_fan2.attr,
> + &dev_attr_fan3.attr,
> + &dev_attr_count.attr,
> + &dev_attr_config.attr,
> + &dev_attr_speed.attr,
> + NULL,
> +};
> +
> +static struct attribute_group max6650_attr_grp = {
> + .attrs = max6650_attrs,
> +};
> +
> +/*
> + * Real code
> + */
> +
> +static int max6650_attach_adapter(struct i2c_adapter *adapter)
> +{
> + dev_dbg(adapter->dev, "max6650_attach_adapter called for %s\n",
> + (char*)&adapter->name);
> +
> + if (!(adapter->class & I2C_CLASS_HWMON)) {
> + dev_dbg(adapter->dev,
> + "FATAL: max6650_attach_adapter class HWMON not set\n");
> + return 0;
> + }
> +
> + return i2c_probe(adapter, &addr_data, max6650_detect);
> +}
> +
> +/*
> + * The following function does more than just detection. If detection
> + * succeeds, it also registers the new chip.
> + */
> +
> +static int max6650_detect(struct i2c_adapter *adapter, int address, int kind)
> +{
> + struct i2c_client *new_client;
> + struct max6650_data *data;
> + int err = -ENODEV;
> + const char *name = "";
> +
> + dev_dbg(adapter->dev, "max6650_detect called, kind = %d\n",kind);
> +
> + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) {
> + dev_dbg(adapter->dev, "MAX6650: I2C bus doesn't support byte"
> + "read mode, skipping.\n");
> + return 0;
> + }
> +
> + if (!(data = kzalloc(sizeof(struct max6650_data), GFP_KERNEL))) {
> + printk("MAX6650: Out of memory in max6650_detect "
> + "(new_client).\n");
> + return -ENOMEM;
> + }
> +
> + /*
> + * The common I2C client data is placed right before the
> + * max6650-specific data. The max6650-specific data is pointed to by
> + * the data field from the I2C client data.
> + */
> +
> + new_client = &data->client;
> + i2c_set_clientdata(new_client, data);
> + new_client->addr = address;
> + new_client->adapter = adapter;
> + new_client->driver = &max6650_driver;
> + new_client->flags = 0;
> +
> + /*
> + * Now we do the remaining detection. A negative kind means that
> + * the driver was loaded with no force parameter (default), so we
> + * must both detect and identify the chip (actually there is only
> + * one possible kind of chip for now, max6650). A zero kind means that
> + * the driver was loaded with the force parameter, the detection
> + * step shall be skipped. A positive kind means that the driver
> + * was loaded with the force parameter and a given kind of chip is
> + * requested, so both the detection and the identification steps
> + * are skipped.
> + *
> + * Currently I can find no way to distinguish between a MAX6650 and
> + * a MAX6651. This driver has only been tried on the latter.
> + */
> +
> + if ((kind < 0)&&
> + ( (i2c_smbus_read_byte_data(new_client,MAX6650_REG_CONFIG) & 0xC0)
> + ||(i2c_smbus_read_byte_data(new_client,MAX6650_REG_GPIO_STAT)& 0xE0)
> + ||(i2c_smbus_read_byte_data(new_client,MAX6650_REG_ALARM_EN) & 0xE0)
> + ||(i2c_smbus_read_byte_data(new_client,MAX6650_REG_ALARM) & 0xE0)
> + ||(i2c_smbus_read_byte_data(new_client,MAX6650_REG_COUNT) & 0xFC)))
> + {
> + dev_dbg(adapter->dev, "max6650.o: max6650 detection failed"
> + "at 0x%02x.\n", address);
> +
> + goto err_free;
> + }
> +
> + /* Configure HW-related voltage setting */
> + if (voltage_12V) {
> + i2c_smbus_write_byte_data(new_client, MAX6650_REG_CONFIG,
> + i2c_smbus_read_byte_data(new_client, MAX6650_REG_CONFIG)
> + | MAX6650_CFG_V12);
> + }
> + else {
> + i2c_smbus_write_byte_data(new_client, MAX6650_REG_CONFIG,
> + i2c_smbus_read_byte_data(new_client, MAX6650_REG_CONFIG)
> + & ~MAX6650_CFG_V12);
> + }
> +
> + if (kind <= 0) {
> + kind = max6650;
> + }
> +
> + if (kind == max6650) {
> + name = "max6650";
> + printk("MAX6650: Chip found at 0x%02x.\n", address);
> + }
> + else {
> + printk("MAX6650: Unsupported chip.\n");
> + goto err_free;
> + }
> +
> + /*
> + * OK, we got a valid chip so we can fill in the remaining client
> + * fields.
> + */
> +
> + strlcpy(new_client->name, name, I2C_NAME_SIZE);
> + data->valid = 0;
> + mutex_init(&data->update_lock);
> +
> + /*
> + * Tell the I2C layer a new client has arrived.
> + */
> +
> + if ((err = i2c_attach_client(new_client))) {
> + dev_dbg(adapter->dev, "max6650.o: Failed attaching client.\n");
> + goto err_free;
> + }
> +
> + /*
> + * Initialize the max6650 chip
> + */
> + max6650_init_client(new_client);
> +
> + /* Register sysfs hooks */
> + data->class_dev = hwmon_device_register(&new_client->dev);
> + if (IS_ERR(data->class_dev)) {
> + err = PTR_ERR(data->class_dev);
> + goto err_detach;
> + }
> +
> + err = sysfs_create_group(&new_client->dev.kobj, &max6650_attr_grp);
> + if (!err)
> + return 0;
> +
> + dev_err(&new_client->dev, "error creating sysfs files (%d)\n", err);
> + hwmon_device_unregister(data->class_dev);
> +err_detach:
> + i2c_detach_client(new_client);
> +err_free:
> + kfree(data);
> + return err;
> +}
> +
> +static int max6650_detach_client(struct i2c_client *client)
> +{
> + struct max6650_data *data = i2c_get_clientdata(client);
> + int err;
> + hwmon_device_unregister(data->class_dev);
> + err = i2c_detach_client(client);
> + sysfs_remove_group(&client->dev.kobj, &max6650_attr_grp);
> + kfree(data);
> + return err;
> +}
> +
> +static void max6650_init_client(struct i2c_client *client)
> +{
> + /* Nothing to do here - assume the BIOS has initialized the chip */
> +}
> +
> +static const u8 tach_reg[] =
> +{
> + MAX6650_REG_TACH0,
> + MAX6650_REG_TACH1,
> + MAX6650_REG_TACH2,
> + MAX6650_REG_TACH3,
> +};
> +
> +static struct max6650_data *max6650_update_device(struct device *dev)
> +{
> + int i;
> + struct i2c_client *client = to_i2c_client(dev);
> + struct max6650_data *data = i2c_get_clientdata(client);
> +
> + mutex_lock(&data->update_lock);
> +
> + if (time_after(jiffies, data->last_updated + HZ) || !data->valid) {
> + data->speed = i2c_smbus_read_byte_data(client,
> + MAX6650_REG_SPEED);
> + data->config = i2c_smbus_read_byte_data(client,
> + MAX6650_REG_CONFIG);
> + for (i = 0; i < 4; i++) {
> + data->tach[i] = i2c_smbus_read_byte_data(client,
> + tach_reg[i]);
> + }
> + data->count = i2c_smbus_read_byte_data (client,
> + MAX6650_REG_COUNT);
> + data->last_updated = jiffies;
> + data->valid = 1;
> + }
> +
> + mutex_unlock(&data->update_lock);
> +
> + return data;
> +}
> +
> +static int __init sensors_max6650_init(void)
> +{
> + return i2c_add_driver(&max6650_driver);
> +}
> +
> +static void __exit sensors_max6650_exit(void)
> +{
> + i2c_del_driver(&max6650_driver);
> +}
> +
> +MODULE_AUTHOR("john.morris at spirentcom.com");
> +MODULE_DESCRIPTION("max6650 sensor driver");
> +MODULE_LICENSE("GPL");
> +
> +module_init(sensors_max6650_init);
> +module_exit(sensors_max6650_exit);
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> lm-sensors mailing list
> lm-sensors at lm-sensors.org
> http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
hello
Instead of declaring four static DEVICE_ATTR(fan1, S_IRUGO, get_fan1, NULL);
I'm suggest you to use
static SENSOR_DEVICE_ATTR(fan1, S_IRUGO, get_fan, NULL,1);
static SENSOR_DEVICE_ATTR(fan2, S_IRUGO, get_fan, NULL,2);
etc....
secondly in /Documentation/hwmon/max6650 you have a wrong prefix
+Supported chips:
> + * Maxim 6650 / 6651
> + Prefix: 'w83792d'
and 2 typos errors
(but and not bit)
-> bit the Maxim 6550 has a reduced feature
6550 not 6650
> + * Maxim 6650 / 6651
> + Prefix: 'w83792d'
