* [PATCH] drivers/hwmon/emc1403.c: add support for emc1412
@ 2014-05-11 13:00 ` Josef Gajdusek
0 siblings, 0 replies; 26+ messages in thread
From: Josef Gajdusek @ 2014-05-11 13:00 UTC (permalink / raw)
To: jdelvare; +Cc: linux, lm-sensors, linux-kernel
Adds support for emc1412.
---
diff --git a/drivers/hwmon/emc1403.c b/drivers/hwmon/emc1403.c
index 90ec117..47a98a7 100644
--- a/drivers/hwmon/emc1403.c
+++ b/drivers/hwmon/emc1403.c
@@ -40,7 +40,7 @@
struct thermal_data {
struct i2c_client *client;
- const struct attribute_group *groups[3];
+ const struct attribute_group *groups[2];
struct mutex mutex;
/*
* Cache the hyst value so we don't keep re-reading it. In theory
@@ -252,6 +252,28 @@ static SENSOR_DEVICE_ATTR(temp4_crit_hyst, S_IRUGO | S_IWUSR,
static SENSOR_DEVICE_ATTR_2(power_state, S_IRUGO | S_IWUSR,
show_bit, store_bit, 0x03, 0x40);
+
+static struct attribute *emc1412_attrs[] = {
+ &sensor_dev_attr_temp1_min.dev_attr.attr,
+ &sensor_dev_attr_temp1_max.dev_attr.attr,
+ &sensor_dev_attr_temp1_crit.dev_attr.attr,
+ &sensor_dev_attr_temp1_input.dev_attr.attr,
+ &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
+
+ &sensor_dev_attr_temp2_min.dev_attr.attr,
+ &sensor_dev_attr_temp2_max.dev_attr.attr,
+ &sensor_dev_attr_temp2_crit.dev_attr.attr,
+ &sensor_dev_attr_temp2_input.dev_attr.attr,
+ &sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
+
+ &sensor_dev_attr_power_state.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group emc1412_group = {
+ .attrs = emc1412_attrs,
+};
+
static struct attribute *emc1403_attrs[] = {
&sensor_dev_attr_temp1_min.dev_attr.attr,
&sensor_dev_attr_temp1_max.dev_attr.attr,
@@ -261,6 +283,7 @@ static struct attribute *emc1403_attrs[] = {
&sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
&sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
&sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
+
&sensor_dev_attr_temp2_min.dev_attr.attr,
&sensor_dev_attr_temp2_max.dev_attr.attr,
&sensor_dev_attr_temp2_crit.dev_attr.attr,
@@ -269,6 +292,7 @@ static struct attribute *emc1403_attrs[] = {
&sensor_dev_attr_temp2_max_alarm.dev_attr.attr,
&sensor_dev_attr_temp2_crit_alarm.dev_attr.attr,
&sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
+
&sensor_dev_attr_temp3_min.dev_attr.attr,
&sensor_dev_attr_temp3_max.dev_attr.attr,
&sensor_dev_attr_temp3_crit.dev_attr.attr,
@@ -277,6 +301,7 @@ static struct attribute *emc1403_attrs[] = {
&sensor_dev_attr_temp3_max_alarm.dev_attr.attr,
&sensor_dev_attr_temp3_crit_alarm.dev_attr.attr,
&sensor_dev_attr_temp3_crit_hyst.dev_attr.attr,
+
&sensor_dev_attr_power_state.dev_attr.attr,
NULL
};
@@ -286,6 +311,33 @@ static const struct attribute_group emc1403_group = {
};
static struct attribute *emc1404_attrs[] = {
+ &sensor_dev_attr_temp1_min.dev_attr.attr,
+ &sensor_dev_attr_temp1_max.dev_attr.attr,
+ &sensor_dev_attr_temp1_crit.dev_attr.attr,
+ &sensor_dev_attr_temp1_input.dev_attr.attr,
+ &sensor_dev_attr_temp1_min_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
+
+ &sensor_dev_attr_temp2_min.dev_attr.attr,
+ &sensor_dev_attr_temp2_max.dev_attr.attr,
+ &sensor_dev_attr_temp2_crit.dev_attr.attr,
+ &sensor_dev_attr_temp2_input.dev_attr.attr,
+ &sensor_dev_attr_temp2_min_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp2_max_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp2_crit_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
+
+ &sensor_dev_attr_temp3_min.dev_attr.attr,
+ &sensor_dev_attr_temp3_max.dev_attr.attr,
+ &sensor_dev_attr_temp3_crit.dev_attr.attr,
+ &sensor_dev_attr_temp3_input.dev_attr.attr,
+ &sensor_dev_attr_temp3_min_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp3_max_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp3_crit_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp3_crit_hyst.dev_attr.attr,
+
&sensor_dev_attr_temp4_min.dev_attr.attr,
&sensor_dev_attr_temp4_max.dev_attr.attr,
&sensor_dev_attr_temp4_crit.dev_attr.attr,
@@ -294,6 +346,8 @@ static struct attribute *emc1404_attrs[] = {
&sensor_dev_attr_temp4_max_alarm.dev_attr.attr,
&sensor_dev_attr_temp4_crit_alarm.dev_attr.attr,
&sensor_dev_attr_temp4_crit_hyst.dev_attr.attr,
+
+ &sensor_dev_attr_power_state.dev_attr.attr,
NULL
};
@@ -301,11 +355,13 @@ static const struct attribute_group emc1404_group = {
.attrs = emc1404_attrs,
};
+
+
static int emc1403_detect(struct i2c_client *client,
struct i2c_board_info *info)
{
int id;
- /* Check if thermal chip is SMSC and EMC1403 or EMC1423 */
+ /* Check whether the chip is SMSC and get its type */
id = i2c_smbus_read_byte_data(client, THERMAL_SMSC_ID_REG);
if (id != 0x5d)
@@ -313,6 +369,9 @@ static int emc1403_detect(struct i2c_client *client,
id = i2c_smbus_read_byte_data(client, THERMAL_PID_REG);
switch (id) {
+ case 0x20:
+ strlcpy(info->type, "emc1412", I2C_NAME_SIZE);
+ break;
case 0x21:
strlcpy(info->type, "emc1403", I2C_NAME_SIZE);
break;
@@ -330,9 +389,9 @@ static int emc1403_detect(struct i2c_client *client,
}
id = i2c_smbus_read_byte_data(client, THERMAL_REVISION_REG);
- if (id != 0x01)
+ if (id != 0x01 && id != 0x04) {
return -ENODEV;
-
+ }
return 0;
}
@@ -351,9 +410,17 @@ static int emc1403_probe(struct i2c_client *client,
mutex_init(&data->mutex);
data->hyst_valid = jiffies - 1; /* Expired */
- data->groups[0] = &emc1403_group;
- if (id->driver_data)
- data->groups[1] = &emc1404_group;
+ switch (id->driver_data) {
+ case 0:
+ data->groups[0] = &emc1412_group;
+ break;
+ case 1:
+ data->groups[0] = &emc1403_group;
+ break;
+ case 2:
+ data->groups[0] = &emc1404_group;
+ break;
+ }
hwmon_dev = hwmon_device_register_with_groups(&client->dev,
client->name, data,
@@ -366,14 +433,19 @@ static int emc1403_probe(struct i2c_client *client,
}
static const unsigned short emc1403_address_list[] = {
- 0x18, 0x29, 0x4c, 0x4d, I2C_CLIENT_END
+ /* emc1403/emc1404/emc1423/emc1424 */
+ 0x4c, 0x4d, 0x18, 0x29,
+ /* emc1412 */
+ 0x5c, 0x4c, 0x6c, 0x1c, 0x3c, I2C_CLIENT_END
};
+/* Last number in name indicates the amount of channels */
static const struct i2c_device_id emc1403_idtable[] = {
- { "emc1403", 0 },
- { "emc1404", 1 },
- { "emc1423", 0 },
- { "emc1424", 1 },
+ { "emc1412", 0 },
+ { "emc1403", 1 },
+ { "emc1423", 1 },
+ { "emc1404", 2 },
+ { "emc1424", 2 },
{ }
};
MODULE_DEVICE_TABLE(i2c, emc1403_idtable);
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [lm-sensors] [PATCH] drivers/hwmon/emc1403.c: add support for emc1412
2014-05-11 13:00 ` Josef Gajdusek
@ 2014-05-11 22:40 ` Guenter Roeck
-1 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2014-05-11 22:40 UTC (permalink / raw)
To: Josef Gajdusek, jdelvare; +Cc: lm-sensors, linux-kernel
On 05/11/2014 06:00 AM, Josef Gajdusek wrote:
> Adds support for emc1412.
>
It is customary to explain the difference to already supported chips
and what changes were made to support the new chip(s).
> ---
> diff --git a/drivers/hwmon/emc1403.c b/drivers/hwmon/emc1403.c
> index 90ec117..47a98a7 100644
> --- a/drivers/hwmon/emc1403.c
> +++ b/drivers/hwmon/emc1403.c
> @@ -40,7 +40,7 @@
>
> struct thermal_data {
> struct i2c_client *client;
> - const struct attribute_group *groups[3];
> + const struct attribute_group *groups[2];
The purpose of having more than one group is to simplify support for optional
features while at the same time avoiding duplication. This is why there used to be
two groups (plus an unused entry for list termination).
Separating and duplicating all attributes into one group per supported chip
type defeats that purpose. Please don't do that, and revert the related changes.
This should not be reduced to two entries, but increased to four or even five;
see below.
> struct mutex mutex;
> /*
> * Cache the hyst value so we don't keep re-reading it. In theory
> @@ -252,6 +252,28 @@ static SENSOR_DEVICE_ATTR(temp4_crit_hyst, S_IRUGO | S_IWUSR,
> static SENSOR_DEVICE_ATTR_2(power_state, S_IRUGO | S_IWUSR,
> show_bit, store_bit, 0x03, 0x40);
>
> +
> +static struct attribute *emc1412_attrs[] = {
> + &sensor_dev_attr_temp1_min.dev_attr.attr,
> + &sensor_dev_attr_temp1_max.dev_attr.attr,
> + &sensor_dev_attr_temp1_crit.dev_attr.attr,
> + &sensor_dev_attr_temp1_input.dev_attr.attr,
> + &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
> +
> + &sensor_dev_attr_temp2_min.dev_attr.attr,
> + &sensor_dev_attr_temp2_max.dev_attr.attr,
> + &sensor_dev_attr_temp2_crit.dev_attr.attr,
> + &sensor_dev_attr_temp2_input.dev_attr.attr,
> + &sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
> +
> + &sensor_dev_attr_power_state.dev_attr.attr,
> + NULL
> +};
> +
This should become the core attribute group, ie the list of attributes supported
by all chips.
The EMC1412 does support alarms, so I would suggest to add another optional
group for EMC1412 alarms (needed because the alarm register and bits are not
compatible to the registers/bits used by other chips supported by this driver).
> +static const struct attribute_group emc1412_group = {
> + .attrs = emc1412_attrs,
> +};
> +
> static struct attribute *emc1403_attrs[] = {
> &sensor_dev_attr_temp1_min.dev_attr.attr,
> &sensor_dev_attr_temp1_max.dev_attr.attr,
> @@ -261,6 +283,7 @@ static struct attribute *emc1403_attrs[] = {
> &sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
> &sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
> &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
> +
> &sensor_dev_attr_temp2_min.dev_attr.attr,
> &sensor_dev_attr_temp2_max.dev_attr.attr,
> &sensor_dev_attr_temp2_crit.dev_attr.attr,
> @@ -269,6 +292,7 @@ static struct attribute *emc1403_attrs[] = {
> &sensor_dev_attr_temp2_max_alarm.dev_attr.attr,
> &sensor_dev_attr_temp2_crit_alarm.dev_attr.attr,
> &sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
> +
> &sensor_dev_attr_temp3_min.dev_attr.attr,
> &sensor_dev_attr_temp3_max.dev_attr.attr,
> &sensor_dev_attr_temp3_crit.dev_attr.attr,
> @@ -277,6 +301,7 @@ static struct attribute *emc1403_attrs[] = {
> &sensor_dev_attr_temp3_max_alarm.dev_attr.attr,
> &sensor_dev_attr_temp3_crit_alarm.dev_attr.attr,
> &sensor_dev_attr_temp3_crit_hyst.dev_attr.attr,
> +
> &sensor_dev_attr_power_state.dev_attr.attr,
> NULL
> };
Modify this group to list the attributes only supported by EMC1403
and compatible chips (on top of the core or emc1412 group).
> @@ -286,6 +311,33 @@ static const struct attribute_group emc1403_group = {
> };
>
> static struct attribute *emc1404_attrs[] = {
> + &sensor_dev_attr_temp1_min.dev_attr.attr,
> + &sensor_dev_attr_temp1_max.dev_attr.attr,
> + &sensor_dev_attr_temp1_crit.dev_attr.attr,
> + &sensor_dev_attr_temp1_input.dev_attr.attr,
> + &sensor_dev_attr_temp1_min_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
> +
> + &sensor_dev_attr_temp2_min.dev_attr.attr,
> + &sensor_dev_attr_temp2_max.dev_attr.attr,
> + &sensor_dev_attr_temp2_crit.dev_attr.attr,
> + &sensor_dev_attr_temp2_input.dev_attr.attr,
> + &sensor_dev_attr_temp2_min_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp2_max_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp2_crit_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
> +
> + &sensor_dev_attr_temp3_min.dev_attr.attr,
> + &sensor_dev_attr_temp3_max.dev_attr.attr,
> + &sensor_dev_attr_temp3_crit.dev_attr.attr,
> + &sensor_dev_attr_temp3_input.dev_attr.attr,
> + &sensor_dev_attr_temp3_min_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp3_max_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp3_crit_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp3_crit_hyst.dev_attr.attr,
> +
This is exactly what you don't want to do; it is unnecessary duplication.
There was a reason for having the second group on top of the first one,
as explained above.
> &sensor_dev_attr_temp4_min.dev_attr.attr,
> &sensor_dev_attr_temp4_max.dev_attr.attr,
> &sensor_dev_attr_temp4_crit.dev_attr.attr,
> @@ -294,6 +346,8 @@ static struct attribute *emc1404_attrs[] = {
> &sensor_dev_attr_temp4_max_alarm.dev_attr.attr,
> &sensor_dev_attr_temp4_crit_alarm.dev_attr.attr,
> &sensor_dev_attr_temp4_crit_hyst.dev_attr.attr,
> +
> + &sensor_dev_attr_power_state.dev_attr.attr,
> NULL
> };
>
> @@ -301,11 +355,13 @@ static const struct attribute_group emc1404_group = {
> .attrs = emc1404_attrs,
> };
>
> +
> +
Unnecessary empty lines and unnecessary whitespace change.
> static int emc1403_detect(struct i2c_client *client,
> struct i2c_board_info *info)
> {
> int id;
> - /* Check if thermal chip is SMSC and EMC1403 or EMC1423 */
> + /* Check whether the chip is SMSC and get its type */
This change makes the comment pretty much worthless. It used to describe
what the function is doing, which is no longer the case after your change.
>
> id = i2c_smbus_read_byte_data(client, THERMAL_SMSC_ID_REG);
> if (id != 0x5d)
> @@ -313,6 +369,9 @@ static int emc1403_detect(struct i2c_client *client,
>
> id = i2c_smbus_read_byte_data(client, THERMAL_PID_REG);
> switch (id) {
> + case 0x20:
> + strlcpy(info->type, "emc1412", I2C_NAME_SIZE);
> + break;
> case 0x21:
> strlcpy(info->type, "emc1403", I2C_NAME_SIZE);
> break;
> @@ -330,9 +389,9 @@ static int emc1403_detect(struct i2c_client *client,
> }
>
> id = i2c_smbus_read_byte_data(client, THERMAL_REVISION_REG);
> - if (id != 0x01)
> + if (id != 0x01 && id != 0x04) {
> return -ENODEV;
This should be a separate patch, as it applies to emc1403/emc1404 as well,
so we can backport it into -stable.
> -
> + }
> return 0;
> }
>
> @@ -351,9 +410,17 @@ static int emc1403_probe(struct i2c_client *client,
> mutex_init(&data->mutex);
> data->hyst_valid = jiffies - 1; /* Expired */
>
> - data->groups[0] = &emc1403_group;
> - if (id->driver_data)
> - data->groups[1] = &emc1404_group;
> + switch (id->driver_data) {
> + case 0:
> + data->groups[0] = &emc1412_group;
> + break;
> + case 1:
> + data->groups[0] = &emc1403_group;
> + break;
> + case 2:
> + data->groups[0] = &emc1404_group;
> + break;
> + }
>
Not acceptable. Again, there is a reason for having multiple attribute groups.
> hwmon_dev = hwmon_device_register_with_groups(&client->dev,
> client->name, data,
> @@ -366,14 +433,19 @@ static int emc1403_probe(struct i2c_client *client,
> }
>
> static const unsigned short emc1403_address_list[] = {
> - 0x18, 0x29, 0x4c, 0x4d, I2C_CLIENT_END
> + /* emc1403/emc1404/emc1423/emc1424 */
> + 0x4c, 0x4d, 0x18, 0x29,
> + /* emc1412 */
> + 0x5c, 0x4c, 0x6c, 0x1c, 0x3c, I2C_CLIENT_END
No duplication of addresses, and addresses are by convention in order.
Jean, any addresses which should not be scanned ?
> };
>
> +/* Last number in name indicates the amount of channels */
> static const struct i2c_device_id emc1403_idtable[] = {
> - { "emc1403", 0 },
> - { "emc1404", 1 },
> - { "emc1423", 0 },
> - { "emc1424", 1 },
> + { "emc1412", 0 },
> + { "emc1403", 1 },
> + { "emc1423", 1 },
> + { "emc1404", 2 },
> + { "emc1424", 2 },
It may be time to introduce an enum for supported chip variants.
Also, the list is by custom in alphabetical order.
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH] drivers/hwmon/emc1403.c: add support for emc1412
@ 2014-05-11 22:40 ` Guenter Roeck
0 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2014-05-11 22:40 UTC (permalink / raw)
To: Josef Gajdusek, jdelvare; +Cc: lm-sensors, linux-kernel
On 05/11/2014 06:00 AM, Josef Gajdusek wrote:
> Adds support for emc1412.
>
It is customary to explain the difference to already supported chips
and what changes were made to support the new chip(s).
> ---
> diff --git a/drivers/hwmon/emc1403.c b/drivers/hwmon/emc1403.c
> index 90ec117..47a98a7 100644
> --- a/drivers/hwmon/emc1403.c
> +++ b/drivers/hwmon/emc1403.c
> @@ -40,7 +40,7 @@
>
> struct thermal_data {
> struct i2c_client *client;
> - const struct attribute_group *groups[3];
> + const struct attribute_group *groups[2];
The purpose of having more than one group is to simplify support for optional
features while at the same time avoiding duplication. This is why there used to be
two groups (plus an unused entry for list termination).
Separating and duplicating all attributes into one group per supported chip
type defeats that purpose. Please don't do that, and revert the related changes.
This should not be reduced to two entries, but increased to four or even five;
see below.
> struct mutex mutex;
> /*
> * Cache the hyst value so we don't keep re-reading it. In theory
> @@ -252,6 +252,28 @@ static SENSOR_DEVICE_ATTR(temp4_crit_hyst, S_IRUGO | S_IWUSR,
> static SENSOR_DEVICE_ATTR_2(power_state, S_IRUGO | S_IWUSR,
> show_bit, store_bit, 0x03, 0x40);
>
> +
> +static struct attribute *emc1412_attrs[] = {
> + &sensor_dev_attr_temp1_min.dev_attr.attr,
> + &sensor_dev_attr_temp1_max.dev_attr.attr,
> + &sensor_dev_attr_temp1_crit.dev_attr.attr,
> + &sensor_dev_attr_temp1_input.dev_attr.attr,
> + &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
> +
> + &sensor_dev_attr_temp2_min.dev_attr.attr,
> + &sensor_dev_attr_temp2_max.dev_attr.attr,
> + &sensor_dev_attr_temp2_crit.dev_attr.attr,
> + &sensor_dev_attr_temp2_input.dev_attr.attr,
> + &sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
> +
> + &sensor_dev_attr_power_state.dev_attr.attr,
> + NULL
> +};
> +
This should become the core attribute group, ie the list of attributes supported
by all chips.
The EMC1412 does support alarms, so I would suggest to add another optional
group for EMC1412 alarms (needed because the alarm register and bits are not
compatible to the registers/bits used by other chips supported by this driver).
> +static const struct attribute_group emc1412_group = {
> + .attrs = emc1412_attrs,
> +};
> +
> static struct attribute *emc1403_attrs[] = {
> &sensor_dev_attr_temp1_min.dev_attr.attr,
> &sensor_dev_attr_temp1_max.dev_attr.attr,
> @@ -261,6 +283,7 @@ static struct attribute *emc1403_attrs[] = {
> &sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
> &sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
> &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
> +
> &sensor_dev_attr_temp2_min.dev_attr.attr,
> &sensor_dev_attr_temp2_max.dev_attr.attr,
> &sensor_dev_attr_temp2_crit.dev_attr.attr,
> @@ -269,6 +292,7 @@ static struct attribute *emc1403_attrs[] = {
> &sensor_dev_attr_temp2_max_alarm.dev_attr.attr,
> &sensor_dev_attr_temp2_crit_alarm.dev_attr.attr,
> &sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
> +
> &sensor_dev_attr_temp3_min.dev_attr.attr,
> &sensor_dev_attr_temp3_max.dev_attr.attr,
> &sensor_dev_attr_temp3_crit.dev_attr.attr,
> @@ -277,6 +301,7 @@ static struct attribute *emc1403_attrs[] = {
> &sensor_dev_attr_temp3_max_alarm.dev_attr.attr,
> &sensor_dev_attr_temp3_crit_alarm.dev_attr.attr,
> &sensor_dev_attr_temp3_crit_hyst.dev_attr.attr,
> +
> &sensor_dev_attr_power_state.dev_attr.attr,
> NULL
> };
Modify this group to list the attributes only supported by EMC1403
and compatible chips (on top of the core or emc1412 group).
> @@ -286,6 +311,33 @@ static const struct attribute_group emc1403_group = {
> };
>
> static struct attribute *emc1404_attrs[] = {
> + &sensor_dev_attr_temp1_min.dev_attr.attr,
> + &sensor_dev_attr_temp1_max.dev_attr.attr,
> + &sensor_dev_attr_temp1_crit.dev_attr.attr,
> + &sensor_dev_attr_temp1_input.dev_attr.attr,
> + &sensor_dev_attr_temp1_min_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
> +
> + &sensor_dev_attr_temp2_min.dev_attr.attr,
> + &sensor_dev_attr_temp2_max.dev_attr.attr,
> + &sensor_dev_attr_temp2_crit.dev_attr.attr,
> + &sensor_dev_attr_temp2_input.dev_attr.attr,
> + &sensor_dev_attr_temp2_min_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp2_max_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp2_crit_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
> +
> + &sensor_dev_attr_temp3_min.dev_attr.attr,
> + &sensor_dev_attr_temp3_max.dev_attr.attr,
> + &sensor_dev_attr_temp3_crit.dev_attr.attr,
> + &sensor_dev_attr_temp3_input.dev_attr.attr,
> + &sensor_dev_attr_temp3_min_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp3_max_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp3_crit_alarm.dev_attr.attr,
> + &sensor_dev_attr_temp3_crit_hyst.dev_attr.attr,
> +
This is exactly what you don't want to do; it is unnecessary duplication.
There was a reason for having the second group on top of the first one,
as explained above.
> &sensor_dev_attr_temp4_min.dev_attr.attr,
> &sensor_dev_attr_temp4_max.dev_attr.attr,
> &sensor_dev_attr_temp4_crit.dev_attr.attr,
> @@ -294,6 +346,8 @@ static struct attribute *emc1404_attrs[] = {
> &sensor_dev_attr_temp4_max_alarm.dev_attr.attr,
> &sensor_dev_attr_temp4_crit_alarm.dev_attr.attr,
> &sensor_dev_attr_temp4_crit_hyst.dev_attr.attr,
> +
> + &sensor_dev_attr_power_state.dev_attr.attr,
> NULL
> };
>
> @@ -301,11 +355,13 @@ static const struct attribute_group emc1404_group = {
> .attrs = emc1404_attrs,
> };
>
> +
> +
Unnecessary empty lines and unnecessary whitespace change.
> static int emc1403_detect(struct i2c_client *client,
> struct i2c_board_info *info)
> {
> int id;
> - /* Check if thermal chip is SMSC and EMC1403 or EMC1423 */
> + /* Check whether the chip is SMSC and get its type */
This change makes the comment pretty much worthless. It used to describe
what the function is doing, which is no longer the case after your change.
>
> id = i2c_smbus_read_byte_data(client, THERMAL_SMSC_ID_REG);
> if (id != 0x5d)
> @@ -313,6 +369,9 @@ static int emc1403_detect(struct i2c_client *client,
>
> id = i2c_smbus_read_byte_data(client, THERMAL_PID_REG);
> switch (id) {
> + case 0x20:
> + strlcpy(info->type, "emc1412", I2C_NAME_SIZE);
> + break;
> case 0x21:
> strlcpy(info->type, "emc1403", I2C_NAME_SIZE);
> break;
> @@ -330,9 +389,9 @@ static int emc1403_detect(struct i2c_client *client,
> }
>
> id = i2c_smbus_read_byte_data(client, THERMAL_REVISION_REG);
> - if (id != 0x01)
> + if (id != 0x01 && id != 0x04) {
> return -ENODEV;
This should be a separate patch, as it applies to emc1403/emc1404 as well,
so we can backport it into -stable.
> -
> + }
> return 0;
> }
>
> @@ -351,9 +410,17 @@ static int emc1403_probe(struct i2c_client *client,
> mutex_init(&data->mutex);
> data->hyst_valid = jiffies - 1; /* Expired */
>
> - data->groups[0] = &emc1403_group;
> - if (id->driver_data)
> - data->groups[1] = &emc1404_group;
> + switch (id->driver_data) {
> + case 0:
> + data->groups[0] = &emc1412_group;
> + break;
> + case 1:
> + data->groups[0] = &emc1403_group;
> + break;
> + case 2:
> + data->groups[0] = &emc1404_group;
> + break;
> + }
>
Not acceptable. Again, there is a reason for having multiple attribute groups.
> hwmon_dev = hwmon_device_register_with_groups(&client->dev,
> client->name, data,
> @@ -366,14 +433,19 @@ static int emc1403_probe(struct i2c_client *client,
> }
>
> static const unsigned short emc1403_address_list[] = {
> - 0x18, 0x29, 0x4c, 0x4d, I2C_CLIENT_END
> + /* emc1403/emc1404/emc1423/emc1424 */
> + 0x4c, 0x4d, 0x18, 0x29,
> + /* emc1412 */
> + 0x5c, 0x4c, 0x6c, 0x1c, 0x3c, I2C_CLIENT_END
No duplication of addresses, and addresses are by convention in order.
Jean, any addresses which should not be scanned ?
> };
>
> +/* Last number in name indicates the amount of channels */
> static const struct i2c_device_id emc1403_idtable[] = {
> - { "emc1403", 0 },
> - { "emc1404", 1 },
> - { "emc1423", 0 },
> - { "emc1424", 1 },
> + { "emc1412", 0 },
> + { "emc1403", 1 },
> + { "emc1423", 1 },
> + { "emc1404", 2 },
> + { "emc1424", 2 },
It may be time to introduce an enum for supported chip variants.
Also, the list is by custom in alphabetical order.
Guenter
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [lm-sensors] [PATCH] drivers/hwmon/emc1403.c: add support for emc1412
2014-05-11 22:40 ` Guenter Roeck
@ 2014-05-12 2:20 ` Guenter Roeck
-1 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2014-05-12 2:20 UTC (permalink / raw)
To: Josef Gajdusek, jdelvare; +Cc: linux-kernel, lm-sensors
On 05/11/2014 03:40 PM, Guenter Roeck wrote:
[ ... ]
>>
>> id = i2c_smbus_read_byte_data(client, THERMAL_REVISION_REG);
>> - if (id != 0x01)
>> + if (id != 0x01 && id != 0x04) {
>> return -ENODEV;
>
> This should be a separate patch, as it applies to emc1403/emc1404 as well,
> so we can backport it into -stable.
>
Also, the chip datasheet suggests that chip revision 3 exists as well.
Given that, I would suggest to replace the revision number check with
something like
if (id < 0x01 || id > 0x04)
return -ENODEV;
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [lm-sensors] [PATCH] drivers/hwmon/emc1403.c: add support for emc1412
@ 2014-05-12 2:20 ` Guenter Roeck
0 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2014-05-12 2:20 UTC (permalink / raw)
To: Josef Gajdusek, jdelvare; +Cc: linux-kernel, lm-sensors
On 05/11/2014 03:40 PM, Guenter Roeck wrote:
[ ... ]
>>
>> id = i2c_smbus_read_byte_data(client, THERMAL_REVISION_REG);
>> - if (id != 0x01)
>> + if (id != 0x01 && id != 0x04) {
>> return -ENODEV;
>
> This should be a separate patch, as it applies to emc1403/emc1404 as well,
> so we can backport it into -stable.
>
Also, the chip datasheet suggests that chip revision 3 exists as well.
Given that, I would suggest to replace the revision number check with
something like
if (id < 0x01 || id > 0x04)
return -ENODEV;
Guenter
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [lm-sensors] [PATCH] drivers/hwmon/emc1403.c: add support for emc1412
2014-05-11 22:40 ` Guenter Roeck
@ 2014-05-12 6:10 ` Jean Delvare
-1 siblings, 0 replies; 26+ messages in thread
From: Jean Delvare @ 2014-05-12 6:10 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Josef Gajdusek, lm-sensors, linux-kernel
Hi Guenter, Josef,
On Sun, 11 May 2014 15:40:21 -0700, Guenter Roeck wrote:
> On 05/11/2014 06:00 AM, Josef Gajdusek wrote:
> > @@ -366,14 +433,19 @@ static int emc1403_probe(struct i2c_client *client,
> > }
> >
> > static const unsigned short emc1403_address_list[] = {
> > - 0x18, 0x29, 0x4c, 0x4d, I2C_CLIENT_END
> > + /* emc1403/emc1404/emc1423/emc1424 */
> > + 0x4c, 0x4d, 0x18, 0x29,
> > + /* emc1412 */
> > + 0x5c, 0x4c, 0x6c, 0x1c, 0x3c, I2C_CLIENT_END
>
> No duplication of addresses, and addresses are by convention in order.
> Jean, any addresses which should not be scanned ?
0x3c and 0x6c should indeed not be scanned, sensors-detect does not
scan them as they aren't typically used by hwmon devices. 0x5c is
questionable (currently scanned, but used only by a limited number of
chips, we may drop it at some point.) 0x1c and 0x4c are OK to scan.
--
Jean Delvare
SUSE L3 Support
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH] drivers/hwmon/emc1403.c: add support for emc1412
@ 2014-05-12 6:10 ` Jean Delvare
0 siblings, 0 replies; 26+ messages in thread
From: Jean Delvare @ 2014-05-12 6:10 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Josef Gajdusek, lm-sensors, linux-kernel
Hi Guenter, Josef,
On Sun, 11 May 2014 15:40:21 -0700, Guenter Roeck wrote:
> On 05/11/2014 06:00 AM, Josef Gajdusek wrote:
> > @@ -366,14 +433,19 @@ static int emc1403_probe(struct i2c_client *client,
> > }
> >
> > static const unsigned short emc1403_address_list[] = {
> > - 0x18, 0x29, 0x4c, 0x4d, I2C_CLIENT_END
> > + /* emc1403/emc1404/emc1423/emc1424 */
> > + 0x4c, 0x4d, 0x18, 0x29,
> > + /* emc1412 */
> > + 0x5c, 0x4c, 0x6c, 0x1c, 0x3c, I2C_CLIENT_END
>
> No duplication of addresses, and addresses are by convention in order.
> Jean, any addresses which should not be scanned ?
0x3c and 0x6c should indeed not be scanned, sensors-detect does not
scan them as they aren't typically used by hwmon devices. 0x5c is
questionable (currently scanned, but used only by a limited number of
chips, we may drop it at some point.) 0x1c and 0x4c are OK to scan.
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [lm-sensors] [PATCH] drivers/hwmon/emc1403.c: add support for emc1412
2014-05-12 6:10 ` Jean Delvare
@ 2014-05-12 15:59 ` Guenter Roeck
-1 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2014-05-12 15:59 UTC (permalink / raw)
To: Jean Delvare; +Cc: Josef Gajdusek, lm-sensors, linux-kernel
On Mon, May 12, 2014 at 08:10:51AM +0200, Jean Delvare wrote:
> Hi Guenter, Josef,
>
> On Sun, 11 May 2014 15:40:21 -0700, Guenter Roeck wrote:
> > On 05/11/2014 06:00 AM, Josef Gajdusek wrote:
> > > @@ -366,14 +433,19 @@ static int emc1403_probe(struct i2c_client *client,
> > > }
> > >
> > > static const unsigned short emc1403_address_list[] = {
> > > - 0x18, 0x29, 0x4c, 0x4d, I2C_CLIENT_END
> > > + /* emc1403/emc1404/emc1423/emc1424 */
> > > + 0x4c, 0x4d, 0x18, 0x29,
> > > + /* emc1412 */
> > > + 0x5c, 0x4c, 0x6c, 0x1c, 0x3c, I2C_CLIENT_END
> >
> > No duplication of addresses, and addresses are by convention in order.
> > Jean, any addresses which should not be scanned ?
>
> 0x3c and 0x6c should indeed not be scanned, sensors-detect does not
> scan them as they aren't typically used by hwmon devices. 0x5c is
> questionable (currently scanned, but used only by a limited number of
> chips, we may drop it at some point.) 0x1c and 0x4c are OK to scan.
>
Hi Jean,
I only see the adt7462 driver scanning for 0x5c. Guess I'll accept the
address for now; I don't see a good reason not to.
Couple of other questions:
- would it make sense to relax store_hyst to not return ERANGE but use clamp_val
instead ?
- Currently hyst can be stored for all crit attributes even though there is only
one hyst register. Should we change this to only support writing it for
temp1_crit_hyst ?
- I might convert the driver to use regmap if I find the time. Do you have any
concerns with that ?
Thanks,
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH] drivers/hwmon/emc1403.c: add support for emc1412
@ 2014-05-12 15:59 ` Guenter Roeck
0 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2014-05-12 15:59 UTC (permalink / raw)
To: Jean Delvare; +Cc: Josef Gajdusek, lm-sensors, linux-kernel
On Mon, May 12, 2014 at 08:10:51AM +0200, Jean Delvare wrote:
> Hi Guenter, Josef,
>
> On Sun, 11 May 2014 15:40:21 -0700, Guenter Roeck wrote:
> > On 05/11/2014 06:00 AM, Josef Gajdusek wrote:
> > > @@ -366,14 +433,19 @@ static int emc1403_probe(struct i2c_client *client,
> > > }
> > >
> > > static const unsigned short emc1403_address_list[] = {
> > > - 0x18, 0x29, 0x4c, 0x4d, I2C_CLIENT_END
> > > + /* emc1403/emc1404/emc1423/emc1424 */
> > > + 0x4c, 0x4d, 0x18, 0x29,
> > > + /* emc1412 */
> > > + 0x5c, 0x4c, 0x6c, 0x1c, 0x3c, I2C_CLIENT_END
> >
> > No duplication of addresses, and addresses are by convention in order.
> > Jean, any addresses which should not be scanned ?
>
> 0x3c and 0x6c should indeed not be scanned, sensors-detect does not
> scan them as they aren't typically used by hwmon devices. 0x5c is
> questionable (currently scanned, but used only by a limited number of
> chips, we may drop it at some point.) 0x1c and 0x4c are OK to scan.
>
Hi Jean,
I only see the adt7462 driver scanning for 0x5c. Guess I'll accept the
address for now; I don't see a good reason not to.
Couple of other questions:
- would it make sense to relax store_hyst to not return ERANGE but use clamp_val
instead ?
- Currently hyst can be stored for all crit attributes even though there is only
one hyst register. Should we change this to only support writing it for
temp1_crit_hyst ?
- I might convert the driver to use regmap if I find the time. Do you have any
concerns with that ?
Thanks,
Guenter
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [lm-sensors] [PATCH] drivers/hwmon/emc1403.c: add support for emc1412
2014-05-12 15:59 ` Guenter Roeck
@ 2014-05-12 16:48 ` Jean Delvare
-1 siblings, 0 replies; 26+ messages in thread
From: Jean Delvare @ 2014-05-12 16:48 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Josef Gajdusek, lm-sensors, linux-kernel
Hi Guenter,
On Mon, 12 May 2014 08:59:31 -0700, Guenter Roeck wrote:
> I only see the adt7462 driver scanning for 0x5c. Guess I'll accept the
> address for now; I don't see a good reason not to.
sensors-detect also scans it for the SMSC EMC1072, EMC1073 and EMC1074
which we don't support yet. I have no objection to scanning it.
> Couple of other questions:
> - would it make sense to relax store_hyst to not return ERANGE but use clamp_val
> instead ?
Yes, I had exactly the same thought when reading the code this morning.
> - Currently hyst can be stored for all crit attributes even though there is only
> one hyst register. Should we change this to only support writing it for
> temp1_crit_hyst ?
Yes, that would align this driver with what other drivers do.
> - I might convert the driver to use regmap if I find the time. Do you have any
> concerns with that ?
I know nothing about regmap, so no objection, I simply don't care ;-)
--
Jean Delvare
SUSE L3 Support
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] drivers/hwmon/emc1403.c: add support for emc1412
@ 2014-05-12 16:48 ` Jean Delvare
0 siblings, 0 replies; 26+ messages in thread
From: Jean Delvare @ 2014-05-12 16:48 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Josef Gajdusek, lm-sensors, linux-kernel
Hi Guenter,
On Mon, 12 May 2014 08:59:31 -0700, Guenter Roeck wrote:
> I only see the adt7462 driver scanning for 0x5c. Guess I'll accept the
> address for now; I don't see a good reason not to.
sensors-detect also scans it for the SMSC EMC1072, EMC1073 and EMC1074
which we don't support yet. I have no objection to scanning it.
> Couple of other questions:
> - would it make sense to relax store_hyst to not return ERANGE but use clamp_val
> instead ?
Yes, I had exactly the same thought when reading the code this morning.
> - Currently hyst can be stored for all crit attributes even though there is only
> one hyst register. Should we change this to only support writing it for
> temp1_crit_hyst ?
Yes, that would align this driver with what other drivers do.
> - I might convert the driver to use regmap if I find the time. Do you have any
> concerns with that ?
I know nothing about regmap, so no objection, I simply don't care ;-)
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [lm-sensors] [PATCH] drivers/hwmon/emc1403.c: add support for emc1412
2014-05-12 16:48 ` Jean Delvare
@ 2014-05-12 18:09 ` Guenter Roeck
-1 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2014-05-12 18:09 UTC (permalink / raw)
To: Jean Delvare; +Cc: Josef Gajdusek, lm-sensors, linux-kernel
On Mon, May 12, 2014 at 06:48:06PM +0200, Jean Delvare wrote:
> Hi Guenter,
>
> On Mon, 12 May 2014 08:59:31 -0700, Guenter Roeck wrote:
> > I only see the adt7462 driver scanning for 0x5c. Guess I'll accept the
> > address for now; I don't see a good reason not to.
>
> sensors-detect also scans it for the SMSC EMC1072, EMC1073 and EMC1074
> which we don't support yet. I have no objection to scanning it.
>
> > Couple of other questions:
> > - would it make sense to relax store_hyst to not return ERANGE but use clamp_val
> > instead ?
>
> Yes, I had exactly the same thought when reading the code this morning.
>
> > - Currently hyst can be stored for all crit attributes even though there is only
> > one hyst register. Should we change this to only support writing it for
> > temp1_crit_hyst ?
>
> Yes, that would align this driver with what other drivers do.
>
> > - I might convert the driver to use regmap if I find the time. Do you have any
> > concerns with that ?
>
> I know nothing about regmap, so no objection, I simply don't care ;-)
>
Pretty much provides caching.
I'll submit patches for all of it, plus some more (add support for fault
attributes and for the alarm attributes on the emc14x2 chips, and add driver
documentation).
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] drivers/hwmon/emc1403.c: add support for emc1412
@ 2014-05-12 18:09 ` Guenter Roeck
0 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2014-05-12 18:09 UTC (permalink / raw)
To: Jean Delvare; +Cc: Josef Gajdusek, lm-sensors, linux-kernel
On Mon, May 12, 2014 at 06:48:06PM +0200, Jean Delvare wrote:
> Hi Guenter,
>
> On Mon, 12 May 2014 08:59:31 -0700, Guenter Roeck wrote:
> > I only see the adt7462 driver scanning for 0x5c. Guess I'll accept the
> > address for now; I don't see a good reason not to.
>
> sensors-detect also scans it for the SMSC EMC1072, EMC1073 and EMC1074
> which we don't support yet. I have no objection to scanning it.
>
> > Couple of other questions:
> > - would it make sense to relax store_hyst to not return ERANGE but use clamp_val
> > instead ?
>
> Yes, I had exactly the same thought when reading the code this morning.
>
> > - Currently hyst can be stored for all crit attributes even though there is only
> > one hyst register. Should we change this to only support writing it for
> > temp1_crit_hyst ?
>
> Yes, that would align this driver with what other drivers do.
>
> > - I might convert the driver to use regmap if I find the time. Do you have any
> > concerns with that ?
>
> I know nothing about regmap, so no objection, I simply don't care ;-)
>
Pretty much provides caching.
I'll submit patches for all of it, plus some more (add support for fault
attributes and for the alarm attributes on the emc14x2 chips, and add driver
documentation).
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [lm-sensors] [PATCH] drivers/hwmon/emc1403.c: add support for emc1412
2014-05-11 13:00 ` Josef Gajdusek
@ 2014-05-11 22:47 ` Guenter Roeck
-1 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2014-05-11 22:47 UTC (permalink / raw)
To: Josef Gajdusek, jdelvare; +Cc: lm-sensors, linux-kernel
On 05/11/2014 06:00 AM, Josef Gajdusek wrote:
> Adds support for emc1412.
>
[ ... ]
> @@ -330,9 +389,9 @@ static int emc1403_detect(struct i2c_client *client,
> }
>
> id = i2c_smbus_read_byte_data(client, THERMAL_REVISION_REG);
> - if (id != 0x01)
> + if (id != 0x01 && id != 0x04) {
> return -ENODEV;
> -
> + }
Forgot: Please see Documents/CodingStyle, Chapter 3:
"Do not unnecessarily use braces where a single statement will do."
As it turns out, this change generates a checkpatch warning.
Also, you forgot to sign this patch.
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH] drivers/hwmon/emc1403.c: add support for emc1412
@ 2014-05-11 22:47 ` Guenter Roeck
0 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2014-05-11 22:47 UTC (permalink / raw)
To: Josef Gajdusek, jdelvare; +Cc: lm-sensors, linux-kernel
On 05/11/2014 06:00 AM, Josef Gajdusek wrote:
> Adds support for emc1412.
>
[ ... ]
> @@ -330,9 +389,9 @@ static int emc1403_detect(struct i2c_client *client,
> }
>
> id = i2c_smbus_read_byte_data(client, THERMAL_REVISION_REG);
> - if (id != 0x01)
> + if (id != 0x01 && id != 0x04) {
> return -ENODEV;
> -
> + }
Forgot: Please see Documents/CodingStyle, Chapter 3:
"Do not unnecessarily use braces where a single statement will do."
As it turns out, this change generates a checkpatch warning.
Also, you forgot to sign this patch.
Guenter
^ permalink raw reply [flat|nested] 26+ messages in thread
* [lm-sensors] [PATCH] drivers/hwmon/emc1403.c: add support for emc14x2
@ 2014-05-12 12:34 ` Josef Gajdusek
0 siblings, 0 replies; 26+ messages in thread
From: Josef Gajdusek @ 2014-05-12 12:34 UTC (permalink / raw)
To: jdelvare; +Cc: linux, lm-sensors, linux-kernel
Adds support for emc1402/emc1412/emc1422 temperature monitoring chips.
This line of sensors does only have 2 channels (internal and external) in comparison to the emc14x3 (3 channels) and emc14x4 (4 channels) lines.
Signed-off-by: Josef Gajdusek <atx@atx.name>
---
Hello,
I modified the patches according to the feedback I received. I also added 1422 and changed the 1412 part to 1402 (They are practically identical and the rest of the driver uses 140x and 142x
pairs).
---
diff --git a/drivers/hwmon/emc1403.c b/drivers/hwmon/emc1403.c
index b2b6a52..8b3df53 100644
--- a/drivers/hwmon/emc1403.c
+++ b/drivers/hwmon/emc1403.c
@@ -38,9 +38,11 @@
#define THERMAL_SMSC_ID_REG 0xfe
#define THERMAL_REVISION_REG 0xff
+enum emc1403_chip { emc1402, emc1403, emc1404 };
+
struct thermal_data {
struct i2c_client *client;
- const struct attribute_group *groups[3];
+ const struct attribute_group *groups[4];
struct mutex mutex;
/*
* Cache the hyst value so we don't keep re-reading it. In theory
@@ -252,23 +254,36 @@ static SENSOR_DEVICE_ATTR(temp4_crit_hyst, S_IRUGO | S_IWUSR,
static SENSOR_DEVICE_ATTR_2(power_state, S_IRUGO | S_IWUSR,
show_bit, store_bit, 0x03, 0x40);
-static struct attribute *emc1403_attrs[] = {
+static struct attribute *emc1402_attrs[] = {
&sensor_dev_attr_temp1_min.dev_attr.attr,
&sensor_dev_attr_temp1_max.dev_attr.attr,
&sensor_dev_attr_temp1_crit.dev_attr.attr,
&sensor_dev_attr_temp1_input.dev_attr.attr,
- &sensor_dev_attr_temp1_min_alarm.dev_attr.attr,
- &sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
- &sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
&sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
+
&sensor_dev_attr_temp2_min.dev_attr.attr,
&sensor_dev_attr_temp2_max.dev_attr.attr,
&sensor_dev_attr_temp2_crit.dev_attr.attr,
&sensor_dev_attr_temp2_input.dev_attr.attr,
+ &sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
+
+ &sensor_dev_attr_power_state.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group emc1402_group = {
+ .attrs = emc1402_attrs,
+};
+
+static struct attribute *emc1403_attrs[] = {
+ &sensor_dev_attr_temp1_min_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
+
&sensor_dev_attr_temp2_min_alarm.dev_attr.attr,
&sensor_dev_attr_temp2_max_alarm.dev_attr.attr,
&sensor_dev_attr_temp2_crit_alarm.dev_attr.attr,
- &sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
+
&sensor_dev_attr_temp3_min.dev_attr.attr,
&sensor_dev_attr_temp3_max.dev_attr.attr,
&sensor_dev_attr_temp3_crit.dev_attr.attr,
@@ -277,7 +292,6 @@ static struct attribute *emc1403_attrs[] = {
&sensor_dev_attr_temp3_max_alarm.dev_attr.attr,
&sensor_dev_attr_temp3_crit_alarm.dev_attr.attr,
&sensor_dev_attr_temp3_crit_hyst.dev_attr.attr,
- &sensor_dev_attr_power_state.dev_attr.attr,
NULL
};
@@ -313,9 +327,15 @@ static int emc1403_detect(struct i2c_client *client,
id = i2c_smbus_read_byte_data(client, THERMAL_PID_REG);
switch (id) {
+ case 0x20:
+ strlcpy(info->type, "emc1402", I2C_NAME_SIZE);
+ break;
case 0x21:
strlcpy(info->type, "emc1403", I2C_NAME_SIZE);
break;
+ case 0x22:
+ strlcpy(info->type, "emc1422", I2C_NAME_SIZE);
+ break;
case 0x23:
strlcpy(info->type, "emc1423", I2C_NAME_SIZE);
break;
@@ -351,9 +371,14 @@ static int emc1403_probe(struct i2c_client *client,
mutex_init(&data->mutex);
data->hyst_valid = jiffies - 1; /* Expired */
- data->groups[0] = &emc1403_group;
- if (id->driver_data)
- data->groups[1] = &emc1404_group;
+ switch (id->driver_data) {
+ case emc1404:
+ data->groups[2] = &emc1404_group;
+ case emc1403:
+ data->groups[1] = &emc1403_group;
+ case emc1402:
+ data->groups[0] = &emc1402_group;
+ }
hwmon_dev = hwmon_device_register_with_groups(&client->dev,
client->name, data,
@@ -366,14 +391,17 @@ static int emc1403_probe(struct i2c_client *client,
}
static const unsigned short emc1403_address_list[] = {
- 0x18, 0x29, 0x4c, 0x4d, I2C_CLIENT_END
+ 0x18, 0x29, 0x1c, 0x4c, 0x4d, 0x5c, I2C_CLIENT_END
};
+/* Last number in name indicates the amount of channels */
static const struct i2c_device_id emc1403_idtable[] = {
- { "emc1403", 0 },
- { "emc1404", 1 },
- { "emc1423", 0 },
- { "emc1424", 1 },
+ { "emc1402", emc1402 },
+ { "emc1403", emc1403 },
+ { "emc1404", emc1404 },
+ { "emc1422", emc1402 },
+ { "emc1423", emc1403 },
+ { "emc1424", emc1404 },
{ }
};
MODULE_DEVICE_TABLE(i2c, emc1403_idtable);
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply related [flat|nested] 26+ messages in thread* [PATCH] drivers/hwmon/emc1403.c: add support for emc14x2
@ 2014-05-12 12:34 ` Josef Gajdusek
0 siblings, 0 replies; 26+ messages in thread
From: Josef Gajdusek @ 2014-05-12 12:34 UTC (permalink / raw)
To: jdelvare; +Cc: linux, lm-sensors, linux-kernel
Adds support for emc1402/emc1412/emc1422 temperature monitoring chips.
This line of sensors does only have 2 channels (internal and external) in comparison to the emc14x3 (3 channels) and emc14x4 (4 channels) lines.
Signed-off-by: Josef Gajdusek <atx@atx.name>
---
Hello,
I modified the patches according to the feedback I received. I also added 1422 and changed the 1412 part to 1402 (They are practically identical and the rest of the driver uses 140x and 142x
pairs).
---
diff --git a/drivers/hwmon/emc1403.c b/drivers/hwmon/emc1403.c
index b2b6a52..8b3df53 100644
--- a/drivers/hwmon/emc1403.c
+++ b/drivers/hwmon/emc1403.c
@@ -38,9 +38,11 @@
#define THERMAL_SMSC_ID_REG 0xfe
#define THERMAL_REVISION_REG 0xff
+enum emc1403_chip { emc1402, emc1403, emc1404 };
+
struct thermal_data {
struct i2c_client *client;
- const struct attribute_group *groups[3];
+ const struct attribute_group *groups[4];
struct mutex mutex;
/*
* Cache the hyst value so we don't keep re-reading it. In theory
@@ -252,23 +254,36 @@ static SENSOR_DEVICE_ATTR(temp4_crit_hyst, S_IRUGO | S_IWUSR,
static SENSOR_DEVICE_ATTR_2(power_state, S_IRUGO | S_IWUSR,
show_bit, store_bit, 0x03, 0x40);
-static struct attribute *emc1403_attrs[] = {
+static struct attribute *emc1402_attrs[] = {
&sensor_dev_attr_temp1_min.dev_attr.attr,
&sensor_dev_attr_temp1_max.dev_attr.attr,
&sensor_dev_attr_temp1_crit.dev_attr.attr,
&sensor_dev_attr_temp1_input.dev_attr.attr,
- &sensor_dev_attr_temp1_min_alarm.dev_attr.attr,
- &sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
- &sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
&sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
+
&sensor_dev_attr_temp2_min.dev_attr.attr,
&sensor_dev_attr_temp2_max.dev_attr.attr,
&sensor_dev_attr_temp2_crit.dev_attr.attr,
&sensor_dev_attr_temp2_input.dev_attr.attr,
+ &sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
+
+ &sensor_dev_attr_power_state.dev_attr.attr,
+ NULL
+};
+
+static const struct attribute_group emc1402_group = {
+ .attrs = emc1402_attrs,
+};
+
+static struct attribute *emc1403_attrs[] = {
+ &sensor_dev_attr_temp1_min_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp1_max_alarm.dev_attr.attr,
+ &sensor_dev_attr_temp1_crit_alarm.dev_attr.attr,
+
&sensor_dev_attr_temp2_min_alarm.dev_attr.attr,
&sensor_dev_attr_temp2_max_alarm.dev_attr.attr,
&sensor_dev_attr_temp2_crit_alarm.dev_attr.attr,
- &sensor_dev_attr_temp2_crit_hyst.dev_attr.attr,
+
&sensor_dev_attr_temp3_min.dev_attr.attr,
&sensor_dev_attr_temp3_max.dev_attr.attr,
&sensor_dev_attr_temp3_crit.dev_attr.attr,
@@ -277,7 +292,6 @@ static struct attribute *emc1403_attrs[] = {
&sensor_dev_attr_temp3_max_alarm.dev_attr.attr,
&sensor_dev_attr_temp3_crit_alarm.dev_attr.attr,
&sensor_dev_attr_temp3_crit_hyst.dev_attr.attr,
- &sensor_dev_attr_power_state.dev_attr.attr,
NULL
};
@@ -313,9 +327,15 @@ static int emc1403_detect(struct i2c_client *client,
id = i2c_smbus_read_byte_data(client, THERMAL_PID_REG);
switch (id) {
+ case 0x20:
+ strlcpy(info->type, "emc1402", I2C_NAME_SIZE);
+ break;
case 0x21:
strlcpy(info->type, "emc1403", I2C_NAME_SIZE);
break;
+ case 0x22:
+ strlcpy(info->type, "emc1422", I2C_NAME_SIZE);
+ break;
case 0x23:
strlcpy(info->type, "emc1423", I2C_NAME_SIZE);
break;
@@ -351,9 +371,14 @@ static int emc1403_probe(struct i2c_client *client,
mutex_init(&data->mutex);
data->hyst_valid = jiffies - 1; /* Expired */
- data->groups[0] = &emc1403_group;
- if (id->driver_data)
- data->groups[1] = &emc1404_group;
+ switch (id->driver_data) {
+ case emc1404:
+ data->groups[2] = &emc1404_group;
+ case emc1403:
+ data->groups[1] = &emc1403_group;
+ case emc1402:
+ data->groups[0] = &emc1402_group;
+ }
hwmon_dev = hwmon_device_register_with_groups(&client->dev,
client->name, data,
@@ -366,14 +391,17 @@ static int emc1403_probe(struct i2c_client *client,
}
static const unsigned short emc1403_address_list[] = {
- 0x18, 0x29, 0x4c, 0x4d, I2C_CLIENT_END
+ 0x18, 0x29, 0x1c, 0x4c, 0x4d, 0x5c, I2C_CLIENT_END
};
+/* Last number in name indicates the amount of channels */
static const struct i2c_device_id emc1403_idtable[] = {
- { "emc1403", 0 },
- { "emc1404", 1 },
- { "emc1423", 0 },
- { "emc1424", 1 },
+ { "emc1402", emc1402 },
+ { "emc1403", emc1403 },
+ { "emc1404", emc1404 },
+ { "emc1422", emc1402 },
+ { "emc1423", emc1403 },
+ { "emc1424", emc1404 },
{ }
};
MODULE_DEVICE_TABLE(i2c, emc1403_idtable);
^ permalink raw reply related [flat|nested] 26+ messages in thread* Re: [lm-sensors] [PATCH] drivers/hwmon/emc1403.c: add support for emc14x2
2014-05-12 12:34 ` Josef Gajdusek
@ 2014-05-12 16:14 ` Guenter Roeck
-1 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2014-05-12 16:14 UTC (permalink / raw)
To: Josef Gajdusek; +Cc: jdelvare, lm-sensors, linux-kernel
Hi Josef,
On Mon, May 12, 2014 at 02:34:09PM +0200, Josef Gajdusek wrote:
> Adds support for emc1402/emc1412/emc1422 temperature monitoring chips.
> This line of sensors does only have 2 channels (internal and external) in comparison to the emc14x3 (3 channels) and emc14x4 (4 channels) lines.
>
> Signed-off-by: Josef Gajdusek <atx@atx.name>
Applied, with a couple of minor adjustments.
> ---
[ ... ]
>
> static const unsigned short emc1403_address_list[] = {
> - 0x18, 0x29, 0x4c, 0x4d, I2C_CLIENT_END
> + 0x18, 0x29, 0x1c, 0x4c, 0x4d, 0x5c, I2C_CLIENT_END
Changed to numerical order.
> };
>
> +/* Last number in name indicates the amount of channels */
I found that comment a bit confusing, so I changed it to
/* Last digit of chip name indicates number of channels */
> static const struct i2c_device_id emc1403_idtable[] = {
> - { "emc1403", 0 },
> - { "emc1404", 1 },
> - { "emc1423", 0 },
> - { "emc1424", 1 },
> + { "emc1402", emc1402 },
> + { "emc1403", emc1403 },
> + { "emc1404", emc1404 },
> + { "emc1422", emc1402 },
> + { "emc1423", emc1403 },
> + { "emc1424", emc1404 },
Wonder if we should list the emc141x chips here. Jean, any thoughts ?
> { }
> };
> MODULE_DEVICE_TABLE(i2c, emc1403_idtable);
>
It would be nice to also have support for the alarms on EMC14x2,
but that can be a separate patch.
Thanks,
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH] drivers/hwmon/emc1403.c: add support for emc14x2
@ 2014-05-12 16:14 ` Guenter Roeck
0 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2014-05-12 16:14 UTC (permalink / raw)
To: Josef Gajdusek; +Cc: jdelvare, lm-sensors, linux-kernel
Hi Josef,
On Mon, May 12, 2014 at 02:34:09PM +0200, Josef Gajdusek wrote:
> Adds support for emc1402/emc1412/emc1422 temperature monitoring chips.
> This line of sensors does only have 2 channels (internal and external) in comparison to the emc14x3 (3 channels) and emc14x4 (4 channels) lines.
>
> Signed-off-by: Josef Gajdusek <atx@atx.name>
Applied, with a couple of minor adjustments.
> ---
[ ... ]
>
> static const unsigned short emc1403_address_list[] = {
> - 0x18, 0x29, 0x4c, 0x4d, I2C_CLIENT_END
> + 0x18, 0x29, 0x1c, 0x4c, 0x4d, 0x5c, I2C_CLIENT_END
Changed to numerical order.
> };
>
> +/* Last number in name indicates the amount of channels */
I found that comment a bit confusing, so I changed it to
/* Last digit of chip name indicates number of channels */
> static const struct i2c_device_id emc1403_idtable[] = {
> - { "emc1403", 0 },
> - { "emc1404", 1 },
> - { "emc1423", 0 },
> - { "emc1424", 1 },
> + { "emc1402", emc1402 },
> + { "emc1403", emc1403 },
> + { "emc1404", emc1404 },
> + { "emc1422", emc1402 },
> + { "emc1423", emc1403 },
> + { "emc1424", emc1404 },
Wonder if we should list the emc141x chips here. Jean, any thoughts ?
> { }
> };
> MODULE_DEVICE_TABLE(i2c, emc1403_idtable);
>
It would be nice to also have support for the alarms on EMC14x2,
but that can be a separate patch.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [lm-sensors] [PATCH] drivers/hwmon/emc1403.c: add support for emc14x2
2014-05-12 16:14 ` Guenter Roeck
@ 2014-05-12 17:03 ` Jean Delvare
-1 siblings, 0 replies; 26+ messages in thread
From: Jean Delvare @ 2014-05-12 17:03 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Josef Gajdusek, lm-sensors, linux-kernel
On Mon, 12 May 2014 09:14:15 -0700, Guenter Roeck wrote:
> On Mon, May 12, 2014 at 02:34:09PM +0200, Josef Gajdusek wrote:
> > static const struct i2c_device_id emc1403_idtable[] = {
> > - { "emc1403", 0 },
> > - { "emc1404", 1 },
> > - { "emc1423", 0 },
> > - { "emc1424", 1 },
> > + { "emc1402", emc1402 },
> > + { "emc1403", emc1403 },
> > + { "emc1404", emc1404 },
> > + { "emc1422", emc1402 },
> > + { "emc1423", emc1403 },
> > + { "emc1424", emc1404 },
>
> Wonder if we should list the emc141x chips here. Jean, any thoughts ?
Yes we should, so that people can declare the right chip in platform
files, device tree etc. We can map the additional names to existing
types if the chips are fully compatible.
--
Jean Delvare
SUSE L3 Support
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH] drivers/hwmon/emc1403.c: add support for emc14x2
@ 2014-05-12 17:03 ` Jean Delvare
0 siblings, 0 replies; 26+ messages in thread
From: Jean Delvare @ 2014-05-12 17:03 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Josef Gajdusek, lm-sensors, linux-kernel
On Mon, 12 May 2014 09:14:15 -0700, Guenter Roeck wrote:
> On Mon, May 12, 2014 at 02:34:09PM +0200, Josef Gajdusek wrote:
> > static const struct i2c_device_id emc1403_idtable[] = {
> > - { "emc1403", 0 },
> > - { "emc1404", 1 },
> > - { "emc1423", 0 },
> > - { "emc1424", 1 },
> > + { "emc1402", emc1402 },
> > + { "emc1403", emc1403 },
> > + { "emc1404", emc1404 },
> > + { "emc1422", emc1402 },
> > + { "emc1423", emc1403 },
> > + { "emc1424", emc1404 },
>
> Wonder if we should list the emc141x chips here. Jean, any thoughts ?
Yes we should, so that people can declare the right chip in platform
files, device tree etc. We can map the additional names to existing
types if the chips are fully compatible.
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [lm-sensors] [PATCH] drivers/hwmon/emc1403.c: add support for emc14x2
2014-05-12 17:03 ` Jean Delvare
@ 2014-05-12 18:47 ` Guenter Roeck
-1 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2014-05-12 18:47 UTC (permalink / raw)
To: Jean Delvare; +Cc: Josef Gajdusek, lm-sensors, linux-kernel
On Mon, May 12, 2014 at 07:03:27PM +0200, Jean Delvare wrote:
> On Mon, 12 May 2014 09:14:15 -0700, Guenter Roeck wrote:
> > On Mon, May 12, 2014 at 02:34:09PM +0200, Josef Gajdusek wrote:
> > > static const struct i2c_device_id emc1403_idtable[] = {
> > > - { "emc1403", 0 },
> > > - { "emc1404", 1 },
> > > - { "emc1423", 0 },
> > > - { "emc1424", 1 },
> > > + { "emc1402", emc1402 },
> > > + { "emc1403", emc1403 },
> > > + { "emc1404", emc1404 },
> > > + { "emc1422", emc1402 },
> > > + { "emc1423", emc1403 },
> > > + { "emc1424", emc1404 },
> >
> > Wonder if we should list the emc141x chips here. Jean, any thoughts ?
>
> Yes we should, so that people can declare the right chip in platform
> files, device tree etc. We can map the additional names to existing
> types if the chips are fully compatible.
>
The chips are not only compatible, the even have the same device IDs.
Maybe I missed it, but I did not find a difference.
I'll add another patch to my list.
Guenter
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [PATCH] drivers/hwmon/emc1403.c: add support for emc14x2
@ 2014-05-12 18:47 ` Guenter Roeck
0 siblings, 0 replies; 26+ messages in thread
From: Guenter Roeck @ 2014-05-12 18:47 UTC (permalink / raw)
To: Jean Delvare; +Cc: Josef Gajdusek, lm-sensors, linux-kernel
On Mon, May 12, 2014 at 07:03:27PM +0200, Jean Delvare wrote:
> On Mon, 12 May 2014 09:14:15 -0700, Guenter Roeck wrote:
> > On Mon, May 12, 2014 at 02:34:09PM +0200, Josef Gajdusek wrote:
> > > static const struct i2c_device_id emc1403_idtable[] = {
> > > - { "emc1403", 0 },
> > > - { "emc1404", 1 },
> > > - { "emc1423", 0 },
> > > - { "emc1424", 1 },
> > > + { "emc1402", emc1402 },
> > > + { "emc1403", emc1403 },
> > > + { "emc1404", emc1404 },
> > > + { "emc1422", emc1402 },
> > > + { "emc1423", emc1403 },
> > > + { "emc1424", emc1404 },
> >
> > Wonder if we should list the emc141x chips here. Jean, any thoughts ?
>
> Yes we should, so that people can declare the right chip in platform
> files, device tree etc. We can map the additional names to existing
> types if the chips are fully compatible.
>
The chips are not only compatible, the even have the same device IDs.
Maybe I missed it, but I did not find a difference.
I'll add another patch to my list.
Guenter
^ permalink raw reply [flat|nested] 26+ messages in thread* Re: [lm-sensors] [PATCH] drivers/hwmon/emc1403.c: add support for emc14x2
2014-05-12 18:47 ` Guenter Roeck
@ 2014-05-12 19:55 ` Jean Delvare
-1 siblings, 0 replies; 26+ messages in thread
From: Jean Delvare @ 2014-05-12 19:55 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Josef Gajdusek, lm-sensors, linux-kernel
On Mon, 12 May 2014 11:47:27 -0700, Guenter Roeck wrote:
> The chips are not only compatible, the even have the same device IDs.
> Maybe I missed it, but I did not find a difference.
The EMC141x seem to have fewer package options (no SOIC) but better
internal sensor accuracy (+/- 1°C instead of +/- 2°C.) The +/- 1°C
accuracy for the external channels is also guaranteed over a wider
temperature range. Just from a quick look at the product features,
there may be more.
--
Jean Delvare
SUSE L3 Support
_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
^ permalink raw reply [flat|nested] 26+ messages in thread
* Re: [PATCH] drivers/hwmon/emc1403.c: add support for emc14x2
@ 2014-05-12 19:55 ` Jean Delvare
0 siblings, 0 replies; 26+ messages in thread
From: Jean Delvare @ 2014-05-12 19:55 UTC (permalink / raw)
To: Guenter Roeck; +Cc: Josef Gajdusek, lm-sensors, linux-kernel
On Mon, 12 May 2014 11:47:27 -0700, Guenter Roeck wrote:
> The chips are not only compatible, the even have the same device IDs.
> Maybe I missed it, but I did not find a difference.
The EMC141x seem to have fewer package options (no SOIC) but better
internal sensor accuracy (+/- 1°C instead of +/- 2°C.) The +/- 1°C
accuracy for the external channels is also guaranteed over a wider
temperature range. Just from a quick look at the product features,
there may be more.
--
Jean Delvare
SUSE L3 Support
^ permalink raw reply [flat|nested] 26+ messages in thread