From c83c41e4e623aa6504d30c858a05eac771048029 Mon Sep 17 00:00:00 2001 From: Krzysztof Helt Date: Thu, 19 Jul 2007 08:00:17 +0200 Subject: [PATCH] hwmon: adm1021 clean ups This patch provides some coding standard cleanups and general code improvements (more debug info, signed values for temperatures, changed names of ADM1023 regs, removed read/write_value functions). Signed-off-by: Krzysztof Helt Acked-by: Jean Delvare Signed-off-by: Mark M. Hoffman --- drivers/hwmon/adm1021.c | 226 ++++++++++++++++++++++------------------ 1 file changed, 122 insertions(+), 104 deletions(-) diff --git a/drivers/hwmon/adm1021.c b/drivers/hwmon/adm1021.c index c466329b2ef4..5418a675bb60 100644 --- a/drivers/hwmon/adm1021.c +++ b/drivers/hwmon/adm1021.c @@ -1,6 +1,6 @@ /* adm1021.c - Part of lm_sensors, Linux kernel modules for hardware - monitoring + monitoring Copyright (c) 1998, 1999 Frodo Looijaard and Philip Edelbrock @@ -32,11 +32,12 @@ /* Addresses to scan */ static unsigned short normal_i2c[] = { 0x18, 0x19, 0x1a, 0x29, 0x2a, 0x2b, - 0x4c, 0x4d, 0x4e, + 0x4c, 0x4d, 0x4e, I2C_CLIENT_END }; /* Insmod parameters */ -I2C_CLIENT_INSMOD_8(adm1021, adm1023, max1617, max1617a, thmc10, lm84, gl523sm, mc1066); +I2C_CLIENT_INSMOD_8(adm1021, adm1023, max1617, max1617a, thmc10, lm84, gl523sm, + mc1066); /* adm1021 constants specified below */ @@ -45,20 +46,21 @@ I2C_CLIENT_INSMOD_8(adm1021, adm1023, max1617, max1617a, thmc10, lm84, gl523sm, #define ADM1021_REG_TEMP 0x00 #define ADM1021_REG_REMOTE_TEMP 0x01 #define ADM1021_REG_STATUS 0x02 -#define ADM1021_REG_MAN_ID 0x0FE /* 0x41 = AMD, 0x49 = TI, 0x4D = Maxim, 0x23 = Genesys , 0x54 = Onsemi*/ -#define ADM1021_REG_DEV_ID 0x0FF /* ADM1021 = 0x0X, ADM1023 = 0x3X */ -#define ADM1021_REG_DIE_CODE 0x0FF /* MAX1617A */ +/* 0x41 = AD, 0x49 = TI, 0x4D = Maxim, 0x23 = Genesys , 0x54 = Onsemi */ +#define ADM1021_REG_MAN_ID 0xFE +/* ADM1021 = 0x0X, ADM1023 = 0x3X */ +#define ADM1021_REG_DEV_ID 0xFF /* These use different addresses for reading/writing */ #define ADM1021_REG_CONFIG_R 0x03 #define ADM1021_REG_CONFIG_W 0x09 #define ADM1021_REG_CONV_RATE_R 0x04 #define ADM1021_REG_CONV_RATE_W 0x0A /* These are for the ADM1023's additional precision on the remote temp sensor */ -#define ADM1021_REG_REM_TEMP_PREC 0x010 -#define ADM1021_REG_REM_OFFSET 0x011 -#define ADM1021_REG_REM_OFFSET_PREC 0x012 -#define ADM1021_REG_REM_TOS_PREC 0x013 -#define ADM1021_REG_REM_THYST_PREC 0x014 +#define ADM1023_REG_REM_TEMP_PREC 0x10 +#define ADM1023_REG_REM_OFFSET 0x11 +#define ADM1023_REG_REM_OFFSET_PREC 0x12 +#define ADM1023_REG_REM_TOS_PREC 0x13 +#define ADM1023_REG_REM_THYST_PREC 0x14 /* limits */ #define ADM1021_REG_TOS_R 0x05 #define ADM1021_REG_TOS_W 0x0B @@ -77,14 +79,13 @@ I2C_CLIENT_INSMOD_8(adm1021, adm1023, max1617, max1617a, thmc10, lm84, gl523sm, these macros are called: arguments may be evaluated more than once. Fixing this is just not worth it. */ /* Conversions note: 1021 uses normal integer signed-byte format*/ -#define TEMP_FROM_REG(val) (val > 127 ? (val-256)*1000 : val*1000) -#define TEMP_TO_REG(val) (SENSORS_LIMIT((val < 0 ? (val/1000)+256 : val/1000),0,255)) +#define TEMP_TO_REG(val) SENSORS_LIMIT((val) / 1000, -128, 127) /* Initial values */ -/* Note: Even though I left the low and high limits named os and hyst, -they don't quite work like a thermostat the way the LM75 does. I.e., -a lower temp than THYST actually triggers an alarm instead of +/* Note: Even though I left the low and high limits named os and hyst, +they don't quite work like a thermostat the way the LM75 does. I.e., +a lower temp than THYST actually triggers an alarm instead of clearing it. Weird, ey? --Phil */ /* Each client has this additional data */ @@ -97,12 +98,12 @@ struct adm1021_data { char valid; /* !=0 if following fields are valid */ unsigned long last_updated; /* In jiffies */ - u8 temp_max; /* Register values */ - u8 temp_hyst; - u8 temp_input; - u8 remote_temp_max; - u8 remote_temp_hyst; - u8 remote_temp_input; + s8 temp_max; /* Register values */ + s8 temp_hyst; + s8 temp_input; + s8 remote_temp_max; + s8 remote_temp_hyst; + s8 remote_temp_input; u8 alarms; /* Special values for ADM1023 only */ u8 remote_temp_prec; @@ -116,9 +117,6 @@ static int adm1021_attach_adapter(struct i2c_adapter *adapter); static int adm1021_detect(struct i2c_adapter *adapter, int address, int kind); static void adm1021_init_client(struct i2c_client *client); static int adm1021_detach_client(struct i2c_client *client); -static int adm1021_read_value(struct i2c_client *client, u8 reg); -static int adm1021_write_value(struct i2c_client *client, u8 reg, - u16 value); static struct adm1021_data *adm1021_update_device(struct device *dev); /* (amalysh) read only mode, otherwise any limit's writing confuse BIOS */ @@ -135,11 +133,12 @@ static struct i2c_driver adm1021_driver = { .detach_client = adm1021_detach_client, }; -#define show(value) \ -static ssize_t show_##value(struct device *dev, struct device_attribute *attr, char *buf) \ +#define show(value) \ +static ssize_t show_##value(struct device *dev, \ + struct device_attribute *attr, char *buf) \ { \ struct adm1021_data *data = adm1021_update_device(dev); \ - return sprintf(buf, "%d\n", TEMP_FROM_REG(data->value)); \ + return sprintf(buf, "%d\n", 1000 * data->value); \ } show(temp_max); show(temp_hyst); @@ -148,26 +147,29 @@ show(remote_temp_max); show(remote_temp_hyst); show(remote_temp_input); -#define show2(value) \ -static ssize_t show_##value(struct device *dev, struct device_attribute *attr, char *buf) \ -{ \ - struct adm1021_data *data = adm1021_update_device(dev); \ - return sprintf(buf, "%d\n", data->value); \ +static ssize_t show_alarms(struct device *dev, + struct device_attribute *attr, + char *buf) +{ + struct adm1021_data *data = adm1021_update_device(dev); + return sprintf(buf, "%u\n", data->alarms); } -show2(alarms); - -#define set(value, reg) \ -static ssize_t set_##value(struct device *dev, struct device_attribute *attr, const char *buf, size_t count) \ -{ \ - struct i2c_client *client = to_i2c_client(dev); \ - struct adm1021_data *data = i2c_get_clientdata(client); \ - int temp = simple_strtoul(buf, NULL, 10); \ - \ - mutex_lock(&data->update_lock); \ - data->value = TEMP_TO_REG(temp); \ - adm1021_write_value(client, reg, data->value); \ - mutex_unlock(&data->update_lock); \ - return count; \ + +#define set(value, reg) \ +static ssize_t set_##value(struct device *dev, \ + struct device_attribute *attr, \ + const char *buf, size_t count) \ +{ \ + struct i2c_client *client = to_i2c_client(dev); \ + struct adm1021_data *data = i2c_get_clientdata(client); \ + int temp = simple_strtoul(buf, NULL, 10); \ + \ + mutex_lock(&data->update_lock); \ + data->value = TEMP_TO_REG(temp); \ + if (!read_only) \ + i2c_smbus_write_byte_data(client, reg, data->value); \ + mutex_unlock(&data->update_lock); \ + return count; \ } set(temp_max, ADM1021_REG_TOS_W); set(temp_hyst, ADM1021_REG_THYST_W); @@ -208,35 +210,44 @@ static const struct attribute_group adm1021_group = { static int adm1021_detect(struct i2c_adapter *adapter, int address, int kind) { int i; - struct i2c_client *new_client; + struct i2c_client *client; struct adm1021_data *data; int err = 0; const char *type_name = ""; + int conv_rate, status, config; - if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) + if (!i2c_check_functionality(adapter, I2C_FUNC_SMBUS_BYTE_DATA)) { + pr_debug("adm1021: detect failed, " + "smbus byte data not supported!\n"); goto error0; + } /* OK. For now, we presume we have a valid client. We now create the client structure, even though we cannot fill it completely yet. - But it allows us to access adm1021_{read,write}_value. */ + But it allows us to access adm1021 register values. */ if (!(data = kzalloc(sizeof(struct adm1021_data), GFP_KERNEL))) { + pr_debug("adm1021: detect failed, kzalloc failed!\n"); err = -ENOMEM; goto error0; } - new_client = &data->client; - i2c_set_clientdata(new_client, data); - new_client->addr = address; - new_client->adapter = adapter; - new_client->driver = &adm1021_driver; - new_client->flags = 0; + client = &data->client; + i2c_set_clientdata(client, data); + client->addr = address; + client->adapter = adapter; + client->driver = &adm1021_driver; + status = i2c_smbus_read_byte_data(client, ADM1021_REG_STATUS); + conv_rate = i2c_smbus_read_byte_data(client, + ADM1021_REG_CONV_RATE_R); + config = i2c_smbus_read_byte_data(client, ADM1021_REG_CONFIG_R); /* Now, we do the remaining detection. */ if (kind < 0) { - if ((adm1021_read_value(new_client, ADM1021_REG_STATUS) & 0x03) != 0x00 - || (adm1021_read_value(new_client, ADM1021_REG_CONFIG_R) & 0x3F) != 0x00 - || (adm1021_read_value(new_client, ADM1021_REG_CONV_RATE_R) & 0xF8) != 0x00) { + if ((status & 0x03) != 0x00 || (config & 0x3F) != 0x00 + || (conv_rate & 0xF8) != 0x00) { + pr_debug("adm1021: detect failed, " + "chip not detected!\n"); err = -ENODEV; goto error1; } @@ -244,9 +255,10 @@ static int adm1021_detect(struct i2c_adapter *adapter, int address, int kind) /* Determine the chip type. */ if (kind <= 0) { - i = adm1021_read_value(new_client, ADM1021_REG_MAN_ID); + i = i2c_smbus_read_byte_data(client, ADM1021_REG_MAN_ID); if (i == 0x41) - if ((adm1021_read_value(new_client, ADM1021_REG_DEV_ID) & 0x0F0) == 0x030) + if ((i2c_smbus_read_byte_data(client, + ADM1021_REG_DEV_ID) & 0xF0) == 0x30) kind = adm1023; else kind = adm1021; @@ -255,15 +267,16 @@ static int adm1021_detect(struct i2c_adapter *adapter, int address, int kind) else if (i == 0x23) kind = gl523sm; else if ((i == 0x4d) && - (adm1021_read_value(new_client, ADM1021_REG_DEV_ID) == 0x01)) + (i2c_smbus_read_byte_data(client, + ADM1021_REG_DEV_ID) == 0x01)) kind = max1617a; else if (i == 0x54) kind = mc1066; /* LM84 Mfr ID in a different place, and it has more unused bits */ - else if (adm1021_read_value(new_client, ADM1021_REG_CONV_RATE_R) == 0x00 - && (kind == 0 /* skip extra detection */ - || ((adm1021_read_value(new_client, ADM1021_REG_CONFIG_R) & 0x7F) == 0x00 - && (adm1021_read_value(new_client, ADM1021_REG_STATUS) & 0xAB) == 0x00))) + else if (conv_rate == 0x00 + && (kind == 0 /* skip extra detection */ + || ((config & 0x7F) == 0x00 + && (status & 0xAB) == 0x00))) kind = lm84; else kind = max1617; @@ -286,26 +299,27 @@ static int adm1021_detect(struct i2c_adapter *adapter, int address, int kind) } else if (kind == mc1066) { type_name = "mc1066"; } + pr_debug("adm1021: Detected chip %s at adapter %d, address 0x%02x.\n", + type_name, i2c_adapter_id(adapter), address); - /* Fill in the remaining client fields and put it into the global list */ - strlcpy(new_client->name, type_name, I2C_NAME_SIZE); + /* Fill in the remaining client fields */ + strlcpy(client->name, type_name, I2C_NAME_SIZE); data->type = kind; - data->valid = 0; mutex_init(&data->update_lock); /* Tell the I2C layer a new client has arrived */ - if ((err = i2c_attach_client(new_client))) + if ((err = i2c_attach_client(client))) goto error1; /* Initialize the ADM1021 chip */ - if (kind != lm84) - adm1021_init_client(new_client); + if (kind != lm84 && !read_only) + adm1021_init_client(client); /* Register sysfs hooks */ - if ((err = sysfs_create_group(&new_client->dev.kobj, &adm1021_group))) + if ((err = sysfs_create_group(&client->dev.kobj, &adm1021_group))) goto error2; - data->class_dev = hwmon_device_register(&new_client->dev); + data->class_dev = hwmon_device_register(&client->dev); if (IS_ERR(data->class_dev)) { err = PTR_ERR(data->class_dev); goto error3; @@ -314,9 +328,9 @@ static int adm1021_detect(struct i2c_adapter *adapter, int address, int kind) return 0; error3: - sysfs_remove_group(&new_client->dev.kobj, &adm1021_group); + sysfs_remove_group(&client->dev.kobj, &adm1021_group); error2: - i2c_detach_client(new_client); + i2c_detach_client(client); error1: kfree(data); error0: @@ -326,10 +340,10 @@ error0: static void adm1021_init_client(struct i2c_client *client) { /* Enable ADC and disable suspend mode */ - adm1021_write_value(client, ADM1021_REG_CONFIG_W, - adm1021_read_value(client, ADM1021_REG_CONFIG_R) & 0xBF); + i2c_smbus_write_byte_data(client, ADM1021_REG_CONFIG_W, + i2c_smbus_read_byte_data(client, ADM1021_REG_CONFIG_R) & 0xBF); /* Set Conversion rate to 1/sec (this can be tinkered with) */ - adm1021_write_value(client, ADM1021_REG_CONV_RATE_W, 0x04); + i2c_smbus_write_byte_data(client, ADM1021_REG_CONV_RATE_W, 0x04); } static int adm1021_detach_client(struct i2c_client *client) @@ -347,19 +361,6 @@ static int adm1021_detach_client(struct i2c_client *client) return 0; } -/* All registers are byte-sized */ -static int adm1021_read_value(struct i2c_client *client, u8 reg) -{ - return i2c_smbus_read_byte_data(client, reg); -} - -static int adm1021_write_value(struct i2c_client *client, u8 reg, u16 value) -{ - if (!read_only) - return i2c_smbus_write_byte_data(client, reg, value); - return 0; -} - static struct adm1021_data *adm1021_update_device(struct device *dev) { struct i2c_client *client = to_i2c_client(dev); @@ -371,19 +372,36 @@ static struct adm1021_data *adm1021_update_device(struct device *dev) || !data->valid) { dev_dbg(&client->dev, "Starting adm1021 update\n"); - data->temp_input = adm1021_read_value(client, ADM1021_REG_TEMP); - data->temp_max = adm1021_read_value(client, ADM1021_REG_TOS_R); - data->temp_hyst = adm1021_read_value(client, ADM1021_REG_THYST_R); - data->remote_temp_input = adm1021_read_value(client, ADM1021_REG_REMOTE_TEMP); - data->remote_temp_max = adm1021_read_value(client, ADM1021_REG_REMOTE_TOS_R); - data->remote_temp_hyst = adm1021_read_value(client, ADM1021_REG_REMOTE_THYST_R); - data->alarms = adm1021_read_value(client, ADM1021_REG_STATUS) & 0x7c; + data->temp_input = i2c_smbus_read_byte_data(client, + ADM1021_REG_TEMP); + data->temp_max = i2c_smbus_read_byte_data(client, + ADM1021_REG_TOS_R); + data->temp_hyst = i2c_smbus_read_byte_data(client, + ADM1021_REG_THYST_R); + data->remote_temp_input = i2c_smbus_read_byte_data(client, + ADM1021_REG_REMOTE_TEMP); + data->remote_temp_max = i2c_smbus_read_byte_data(client, + ADM1021_REG_REMOTE_TOS_R); + data->remote_temp_hyst = i2c_smbus_read_byte_data(client, + ADM1021_REG_REMOTE_THYST_R); + data->alarms = i2c_smbus_read_byte_data(client, + ADM1021_REG_STATUS) & 0x7c; if (data->type == adm1023) { - data->remote_temp_prec = adm1021_read_value(client, ADM1021_REG_REM_TEMP_PREC); - data->remote_temp_os_prec = adm1021_read_value(client, ADM1021_REG_REM_TOS_PREC); - data->remote_temp_hyst_prec = adm1021_read_value(client, ADM1021_REG_REM_THYST_PREC); - data->remote_temp_offset = adm1021_read_value(client, ADM1021_REG_REM_OFFSET); - data->remote_temp_offset_prec = adm1021_read_value(client, ADM1021_REG_REM_OFFSET_PREC); + data->remote_temp_prec = + i2c_smbus_read_byte_data(client, + ADM1023_REG_REM_TEMP_PREC); + data->remote_temp_os_prec = + i2c_smbus_read_byte_data(client, + ADM1023_REG_REM_TOS_PREC); + data->remote_temp_hyst_prec = + i2c_smbus_read_byte_data(client, + ADM1023_REG_REM_THYST_PREC); + data->remote_temp_offset = + i2c_smbus_read_byte_data(client, + ADM1023_REG_REM_OFFSET); + data->remote_temp_offset_prec = + i2c_smbus_read_byte_data(client, + ADM1023_REG_REM_OFFSET_PREC); } data->last_updated = jiffies; data->valid = 1; -- 2.20.1