On 2/21/2010 2:22 AM, Jaswinder Singh Rajput wrote:
Hello George, I tested on Intel WX58BP with Intel Core i7, it works great, thanks for the patch. Here are few knits :
All knits un-knitted. Patch to follow.
From: George Joseph<george.joseph@xxxxxxxxxxxxx> Hwmon driver for Andigilog aSC7621 family monitoring chips. Signed-off-by: George Joseph<george.joseph@xxxxxxxxxxxxx> --- Patch against 2.6.33-rc6. Documentation/hwmon/asc7621 | 295 ++++++++++ MAINTAINERS | 7 drivers/hwmon/Kconfig | 13 drivers/hwmon/Makefile | 1 drivers/hwmon/asc7621.c | 1266 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 1582 insertions(+) diff -uprN a/Documentation/hwmon/asc7621 b/Documentation/hwmon/asc7621 --- a/Documentation/hwmon/asc7621 1969-12-31 17:00:00.000000000 -0700 +++ b/Documentation/hwmon/asc7621 2010-01-30 12:43:13.000000000 -0700 @@ -0,0 +1,295 @@ +Kernel driver asc7621 +================== + +Supported chips: + Andigilog aSC7621 and aSC7621a + Prefix: 'asc7621' + Addresses scanned: I2C 0x2c, 0x2d, 0x2e + Datasheet: http://www.andigilog.com/downloads/aSC7621_70A06010.pdf +This link does not exist, if possible provide this datasheet to Jean so that he can host it on lm-sensors website to use as reference.+Author: + George Joseph + +Description provided by Dave Pivin @ Andigilog: + +Andigilog has both the PECI and pre-PECI versions of the Heceta-6, as +Intel calls them. Heceta-6e has high frequency PWM and Heceta-6p has +added PECI and a 4th thermal zone. The Andigilog aSC7611 is the +Heceta-6e part and aSC7621 is the Heceta-6p part. They are both in +volume production, shipping to Intel and their subs. + +We have enhanced both parts relative to the governing Intel +specification. First enhancement is temperature reading resolution. We +have used registers below 20h for vendor-specific functions in addition +to those in the Intel-specified vendor range. + +Our conversion process produces a result that is reported as two bytes. +The fan speed control uses this finer value to produce a "step-less" fan +PWM output. These two bytes are "read-locked" to guarantee that once a +high or low byte is read, the other byte is locked-in until after the +next read of any register. So to get an atomic reading, read high or low +byte, then the very next read should be the opposite byte. Our data +sheet says 10-bits of resolution, although you may find the lower bits +are active, they are not necessarily reliable or useful externally. We +chose not to mask them. + +We employ significant filtering that is user tunable as described in the +data sheet. Our temperature reports and fan PWM outputs are very smooth +when compared to the competition, in addition to the higher resolution +temperature reports. The smoother PWM output does not require user +intervention. + +We offer GPIO features on the former VID pins. These are open-drain +outputs or inputs and may be used as general purpose I/O or as alarm +outputs that are based on temperature limits. These are in 19h and 1Ah. + +We offer flexible mapping of temperature readings to thermal zones. Any +temperature may be mapped to any zone, which has a default assignment +that follows Intel's specs. + +Since there is a fan to zone assignment that allows for the "hotter" of +a set of zones to control the PWM of an individual fan, but there is no +indication to the user, we have added an indicator that shows which zone +is currently controlling the PWM for a given fan. This is in register +00h. + +Both remote diode temperature readings may be given an offset value such +that the reported reading as well as the temperature used to determine +PWM may be offset for system calibration purposes. + +PECI Extended configuration allows for having more than two domains per +PECI address and also provides an enabling function for each PECI +address. One could use our flexible zone assignment to have a zone +assigned to up to 4 PECI addresses. This is not possible in the default +Intel configuration. This would be useful in multi-CPU systems with +individual fans on each that would benefit from individual fan control. +This is in register 0Eh. + +The tachometer measurement system is flexible and able to adapt to many +fan types. We can also support pulse-stretched PWM so that 3-wire fans +may be used. These characteristics are in registers 04h to 07h. + +Finally, we have added a tach disable function that turns off the tach +measurement system for individual tachs in order to save power. That is +in register 75h. + +-- +aSC7621 Product Description + +The aSC7621 has a two wire digital interface compatible with SMBus 2.0. +Using a 10-bit ADC, the aSC7621 measures the temperature of two remote diode +connected transistors as well as its own die. Support for Platform +Environmental Control Interface (PECI) is included. + +Using temperature information from these four zones, an automatic fan speed +control algorithm is employed to minimize acoustic impact while achieving +recommended CPU temperature under varying operational loads. + +To set fan speed, the aSC7621 has three independent pulse width modulation +(PWM) outputs that are controlled by one, or a combination of three, +temperature zones. Both high- and low-frequency PWM ranges are supported. + +The aSC7621 also includes a digital filter that can be invoked to smooth +temperature readings for better control of fan speed and minimum acoustic +impact. + +The aSC7621 has tachometer inputs to measure fan speed on up to four fans. +Limit and status registers for all measured values are included to alert +the system host that any measurements are outside of programmed limits +via status registers. + +System voltages of VCCP, 2.5V, 3.3V, 5.0V, and 12V motherboard power are +monitored efficiently with internal scaling resistors. + +Features +- Supports PECI interface and monitors internal and remote thermal diodes +- 2-wire, SMBus 2.0 compliant, serial interface +- 10-bit ADC +- Monitors VCCP, 2.5V, 3.3V, 5.0V, and 12V motherboard/processor supplies +- Programmable autonomous fan control based on temperature readings +- Noise filtering of temperature reading for fan speed control +- 0.25C digital temperature sensor resolution +- 3 PWM fan speed control outputs for 2-, 3- or 4-wire fans and up to 4 fan + tachometer inputs +- Enhanced measured temperature to Temperature Zone assignment. +- Provides high and low PWM frequency ranges +- 3 GPIO pins for custom use +- 24-Lead QSOP package + +Configuration Notes +=================== + +Except where noted below, the sysfs entries created by this driver follow +the standards defined in "sysfs-interface". + +temp1_source + 0 (default) peci_legacy = 0, Remote 1 Temperature + peci_legacy = 1, PECI Processor Temperature 0 + 1 Remote 1 Temperature + 2 Remote 2 Temperature + 3 Internal Temperature + 4 PECI Processor Temperature 0 + 5 PECI Processor Temperature 1 + 6 PECI Processor Temperature 2 + 7 PECI Processor Temperature 3 + +temp2_source + 0 (default) Internal Temperature + 1 Remote 1 Temperature + 2 Remote 2 Temperature + 3 Internal Temperature + 4 PECI Processor Temperature 0 + 5 PECI Processor Temperature 1 + 6 PECI Processor Temperature 2 + 7 PECI Processor Temperature 3 + +temp3_source + 0 (default) Remote 2 Temperature + 1 Remote 1 Temperature + 2 Remote 2 Temperature + 3 Internal Temperature + 4 PECI Processor Temperature 0 + 5 PECI Processor Temperature 1 + 6 PECI Processor Temperature 2 + 7 PECI Processor Temperature 3 + +temp4_source + 0 (default) peci_legacy = 0, PECI Processor Temperature 0 + peci_legacy = 1, Remote 1 Temperature + 1 Remote 1 Temperature + 2 Remote 2 Temperature + 3 Internal Temperature + 4 PECI Processor Temperature 0 + 5 PECI Processor Temperature 1 + 6 PECI Processor Temperature 2 + 7 PECI Processor Temperature 3 + +temp[1-4]_smoothing_enable +temp[1-4]_smoothing_time + Smooths spikes in temp readings caused by noise. + 0 35 sec + 1 17.6 sec + 2 11.8 sec + 3 7.0 sec + 4 4.4 sec + 5 3.0 sec + 6 1.6 sec + 7 0.8 sec + +temp[1-4]_crit + When the corresponding zone temperature reaches this value, + ALL pwm outputs will got to 100%. + +temp[5-8]_input +temp[5-8]_enable + The aSC7621 can also read temperatures provided by the processor + via the PECI bus. Usually these are "core" temps and are relative + to the point where the automatical thermal control circuit startsFix the typo : + to the point where the automatically thermal control circuit starts+ throttling. This means that these are usually negative numbers. + +pwm[1-3]_enable + 0 Fan off. + 1 Fan on manual control. + 2 Fan on automatic control and will run at the minimum pwm + if the temperature for the zone is below the minimum. + 3 Fan on automatic control but will be off if the temperature + for the zone is below the minimum. + 4-254 Ignored. + 255 Fan on full. + +pwm[1-3]_auto_channels + Bitmap as described in sysctl-interface with the following + exceptions... + Only the following combinations of zones (and their corresponding masks)+ Only the following combination of zones (and their corresponding masks)+ are valid: + 1 + 2 + 3 + 2,3 + 1,2,3 + 4 + 1,2,3,4 + + Special values: + 0 Disabled. + 16 Fan on manual control. + 31 Fan on full. + + +pwm[1-3]_invert + When set, inverts the meaning of pwm[1-3]. + I.E. when pwm = 0, the fan will be on full andI.E. is used for Internet Explorer, I think you mean : + i.e. when pwm = 0, the fan will be on full and+ when pwm = 255 the fan will be off. + +pwm[1-3]_freq + PWM frequency in Hz + Valid values in Hz are: + + 10 + 15 + 23 + 30 (default) + 38 + 47 + 62 + 94 + 23000 + 24000 + 25000 + 26000 + 27000 + 28000 + 29000 + 30000 + + Setting any other value will be ignored. + +peci_enable + Enables or disables PECI + +peci_avg + Input filter averate time.Fix this typo : + Input filter average time.+ + 0 0 Sec. (no Smoothing) (default) + 1 0.25 Sec. + 2 0.5 Sec. + 3 1.0 Sec. + 4 2.0 Sec. + 5 4.0 Sec. + 6 8.0 Sec. + 7 0.0 Sec. + +peci_legacy + + 0 Standard Mode (default) + Remote Diode 1 reading is associated with + Temperature Zone 1, PECI is associated with + Zone 4 + + 1 Legacy Mode + PECI is associated with Temperature Zone 1, + Remote Diode 1 is associated with Zone 4 + +peci_diode + Diode filter + + 0 0.25 Sec. + 1 1.1 Sec. + 2 2.4 Sec. (default) + 3 3.4 Sec. + 4 5.0 Sec. + 5 6.8 Sec. + 6 10.2 Sec. + 7 16.4 Sec. + +peci_4domain + Four domain enable + + 0 1 or 2 Domains for enabled processors (default) + 1 3 or 4 Domains for enabled processors + +peci_domain + Domain + + 0 Processor contains a single domain (0) (default) + 1 Processor contains two domains (0,1) diff -uprN a/drivers/hwmon/asc7621.c b/drivers/hwmon/asc7621.c --- a/drivers/hwmon/asc7621.c 1969-12-31 17:00:00.000000000 -0700 +++ b/drivers/hwmon/asc7621.c 2010-01-30 14:53:56.000000000 -0700 @@ -0,0 +1,1266 @@ +/* + asc7621.c - Part of lm_sensors, Linux kernel modules for hardware monitoring + Copyright (c) 2007, 2010 George Joseph<george.joseph@xxxxxxxxxxxxx> + + Chip details at<http://www.andigilog.com> +Please keep it consistent and this web site is showing wrong information replace above 5 lines with : + * asc7621.c - Part of lm_sensors, Linux kernel modules for hardware monitoring + * Copyright (C) 2007, 2010 George Joseph<george.joseph@xxxxxxxxxxxxx> + *+ * 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> + +/* Addresses to scan */ +static unsigned short normal_i2c[] = { + 0x2c, 0x2d, 0x2e, I2C_CLIENT_END +}; + +enum asc7621_type { + asc7621, + asc7621a +}; + +#define INTERVAL_HIGH (HZ + HZ / 2) +#define INTERVAL_LOW (1 * 60 * HZ) +#define PRI_NONE 0 +#define PRI_LOW 1 +#define PRI_HIGH 2 +#define FIRST_CHIP asc7621 +#define LAST_CHIP asc7621aPlease aligned it to make it more readable : +#define INTERVAL_HIGH (HZ + HZ / 2) +#define INTERVAL_LOW (1 * 60 * HZ) +#define PRI_NONE 0 +#define PRI_LOW 1 +#define PRI_HIGH 2 +#define FIRST_CHIP asc7621 +#define LAST_CHIP asc7621a+ +struct asc7621_chip { + char *name; + enum asc7621_type chip_type; + u8 company_reg; + u8 company_id; + u8 verstep_reg; + u8 verstep_id; + unsigned short *addresses; +}; + +static struct asc7621_chip asc7621_chips[] = { + { + .name = "asc7621", + .chip_type = asc7621, + .company_reg = 0x3e, + .company_id = 0x61, + .verstep_reg = 0x3f, + .verstep_id = 0x6c, + .addresses = normal_i2c,Please also align this : + .name = "asc7621", + .chip_type = asc7621, + .company_reg = 0x3e, + .company_id = 0x61, + .verstep_reg = 0x3f, + .verstep_id = 0x6c, + .addresses = normal_i2c,+ }, + { + .name = "asc7621a", + .chip_type = asc7621a, + .company_reg = 0x3e, + .company_id = 0x61, + .verstep_reg = 0x3f, + .verstep_id = 0x6d, + .addresses = normal_i2c,ditto : + .name = "asc7621a", + .chip_type = asc7621a, + .company_reg = 0x3e, + .company_id = 0x61, + .verstep_reg = 0x3f, + .verstep_id = 0x6d, + .addresses = normal_i2c,+ }, +}; + +/* + * Defines the highest register to be used, not the count. + * The actual count will probably be smaller because of gaps + * in the implementation (unused register locations). + * This define will safely set the array size of both the parameter + * and data arrays. + * This comes from the data sheet register description table. + */ +#define LAST_REGISTER 0xff + +struct asc7621_data { + struct i2c_client client; + struct device *class_dev; + struct mutex update_lock; + int valid; /* !=0 if following fields are valid */ + unsigned long last_high_reading; /* In jiffies */ + unsigned long last_low_reading; /* In jiffies */ditto : + unsigned long last_low_reading; /* In jiffies */+ /* + * Registers we care about occupy the corresponding index + * in the array. Registers we don't care about are left + * at 0. + */ + u8 reg[LAST_REGISTER + 1]; +}; + +/* + * Macro to get the parent asc7621_param structure + * from a sensor_device_attribute passed into the + * show/store functions. + */ +#define to_asc7621_param(_sda) \ + container_of(_sda, struct asc7621_param, sda) + +/* + * Each parameter to be retrieved needs an asc7621_param structure + * allocated. It contains the sensor_device_attribute structure + * and the control info needed to retrieve the value from the register map. + */ +struct asc7621_param { + struct sensor_device_attribute sda; + u8 priority; + u8 msb[3]; + u8 lsb[3]; + u8 mask[3]; + u8 shift[3]; +}; + +/* + * This is the map that ultimately indicates whether we'll be + * retrieving a register value or not, and at what frequency. + */ +static u8 asc7621_register_priorities[255]; + +static struct asc7621_data *asc7621_update_device(struct device *dev); + +#define read_byte(reg) (i2c_smbus_read_byte_data(client, reg)& 0xff) +#define write_byte(reg, data) i2c_smbus_write_byte_data(client, reg, data)Please also align this : +#define read_byte(reg) (i2c_smbus_read_byte_data(client, reg)& 0xff) +#define write_byte(reg, data) i2c_smbus_write_byte_data(client, reg, data)+ +/* + * Data Handlers + * Each function handles the formatting, storage + * and retrieval of like parameters. + */ + +#define SETUP_SHOW_data_param(d, a) \ + struct sensor_device_attribute *sda = to_sensor_dev_attr(a); \ + struct asc7621_data *data = asc7621_update_device(d); \ + struct asc7621_param *param = to_asc7621_param(sda) + +#define SETUP_STORE_data_param(d, a) \ + struct sensor_device_attribute *sda = to_sensor_dev_attr(a); \ + struct i2c_client *client = to_i2c_client(d); \ + struct asc7621_data *data = i2c_get_clientdata(client); \ + struct asc7621_param *param = to_asc7621_param(sda) + +/* + * u8 is just what it sounds like...an unsigned byte with no + * special formatting. + */ +static ssize_t show_u8(struct device *dev, struct device_attribute *attr, + char *buf) +{ + SETUP_SHOW_data_param(dev, attr); + + return sprintf(buf, "%u\n", data->reg[param->msb[0]]); +} + +static ssize_t store_u8(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + SETUP_STORE_data_param(dev, attr); + long reqval; + + if (strict_strtol(buf, 10,&reqval)) + return -EINVAL; + + reqval = SENSORS_LIMIT(reqval, 0, 255); + + mutex_lock(&data->update_lock); + data->reg[param->msb[0]] = reqval; + write_byte(param->msb[0], reqval); + mutex_unlock(&data->update_lock); + return count; +} + +/* + * Many of the config values occupy only a few bits of a register. + */ +static ssize_t show_bitmask(struct device *dev, + struct device_attribute *attr, char *buf) +{ + SETUP_SHOW_data_param(dev, attr); + + return sprintf(buf, "%u\n", + (data->reg[param->msb[0]]>> param-> + shift[0])& param->mask[0]); +} + +static ssize_t store_bitmask(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + SETUP_STORE_data_param(dev, attr); +remove above line.+ long reqval; + u8 currval; + + if (strict_strtol(buf, 10,&reqval)) + return -EINVAL; + + reqval = SENSORS_LIMIT(reqval, 0, param->mask[0]); + + reqval = (reqval& param->mask[0])<< param->shift[0]; + + mutex_lock(&data->update_lock); + currval = read_byte(param->msb[0]); + reqval |= (currval& ~(param->mask[0]<< param->shift[0])); + data->reg[param->msb[0]] = reqval; + write_byte(param->msb[0], reqval); + mutex_unlock(&data->update_lock); + return count; +} + +/* + * 16 bit fan rpm values + * reported by the device as the number of 11.111us periods (90khz) + * between full fan rotations. Therefore... + * RPM = (90000 * 60) / register value + */ +static ssize_t show_fan16(struct device *dev, + struct device_attribute *attr, char *buf) +{ + SETUP_SHOW_data_param(dev, attr); + u16 regval; + + mutex_lock(&data->update_lock); + regval = (data->reg[param->msb[0]]<< 8) | data->reg[param->lsb[0]]; + mutex_unlock(&data->update_lock); + + return sprintf(buf, "%u\n", + (regval == 0 ? -1 : (regval) == + 0xffff ? 0 : 5400000 / regval)); +} + +static ssize_t store_fan16(struct device *dev, + struct device_attribute *attr, const char *buf, + size_t count) +{ + SETUP_STORE_data_param(dev, attr); + long reqval; + + if (strict_strtol(buf, 10,&reqval)) + return -EINVAL; + + reqval = + (SENSORS_LIMIT((reqval)<= 0 ? 0 : 5400000 / (reqval), 0, 65534)); + + mutex_lock(&data->update_lock); + data->reg[param->msb[0]] = (reqval>> 8)& 0xff; + data->reg[param->lsb[0]] = reqval& 0xff; + write_byte(param->msb[0], data->reg[param->msb[0]]); + write_byte(param->lsb[0], data->reg[param->lsb[0]]); + mutex_unlock(&data->update_lock); + + return count; +} + +/* + * Voltages are scaled in the device so that the nominal voltage + * is 3/4ths of the 0-255 range (I.E 192).you mean : + * is 3/4ths of the 0-255 range (i.e. 192).+ * If all voltages are 'normal' then all voltage registers will + * read 0xC0. This doesn't help us if we don't have a point of refernce. + * The data sheet however provides us with the full scale value for each + * which is stored in in_scaling. The sda->index parameter value provides + * the index into in_scaling. + * + * NOTE: The chip expects the first 2 inputs be 2.5 and 2.25 volts + * respectively. That doesn't mean that's what the motherboard provides. :) + */ + +static int asc7621_in_scaling[] = { + 3320, 3000, 4380, 6640, 16000 +}; + +static ssize_t show_in10(struct device *dev, struct device_attribute *attr, + char *buf) +{ + SETUP_SHOW_data_param(dev, attr); + u16 regval; + u8 nr = sda->index; + + mutex_lock(&data->update_lock); + regval = (data->reg[param->msb[0]] * asc7621_in_scaling[nr]) / 256; + + /* The LSB value is a 2-bit scaling of the MSB's LSbit value. + * I.E. If the maximim voltage for this input is 6640 millivolts then + * a MSB register value of 0 = 0mv and 255 = 6640mv. + * A 1 step change therefore represents 25.9mv (6640 / 256). + * The extra 2-bits therefore represent increments of 6.48mv. + */ + regval += ((asc7621_in_scaling[nr] / 256) / 4) * + (data->reg[param->lsb[0]]>> 6); + + mutex_unlock(&data->update_lock); + + return sprintf(buf, "%u\n", regval); +} + +/* 8 bit voltage values (the mins and maxs) */ +static ssize_t show_in8(struct device *dev, struct device_attribute *attr, + char *buf) +{ + SETUP_SHOW_data_param(dev, attr); +<---remove this line.+ u8 nr = sda->index; + + return sprintf(buf, "%u\n", + ((data->reg[param->msb[0]] * + asc7621_in_scaling[nr]) / 256)); +} + +static ssize_t store_in8(struct device *dev, struct device_attribute *attr, + const char *buf, size_t count) +{ + SETUP_STORE_data_param(dev, attr); + long reqval; + u8 nr = sda->index; + + if (strict_strtol(buf, 10,&reqval)) + return -EINVAL; + + reqval = SENSORS_LIMIT(reqval, 0, asc7621_in_scaling[nr]); + + reqval = (reqval * 256) / asc7621_in_scaling[nr]; + + mutex_lock(&data->update_lock); + data->reg[param->msb[0]] = reqval; + write_byte(param->msb[0], reqval); + mutex_unlock(&data->update_lock); + + return count; +} + +static ssize_t show_temp8(struct device *dev, + struct device_attribute *attr, char *buf) +{ + SETUP_SHOW_data_param(dev, attr); + + return sprintf(buf, "%d\n", ((s8) data->reg[param->msb[0]]) * 1000); +} + +static ssize_t store_temp8(struct device *dev, + struct device_attribute *attr, const char *buf, + size_t count) +{ + SETUP_STORE_data_param(dev, attr); + long reqval; + s8 temp; + + if (strict_strtol(buf, 10,&reqval)) + return -EINVAL; + + reqval = SENSORS_LIMIT(reqval, -127000, 127000); + + temp = reqval / 1000; + + mutex_lock(&data->update_lock); + data->reg[param->msb[0]] = temp; + write_byte(param->msb[0], temp); + mutex_unlock(&data->update_lock); + return count; +} + +/* + * Temperatures that occupy 2 bytes always have the whole + * number of degrees in the MSB with some part of the LSB + * indicating fractional degrees. + */ + +/* mmmmmmmm.llxxxxxx */ +static ssize_t show_temp10(struct device *dev, + struct device_attribute *attr, char *buf) +{ + SETUP_SHOW_data_param(dev, attr); + u8 msb; + u8 lsb;save one line by : + u8 msb, lsb;+ int temp; + + mutex_lock(&data->update_lock); + msb = data->reg[param->msb[0]]; + lsb = (data->reg[param->lsb[0]]>> 6)& 0x03; + temp = (((s8) msb) * 1000) + (lsb * 250); + mutex_unlock(&data->update_lock); + + return sprintf(buf, "%d\n", temp); +} + +/* mmmmmm.ll */ +static ssize_t show_temp62(struct device *dev, + struct device_attribute *attr, char *buf) +{ + SETUP_SHOW_data_param(dev, attr); +<--save one more line by remove this line.+ u8 regval = data->reg[param->msb[0]]; + int temp = ((s8) (regval& 0xfc) * 1000) + ((regval& 0x03) * 250); + + return sprintf(buf, "%d\n", temp); +} + +static ssize_t store_temp62(struct device *dev, + struct device_attribute *attr, const char *buf, + size_t count) +{ + SETUP_STORE_data_param(dev, attr); + long reqval; + long i, f;save one more line : + long reqval, i, f;+ s8 temp; + + if (strict_strtol(buf, 10,&reqval)) + return -EINVAL; + + reqval = SENSORS_LIMIT(reqval, -32000, 31750); + i = reqval / 1000; + f = reqval - (i * 1000); + temp = i<< 2; + temp |= f / 250; + + mutex_lock(&data->update_lock); + data->reg[param->msb[0]] = temp; + write_byte(param->msb[0], temp); + mutex_unlock(&data->update_lock); + return count; +} + +/* + * The aSC7621 doesn't provide an "auto_point2". Instead, you + * specify the auto_point1 and a range. To keep with the sysfs + * hwmon specs, we synthesize the auto_point_2 from them. + */ + +static u32 asc7621_range_map[] = { + 2000, 2500, 3330, 4000, 5000, 6670, 8000, 10000, + 13330, 16000, 20000, 26670, 32000, 40000, 53330, 80000, +}; + +static ssize_t show_ap2_temp(struct device *dev, + struct device_attribute *attr, char *buf) +{ + SETUP_SHOW_data_param(dev, attr); + long auto_point1; + u8 regval; + int temp; + + mutex_lock(&data->update_lock); + auto_point1 = ((s8) data->reg[param->msb[1]]) * 1000; + regval = + ((data->reg[param->msb[0]]>> param->shift[0])& param->mask[0]); + temp = auto_point1 + asc7621_range_map[SENSORS_LIMIT(regval, 0, 15)]; + mutex_unlock(&data->update_lock); + + return sprintf(buf, "%d\n", temp); + +} + +static ssize_t store_ap2_temp(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + SETUP_STORE_data_param(dev, attr); + long reqval; + long auto_point1; + int i = 0; + u8 currval = 0; + u8 newval = 255;Save two lines and no need to initialize i and currval : + long reqval, auto_point1; + int i; + u8 currval, newval = 255;+ + if (strict_strtol(buf, 10,&reqval)) + return -EINVAL; + + mutex_lock(&data->update_lock); + auto_point1 = data->reg[param->msb[1]] * 1000; + for (i = ARRAY_SIZE(asc7621_range_map) - 1; i>= 0; i--) { + if (reqval>= auto_point1 + asc7621_range_map[i]) { + newval = i; + break; + } + } + if (newval == 255) { + mutex_unlock(&data->update_lock); + return -EINVAL; + } + + newval = (newval& param->mask[0])<< param->shift[0]; + currval = read_byte(param->msb[0]); + newval |= (currval& ~(param->mask[0]<< param->shift[0])); + data->reg[param->msb[0]] = newval; + write_byte(param->msb[0], newval); + mutex_unlock(&data->update_lock); + return count; +} + +static ssize_t show_pwm_ac(struct device *dev, + struct device_attribute *attr, char *buf) +{ + SETUP_SHOW_data_param(dev, attr); + u8 config; + u8 altbit; + u8 regval;You can also save 2 lines here : + u8 config, altbit, regval;+ u8 map[] = { + 0x01, 0x02, 0x04, 0x1f, 0x00, 0x06, 0x07, 0x10, + 0x08, 0x0f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f, 0x1f + }; + + mutex_lock(&data->update_lock); + config = (data->reg[param->msb[0]]>> param->shift[0])& param->mask[0]; + altbit = (data->reg[param->msb[1]]>> param->shift[1])& param->mask[1]; + regval = config | (altbit<< 3); + mutex_unlock(&data->update_lock); + + return sprintf(buf, "%u\n", map[SENSORS_LIMIT(regval, 0, 15)]); +} + +static ssize_t store_pwm_ac(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + SETUP_STORE_data_param(dev, attr); + unsigned long reqval; + + u8 currval = 0; + u8 config = 0; + u8 altbit = 0; + u8 newval = 0;You do not need to initialize there variables : + u8 currval, config, altbit, newval;+ u16 map[] = { + 0x04, 0x00, 0x01, 0xff, 0x02, 0xff, 0x05, 0x06, + 0x08, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x0f, + 0x07, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, + 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x03, + }; + + if (strict_strtoul(buf, 10,&reqval)) + return -EINVAL; + + if (reqval> 31) + return -EINVAL; + + reqval = map[reqval]; + + config = reqval& 0x07; + altbit = (reqval>> 3)& 0x01; + + config = (config& param->mask[0])<< param->shift[0]; + altbit = (altbit& param->mask[1])<< param->shift[1]; + + mutex_lock(&data->update_lock); + currval = read_byte(param->msb[0]); + newval = config | (currval& ~(param->mask[0]<< param->shift[0])); + newval = altbit | (newval& ~(param->mask[1]<< param->shift[1])); + data->reg[param->msb[0]] = newval; + write_byte(param->msb[0], newval); + mutex_unlock(&data->update_lock); + return count; +} + +static ssize_t show_pwm_enable(struct device *dev, + struct device_attribute *attr, char *buf) +{ + SETUP_SHOW_data_param(dev, attr); + u8 config; + u8 altbit; + u8 minoff; + u8 val; + u8 newval;Save 4 lines by : + u8 config, altbit, minoff, val, newval;+ + mutex_lock(&data->update_lock); + config = (data->reg[param->msb[0]]>> param->shift[0])& param->mask[0]; + altbit = (data->reg[param->msb[1]]>> param->shift[1])& param->mask[1]; + minoff = (data->reg[param->msb[2]]>> param->shift[2])& param->mask[2]; + mutex_unlock(&data->update_lock); + + val = config | (altbit<< 3); + newval = 0; + + if (val == 3 || val>= 10) + newval = 255; + else if (val == 4) + newval = 0; + else if (val == 7) + newval = 1; + else if (minoff == 1) + newval = 2; + else + newval = 3; + + return sprintf(buf, "%u\n", newval); +} + +static ssize_t store_pwm_enable(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + SETUP_STORE_data_param(dev, attr); + long reqval; + u8 currval = 0; + u8 config = 0; + u8 altbit = 0; + u8 newval = 0; + u8 minoff = 255;Only minoff is need to initialize and save 4 lines by : + u8 currval, config, altbit, newval, minoff = 255;+ + if (strict_strtol(buf, 10,&reqval)) + return -EINVAL; + + switch (reqval) { + case (0):Please use this : + case 0:+ newval = 0x04; + break; + case (1):+ case 1:+ newval = 0x07; + break; + case (2):+ case 2:+ newval = 0x00; + minoff = 1; + break; + case (3):+ case 3:+ newval = 0x00; + minoff = 0; + break; + case (255):+ case 255:+ newval = 0x03; + break; + default: + return -EINVAL; + } + + config = newval& 0x07; + altbit = (newval>> 3)& 0x01; + + mutex_lock(&data->update_lock); + config = (config& param->mask[0])<< param->shift[0]; + altbit = (altbit& param->mask[1])<< param->shift[1]; + currval = read_byte(param->msb[0]); + newval = config | (currval& ~(param->mask[0]<< param->shift[0])); + newval = altbit | (newval& ~(param->mask[1]<< param->shift[1])); + data->reg[param->msb[0]] = newval; + write_byte(param->msb[0], newval); + if (minoff< 255) { + minoff = (minoff& param->mask[2])<< param->shift[2]; + currval = read_byte(param->msb[2]); + newval = + minoff | (currval& ~(param->mask[2]<< param->shift[2])); + data->reg[param->msb[2]] = newval; + write_byte(param->msb[2], newval); + } + mutex_unlock(&data->update_lock); + return count; +} + +static u32 asc7621_pwm_freq_map[] = { + 10, 15, 23, 30, 38, 47, 62, 94, + 23000, 24000, 25000, 26000, 27000, 28000, 29000, 30000 +}; + +static ssize_t show_pwm_freq(struct device *dev, + struct device_attribute *attr, char *buf) +{ + SETUP_SHOW_data_param(dev, attr); +<---Remove above line for consistency.+ u8 regval = + (data->reg[param->msb[0]]>> param->shift[0])& param->mask[0];And give one extra line here : ++ regval = SENSORS_LIMIT(regval, 0, 15); + + return sprintf(buf, "%u\n", asc7621_pwm_freq_map[regval]); +} + +static ssize_t store_pwm_freq(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + SETUP_STORE_data_param(dev, attr); +Remove above line.+ unsigned long reqval; + u8 currval = 0; + u8 newval = 255;+ u8 currval, newval = 255;+ int i; + + if (strict_strtoul(buf, 10,&reqval)) + return -EINVAL; + + for (i = 0; i< ARRAY_SIZE(asc7621_pwm_freq_map); i++) { + if (reqval == asc7621_pwm_freq_map[i]) { + newval = i; + break; + } + } + if (newval == 255) + return -EINVAL; + + newval = (newval& param->mask[0])<< param->shift[0]; + + mutex_lock(&data->update_lock); + currval = read_byte(param->msb[0]); + newval |= (currval& ~(param->mask[0]<< param->shift[0])); + data->reg[param->msb[0]] = newval; + write_byte(param->msb[0], newval); + mutex_unlock(&data->update_lock); + return count; +} + +static u32 asc7621_pwm_auto_spinup_map[] = { + 0, 100, 250, 400, 700, 1000, 2000, 4000 };This gives error by checkpatch : trailing whitespace +static u32 asc7621_pwm_auto_spinup_map[] = { + 0, 100, 250, 400, 700, 1000, 2000, 4000 +};+ +static ssize_t show_pwm_ast(struct device *dev, + struct device_attribute *attr, char *buf) +{ + SETUP_SHOW_data_param(dev, attr); +Remove above line.+ u8 regval = + (data->reg[param->msb[0]]>> param->shift[0])& param->mask[0]; + + regval = SENSORS_LIMIT(regval, 0, 7); + + return sprintf(buf, "%u\n", asc7621_pwm_auto_spinup_map[regval]); + +} + +static ssize_t store_pwm_ast(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + SETUP_STORE_data_param(dev, attr); + long reqval; + u8 currval = 0; + u8 newval = 255;+ u8 currval, newval = 255;+ u32 i; + + if (strict_strtol(buf, 10,&reqval)) + return -EINVAL; + + for (i = 0; i< ARRAY_SIZE(asc7621_pwm_auto_spinup_map); i++) { + if (reqval == asc7621_pwm_auto_spinup_map[i]) { + newval = i; + break; + } + } + if (newval == 255) + return -EINVAL; + + newval = (newval& param->mask[0])<< param->shift[0]; + + mutex_lock(&data->update_lock); + currval = read_byte(param->msb[0]); + newval |= (currval& ~(param->mask[0]<< param->shift[0])); + data->reg[param->msb[0]] = newval; + write_byte(param->msb[0], newval); + mutex_unlock(&data->update_lock); + return count; +} + +static u32 asc7621_temp_smoothing_time_map[] = { + 35000, 17600, 11800, 7000, 4400, 3000, 1600, 800 };This gives error by checkpatch : trailing whitespace +static u32 asc7621_temp_smoothing_time_map[] = { + 35000, 17600, 11800, 7000, 4400, 3000, 1600, 800 +};+ +static ssize_t show_temp_st(struct device *dev, + struct device_attribute *attr, char *buf) +{ + SETUP_SHOW_data_param(dev, attr); +Remove above line.+ u8 regval = + (data->reg[param->msb[0]]>> param->shift[0])& param->mask[0];Give extra line here after variables declaration.+ regval = SENSORS_LIMIT(regval, 0, 7); + + return sprintf(buf, "%u\n", asc7621_temp_smoothing_time_map[regval]); +} + +static ssize_t store_temp_st(struct device *dev, + struct device_attribute *attr, + const char *buf, size_t count) +{ + SETUP_STORE_data_param(dev, attr); + long reqval; + u8 currval; + u8 newval = 255;Save one line : + u8 currval, newval = 255;+ u32 i; + + if (strict_strtol(buf, 10,&reqval)) + return -EINVAL; + + for (i = 0; i< ARRAY_SIZE(asc7621_pwm_auto_spinup_map); i++) { + if (reqval == asc7621_temp_smoothing_time_map[i]) { + newval = i; + break; + } + } + + if (newval == 255) + return -EINVAL; + + newval = (newval& param->mask[0])<< param->shift[0]; + + mutex_lock(&data->update_lock); + currval = read_byte(param->msb[0]); + newval |= (currval& ~(param->mask[0]<< param->shift[0])); + data->reg[param->msb[0]] = newval; + write_byte(param->msb[0], newval); + mutex_unlock(&data->update_lock); + return count; +} + +/* End of data handlers */ + +/* These defines do nothing more than make the table easierReplace above lines by : +/* + * End of data handlers + * + * These defines do nothing more than make the table easier+ * to read when wrapped at column 80. + */ + +/* + * Creates a variable length array inititalizer. + * VAA(1,3,5,7) would produce {1,3,5,7} + */ +#define VAA(args...) {args} + +#define PREAD(name, n, pri, rm, rl, m, s, r) \ + {.sda = SENSOR_ATTR(name, S_IRUGO, show_##r, NULL, n), \ + .priority = pri, .msb[0] = rm, .lsb[0] = rl, .mask[0] = m, \ + .shift[0] = s,} + +#define PWRITE(name, n, pri, rm, rl, m, s, r) \ + {.sda = SENSOR_ATTR(name, S_IRUGO | S_IWUSR, show_##r, store_##r, n), \ + .priority = pri, .msb[0] = rm, .lsb[0] = rl, .mask[0] = m, \ + .shift[0] = s,} + +/* + * PWRITEM assumes that the initializers for the .msb, .lsb, .mask and .shift + * were created using the VAA macro. + */ +#define PWRITEM(name, n, pri, rm, rl, m, s, r) \ + {.sda = SENSOR_ATTR(name, S_IRUGO | S_IWUSR, show_##r, store_##r, n), \ + .priority = pri, .msb = rm, .lsb = rl, .mask = m, .shift = s,} + +static struct asc7621_param asc7621_params[] = { + PREAD(in0_input, 0, PRI_HIGH, 0x20, 0x13, 0, 0, in10), + PREAD(in1_input, 1, PRI_HIGH, 0x21, 0x18, 0, 0, in10), + PREAD(in2_input, 2, PRI_HIGH, 0x22, 0x11, 0, 0, in10), + PREAD(in3_input, 3, PRI_HIGH, 0x23, 0x12, 0, 0, in10), + PREAD(in4_input, 4, PRI_HIGH, 0x24, 0x14, 0, 0, in10), + + PWRITE(in0_min, 0, PRI_LOW, 0x44, 0, 0, 0, in8), + PWRITE(in1_min, 1, PRI_LOW, 0x46, 0, 0, 0, in8), + PWRITE(in2_min, 2, PRI_LOW, 0x48, 0, 0, 0, in8), + PWRITE(in3_min, 3, PRI_LOW, 0x4a, 0, 0, 0, in8), + PWRITE(in4_min, 4, PRI_LOW, 0x4c, 0, 0, 0, in8), + + PWRITE(in0_max, 0, PRI_LOW, 0x45, 0, 0, 0, in8), + PWRITE(in1_max, 1, PRI_LOW, 0x47, 0, 0, 0, in8), + PWRITE(in2_max, 2, PRI_LOW, 0x49, 0, 0, 0, in8), + PWRITE(in3_max, 3, PRI_LOW, 0x4b, 0, 0, 0, in8), + PWRITE(in4_max, 4, PRI_LOW, 0x4d, 0, 0, 0, in8), + + PREAD(in0_alarm, 0, PRI_LOW, 0x41, 0, 0x01, 0, bitmask), + PREAD(in1_alarm, 1, PRI_LOW, 0x41, 0, 0x01, 1, bitmask), + PREAD(in2_alarm, 2, PRI_LOW, 0x41, 0, 0x01, 2, bitmask), + PREAD(in3_alarm, 3, PRI_LOW, 0x41, 0, 0x01, 3, bitmask), + PREAD(in4_alarm, 4, PRI_LOW, 0x42, 0, 0x01, 0, bitmask), + + PREAD(fan1_input, 0, PRI_HIGH, 0x29, 0x28, 0, 0, fan16), + PREAD(fan2_input, 1, PRI_HIGH, 0x2b, 0x2a, 0, 0, fan16), + PREAD(fan3_input, 2, PRI_HIGH, 0x2d, 0x2c, 0, 0, fan16), + PREAD(fan4_input, 3, PRI_HIGH, 0x2f, 0x2e, 0, 0, fan16), + + PWRITE(fan1_min, 0, PRI_LOW, 0x55, 0x54, 0, 0, fan16), + PWRITE(fan2_min, 1, PRI_LOW, 0x57, 0x56, 0, 0, fan16), + PWRITE(fan3_min, 2, PRI_LOW, 0x59, 0x58, 0, 0, fan16), + PWRITE(fan4_min, 3, PRI_LOW, 0x5b, 0x5a, 0, 0, fan16), + + PREAD(fan1_alarm, 0, PRI_LOW, 0x42, 0, 0x01, 0, bitmask), + PREAD(fan2_alarm, 1, PRI_LOW, 0x42, 0, 0x01, 1, bitmask), + PREAD(fan3_alarm, 2, PRI_LOW, 0x42, 0, 0x01, 2, bitmask), + PREAD(fan4_alarm, 3, PRI_LOW, 0x42, 0, 0x01, 3, bitmask), + + PREAD(temp1_input, 0, PRI_HIGH, 0x25, 0x10, 0, 0, temp10), + PREAD(temp2_input, 1, PRI_HIGH, 0x26, 0x15, 0, 0, temp10), + PREAD(temp3_input, 2, PRI_HIGH, 0x27, 0x16, 0, 0, temp10), + PREAD(temp4_input, 3, PRI_HIGH, 0x33, 0x17, 0, 0, temp10), + PREAD(temp5_input, 4, PRI_HIGH, 0xf7, 0xf6, 0, 0, temp10), + PREAD(temp6_input, 5, PRI_HIGH, 0xf9, 0xf8, 0, 0, temp10), + PREAD(temp7_input, 6, PRI_HIGH, 0xfb, 0xfa, 0, 0, temp10), + PREAD(temp8_input, 7, PRI_HIGH, 0xfd, 0xfc, 0, 0, temp10), + + PWRITE(temp1_min, 0, PRI_LOW, 0x4e, 0, 0, 0, temp8), + PWRITE(temp2_min, 1, PRI_LOW, 0x50, 0, 0, 0, temp8), + PWRITE(temp3_min, 2, PRI_LOW, 0x52, 0, 0, 0, temp8), + PWRITE(temp4_min, 3, PRI_LOW, 0x34, 0, 0, 0, temp8), + + PWRITE(temp1_max, 0, PRI_LOW, 0x4f, 0, 0, 0, temp8), + PWRITE(temp2_max, 1, PRI_LOW, 0x51, 0, 0, 0, temp8), + PWRITE(temp3_max, 2, PRI_LOW, 0x53, 0, 0, 0, temp8), + PWRITE(temp4_max, 3, PRI_LOW, 0x35, 0, 0, 0, temp8), + + PREAD(temp1_alarm, 0, PRI_LOW, 0x41, 0, 0x01, 4, bitmask), + PREAD(temp2_alarm, 1, PRI_LOW, 0x41, 0, 0x01, 5, bitmask), + PREAD(temp3_alarm, 2, PRI_LOW, 0x41, 0, 0x01, 6, bitmask), + PREAD(temp4_alarm, 3, PRI_LOW, 0x43, 0, 0x01, 0, bitmask), + + PWRITE(temp1_source, 0, PRI_LOW, 0x02, 0, 0x07, 4, bitmask), + PWRITE(temp2_source, 1, PRI_LOW, 0x02, 0, 0x07, 0, bitmask), + PWRITE(temp3_source, 2, PRI_LOW, 0x03, 0, 0x07, 4, bitmask), + PWRITE(temp4_source, 3, PRI_LOW, 0x03, 0, 0x07, 0, bitmask), + + PWRITE(temp1_smoothing_enable, 0, PRI_LOW, 0x62, 0, 0x01, 3, bitmask), + PWRITE(temp2_smoothing_enable, 1, PRI_LOW, 0x63, 0, 0x01, 7, bitmask), + PWRITE(temp3_smoothing_enable, 2, PRI_LOW, 0x64, 0, 0x01, 3, bitmask), + PWRITE(temp4_smoothing_enable, 3, PRI_LOW, 0x3c, 0, 0x01, 3, bitmask), + + PWRITE(temp1_smoothing_time, 0, PRI_LOW, 0x62, 0, 0x07, 0, temp_st), + PWRITE(temp2_smoothing_time, 1, PRI_LOW, 0x63, 0, 0x07, 4, temp_st), + PWRITE(temp3_smoothing_time, 2, PRI_LOW, 0x63, 0, 0x07, 0, temp_st), + PWRITE(temp4_smoothing_time, 3, PRI_LOW, 0x3c, 0, 0x07, 0, temp_st), + + PWRITE(temp1_auto_point1_temp_hyst, 0, PRI_LOW, 0x6d, 0, 0x0f, 4, + bitmask), + PWRITE(temp2_auto_point1_temp_hyst, 1, PRI_LOW, 0x6d, 0, 0x0f, 0, + bitmask), + PWRITE(temp3_auto_point1_temp_hyst, 2, PRI_LOW, 0x6e, 0, 0x0f, 4, + bitmask), + PWRITE(temp4_auto_point1_temp_hyst, 3, PRI_LOW, 0x6e, 0, 0x0f, 0, + bitmask), + + PREAD(temp1_auto_point2_temp_hyst, 0, PRI_LOW, 0x6d, 0, 0x0f, 4, + bitmask), + PREAD(temp2_auto_point2_temp_hyst, 1, PRI_LOW, 0x6d, 0, 0x0f, 0, + bitmask), + PREAD(temp3_auto_point2_temp_hyst, 2, PRI_LOW, 0x6e, 0, 0x0f, 4, + bitmask), + PREAD(temp4_auto_point2_temp_hyst, 3, PRI_LOW, 0x6e, 0, 0x0f, 0, + bitmask), + + PWRITE(temp1_auto_point1_temp, 0, PRI_LOW, 0x67, 0, 0, 0, temp8), + PWRITE(temp2_auto_point1_temp, 1, PRI_LOW, 0x68, 0, 0, 0, temp8), + PWRITE(temp3_auto_point1_temp, 2, PRI_LOW, 0x69, 0, 0, 0, temp8), + PWRITE(temp4_auto_point1_temp, 3, PRI_LOW, 0x3b, 0, 0, 0, temp8), + + PWRITEM(temp1_auto_point2_temp, 0, PRI_LOW, VAA(0x5f, 0x67), VAA(0), + VAA(0x0f), VAA(4), ap2_temp), + PWRITEM(temp2_auto_point2_temp, 1, PRI_LOW, VAA(0x60, 0x68), VAA(0), + VAA(0x0f), VAA(4), ap2_temp), + PWRITEM(temp3_auto_point2_temp, 2, PRI_LOW, VAA(0x61, 0x69), VAA(0), + VAA(0x0f), VAA(4), ap2_temp), + PWRITEM(temp4_auto_point2_temp, 3, PRI_LOW, VAA(0x3c, 0x3b), VAA(0), + VAA(0x0f), VAA(4), ap2_temp), + + PWRITE(temp1_crit, 0, PRI_LOW, 0x6a, 0, 0, 0, temp8), + PWRITE(temp2_crit, 1, PRI_LOW, 0x6b, 0, 0, 0, temp8), + PWRITE(temp3_crit, 2, PRI_LOW, 0x6c, 0, 0, 0, temp8), + PWRITE(temp4_crit, 3, PRI_LOW, 0x3d, 0, 0, 0, temp8), + + PWRITE(temp5_enable, 4, PRI_LOW, 0x0e, 0, 0x01, 0, bitmask), + PWRITE(temp6_enable, 5, PRI_LOW, 0x0e, 0, 0x01, 1, bitmask), + PWRITE(temp7_enable, 6, PRI_LOW, 0x0e, 0, 0x01, 2, bitmask), + PWRITE(temp8_enable, 7, PRI_LOW, 0x0e, 0, 0x01, 3, bitmask), + + PWRITE(remote1_offset, 0, PRI_LOW, 0x1c, 0, 0, 0, temp62), + PWRITE(remote2_offset, 1, PRI_LOW, 0x1d, 0, 0, 0, temp62), + + PWRITE(pwm1, 0, PRI_HIGH, 0x30, 0, 0, 0, u8), + PWRITE(pwm2, 1, PRI_HIGH, 0x31, 0, 0, 0, u8), + PWRITE(pwm3, 2, PRI_HIGH, 0x32, 0, 0, 0, u8), + + PWRITE(pwm1_invert, 0, PRI_LOW, 0x5c, 0, 0x01, 4, bitmask), + PWRITE(pwm2_invert, 1, PRI_LOW, 0x5d, 0, 0x01, 4, bitmask), + PWRITE(pwm3_invert, 2, PRI_LOW, 0x5e, 0, 0x01, 4, bitmask), + + PWRITEM(pwm1_enable, 0, PRI_LOW, VAA(0x5c, 0x5c, 0x62), VAA(0, 0, 0), + VAA(0x07, 0x01, 0x01), VAA(5, 3, 5), pwm_enable), + PWRITEM(pwm2_enable, 1, PRI_LOW, VAA(0x5d, 0x5d, 0x62), VAA(0, 0, 0), + VAA(0x07, 0x01, 0x01), VAA(5, 3, 6), pwm_enable), + PWRITEM(pwm3_enable, 2, PRI_LOW, VAA(0x5e, 0x5e, 0x62), VAA(0, 0, 0), + VAA(0x07, 0x01, 0x01), VAA(5, 3, 7), pwm_enable), + + PWRITEM(pwm1_auto_channels, 0, PRI_LOW, VAA(0x5c, 0x5c), VAA(0, 0), + VAA(0x07, 0x01), VAA(5, 3), pwm_ac), + PWRITEM(pwm2_auto_channels, 1, PRI_LOW, VAA(0x5d, 0x5d), VAA(0, 0), + VAA(0x07, 0x01), VAA(5, 3), pwm_ac), + PWRITEM(pwm3_auto_channels, 2, PRI_LOW, VAA(0x5e, 0x5e), VAA(0, 0), + VAA(0x07, 0x01), VAA(5, 3), pwm_ac), + + PWRITE(pwm1_auto_point1_pwm, 0, PRI_LOW, 0x64, 0, 0, 0, u8), + PWRITE(pwm2_auto_point1_pwm, 1, PRI_LOW, 0x65, 0, 0, 0, u8), + PWRITE(pwm3_auto_point1_pwm, 2, PRI_LOW, 0x66, 0, 0, 0, u8), + + PWRITE(pwm1_auto_point2_pwm, 0, PRI_LOW, 0x38, 0, 0, 0, u8), + PWRITE(pwm2_auto_point2_pwm, 1, PRI_LOW, 0x39, 0, 0, 0, u8), + PWRITE(pwm3_auto_point2_pwm, 2, PRI_LOW, 0x3a, 0, 0, 0, u8), + + PWRITE(pwm1_freq, 0, PRI_LOW, 0x5f, 0, 0x0f, 0, pwm_freq), + PWRITE(pwm2_freq, 1, PRI_LOW, 0x60, 0, 0x0f, 0, pwm_freq), + PWRITE(pwm3_freq, 2, PRI_LOW, 0x61, 0, 0x0f, 0, pwm_freq), + + PREAD(pwm1_auto_zone_assigned, 0, PRI_LOW, 0, 0, 0x03, 2, bitmask), + PREAD(pwm2_auto_zone_assigned, 1, PRI_LOW, 0, 0, 0x03, 4, bitmask), + PREAD(pwm3_auto_zone_assigned, 2, PRI_LOW, 0, 0, 0x03, 6, bitmask), + + PWRITE(pwm1_auto_spinup_time, 0, PRI_LOW, 0x5c, 0, 0x07, 0, pwm_ast), + PWRITE(pwm2_auto_spinup_time, 1, PRI_LOW, 0x5d, 0, 0x07, 0, pwm_ast), + PWRITE(pwm3_auto_spinup_time, 2, PRI_LOW, 0x5e, 0, 0x07, 0, pwm_ast), + + PWRITE(peci_enable, 0, PRI_LOW, 0x40, 0, 0x01, 4, bitmask), + PWRITE(peci_avg, 0, PRI_LOW, 0x36, 0, 0x07, 0, bitmask), + PWRITE(peci_domain, 0, PRI_LOW, 0x36, 0, 0x01, 3, bitmask), + PWRITE(peci_legacy, 0, PRI_LOW, 0x36, 0, 0x01, 4, bitmask), + PWRITE(peci_diode, 0, PRI_LOW, 0x0e, 0, 0x07, 4, bitmask), + PWRITE(peci_4domain, 0, PRI_LOW, 0x0e, 0, 0x01, 4, bitmask), + +}; + +static struct asc7621_data *asc7621_update_device(struct device *dev) +{ + struct i2c_client *client = to_i2c_client(dev); + struct asc7621_data *data = i2c_get_clientdata(client); + int i; + +/* + * The asc7621 chips guarantee consistent reads of multi-byte values + * regardless of the order of the reads. No special logic is needed + * so we can just read the registers in whatever order they appear + * in the asc7621_params array. + */ + + mutex_lock(&data->update_lock); + + /* Read all the high priority registers */ + + if (!data->valid || + time_after(jiffies, data->last_high_reading + INTERVAL_HIGH)) { + + for (i = 0; i< ARRAY_SIZE(asc7621_register_priorities); i++) { + if (asc7621_register_priorities[i] == PRI_HIGH) { + data->reg[i] = + i2c_smbus_read_byte_data(client, i)& 0xff; + } + } + data->last_high_reading = jiffies; + }; /* last_reading */ + + /* Read all the low priority registers. */ + + if (!data->valid || + time_after(jiffies, data->last_high_reading + INTERVAL_LOW)) { + + for (i = 0; i< ARRAY_SIZE(asc7621_params); i++) { + if (asc7621_register_priorities[i] == PRI_LOW) { + data->reg[i] = + i2c_smbus_read_byte_data(client, i)& 0xff; + } + } + data->last_low_reading = jiffies; + }; /* last_reading */ + + data->valid = 1; + + mutex_unlock(&data->update_lock); + + return data; +} + +/* Standard detection and initialization below */ + +/* Helper function that checks if an address is valid + * for a particular chip.Replace above lines with : +/* + * Standard detection and initialization below + * + * Helper function that checks if an address is valid for a particular chip.+ */ + +static inline int valid_address_for_chip(int chip_type, int address) +{ + int i = 0;+ int i; ++ for (i = 0; asc7621_chips[chip_type].addresses[i] != I2C_CLIENT_END; + i++) { + if (asc7621_chips[chip_type].addresses[i] == address) + return 1; + } + return 0; +} + +static void asc7621_init_client(struct i2c_client *client) +{ + int value, i, j; + + /* Warn if part was not "READY" */ + + value = i2c_smbus_read_byte_data(client, 0x40)& 0xff; +Replace above 3 lines with : + value = read_byte(0x40);+ if (value& 0x02) { + dev_err(&client->dev, + "Client (%d,0x%02x) config is locked.\n", + i2c_adapter_id(client->adapter), client->addr); + }; + if (!(value& 0x04)) { + dev_err(&client->dev, "Client (%d,0x%02x) is not ready.\n", + i2c_adapter_id(client->adapter), client->addr); + }; + + /* Start monitoring */ + + value = i2c_smbus_read_byte_data(client, 0x40)& 0xff;Do we really need to read value here again, we already read value above and it is unchanged.+ /* Try to clear LOCK, Set START, save everything else */ + value = (value& ~0x02) | 0x01; + write_byte(0x40, value& 0xff); + + /* + * Collect all the registers needed into a single array. + * This way, if a register isn't actually used for anything, + * we don't retrieve it. + */ + + for (i = 0; i< ARRAY_SIZE(asc7621_params); i++) { + for (j = 0; j< ARRAY_SIZE(asc7621_params[i].msb); j++) + asc7621_register_priorities[asc7621_params[i].msb[j]] = + asc7621_params[i].priority; + for (j = 0; j< ARRAY_SIZE(asc7621_params[i].lsb); j++) + asc7621_register_priorities[asc7621_params[i].lsb[j]] = + asc7621_params[i].priority; + } +} + +static struct i2c_driver asc7621_driver;Remove this declaration, we do not need it.+ +static int +asc7621_probe(struct i2c_client *client, const struct i2c_device_id *id) +{ + struct asc7621_data *data; + int i; + int err;Save one line : + int i, err;+ + if (!i2c_check_functionality(client->adapter, I2C_FUNC_SMBUS_BYTE_DATA)) + return -EIO; + + data = kzalloc(sizeof(struct asc7621_data), GFP_KERNEL); + if (data == 0)Sparse gives the warning : Using plain integer as NULL pointer + if (data == NULL)+ return -ENOMEM; + + i2c_set_clientdata(client, data); + data->valid = 0; + mutex_init(&data->update_lock); + + /* Initialize the asc7621 chip */ + asc7621_init_client(client); + + /* Create the sysfs entries */ + for (i = 0; i< ARRAY_SIZE(asc7621_params); i++) { + err = + device_create_file(&client->dev, + &(asc7621_params[i].sda.dev_attr)); + if (err) + goto exit_remove; + } + + data->class_dev = hwmon_device_register(&client->dev); + if (IS_ERR(data->class_dev)) { + err = PTR_ERR(data->class_dev); + goto exit_remove; + } + + return 0; + +exit_remove: + for (i = 0; i< ARRAY_SIZE(asc7621_params); i++) { + device_remove_file(&client->dev, + &(asc7621_params[i].sda.dev_attr)); + } + + i2c_set_clientdata(client, NULL); + kfree(data); + return err; +} + +static int asc7621_detect(struct i2c_client *client, + struct i2c_board_info *info) +{ + struct i2c_adapter *adapter = client->adapter; + int company, verstep; + struct device *dev; + int chip_index = 0;Keep struct together and int together like this : + int company, verstep, chip_index;+ + dev =&client->dev; + + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) + return -ENODEV; + + for (chip_index = FIRST_CHIP; chip_index<= LAST_CHIP; chip_index++) { + + if (!valid_address_for_chip(chip_index, client->addr)) + continue; + + company = i2c_smbus_read_byte_data(client, + asc7621_chips + [chip_index].company_reg)& + 0xff; + verstep = + i2c_smbus_read_byte_data(client, + asc7621_chips + [chip_index].verstep_reg)& 0xff;Replace above 8 lines with : + company = read_byte(asc7621_chips[chip_index].company_reg); + verstep = read_byte(asc7621_chips[chip_index].verstep_reg);+ + if (company == asc7621_chips[chip_index].company_id&& + verstep == asc7621_chips[chip_index].verstep_id) { + strlcpy(client->name, asc7621_chips[chip_index].name, + I2C_NAME_SIZE); + strlcpy(info->type, asc7621_chips[chip_index].name, + I2C_NAME_SIZE); + + dev_info(&adapter->dev, " Matched %s\n",Please fix this as this seems funny as output : + dev_info(&adapter->dev, "Matched %s\n", By replacing it, this will looks like : [ 2005.917005] i2c i2c-0: Matched asc7621a [ 2218.577843] i2c i2c-0: Matched asc7621a+ asc7621_chips[chip_index].name); + return 0; + } + } + + return -ENODEV; +} + +static int asc7621_remove(struct i2c_client *client) +{ + struct asc7621_data *data = i2c_get_clientdata(client); + int i; + + hwmon_device_unregister(data->class_dev); + + for (i = 0; i< ARRAY_SIZE(asc7621_params); i++) { + device_remove_file(&client->dev, + &(asc7621_params[i].sda.dev_attr)); + } + + i2c_set_clientdata(client, NULL); + kfree(data); + return 0; +} + +static const struct i2c_device_id asc7621_id[] = { + {"asc7621", asc7621}, + {"asc7621a", asc7621a}, + {}, +}; + +MODULE_DEVICE_TABLE(i2c, asc7621_id); + +static struct i2c_driver asc7621_driver = { + .class = I2C_CLASS_HWMON, + .driver = { + .name = "asc7621", + },Please align this : + .name = "asc7621", + },+ .probe = asc7621_probe, + .remove = asc7621_remove, + .id_table = asc7621_id, + .detect = asc7621_detect, + .address_list = normal_i2c, +}; + +static int __init sm_asc7621_init(void) +{ + return i2c_add_driver(&asc7621_driver); +} + +static void __exit sm_asc7621_exit(void) +{ + i2c_del_driver(&asc7621_driver); +} + +MODULE_LICENSE("GPL"); +MODULE_AUTHOR("George Joseph"); +MODULE_DESCRIPTION("Andigilog aSC7621 and aSC7621a driver"); + +module_init(sm_asc7621_init); +module_exit(sm_asc7621_exit);Thanks, -- Jaswinder Singh.
_______________________________________________ lm-sensors mailing list lm-sensors@xxxxxxxxxxxxxx http://lists.lm-sensors.org/mailman/listinfo/lm-sensors