Skip to contentSkip to conversation

hwmon: (nct7802) add temperature sensor type attribute

PATCH In review Constantine Shulyupin 2 participants 2 messages Jun 27, 2015

Subsystems (2)
Hardware monitoring Checkpatch
lore
Share & tools
lore

Constantine Shulyupin

author
#0 lore raw

PATCH 1/1 hwmon: (nct7802) add temperature sensor type attribute

From: const <[email protected]>

0, 3 - Temperature attributes are hidden 1 - Current mode
2 - Thermistor mode

Reference:
Nuvoton Hardware Monitoring IC NCT7802Y 7.2.32 Mode Selection Register Location : Index 22h

Signed-off-by Constantine Shulyupin <[email protected]>
---
 drivers/hwmon/nct7802.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-
 scripts/checkpatch.pl   |  2 +-
 2 files changed, 53 insertions(+), 2 deletions(-)
drivers/hwmon/nct7802.c +52 -1
index 60cf5d1..444eedf 100644
@@ -49,7 +49,7 @@ static const u8 REG_VOLTAGE_LIMIT_MSB_SHIFT[2][5] = {
 #define REG_VOLTAGE_LOW		0x0f
 #define REG_FANCOUNT_LOW	0x13
 #define REG_START		0x21
-#define REG_MODE		0x22
+#define REG_MODE		0x22 /* 7.2.32 Mode Selection Register */
 #define REG_PECI_ENABLE		0x23
 #define REG_FAN_ENABLE		0x24
 #define REG_VMON_ENABLE		0x25

@@ -725,7 +725,58 @@ static struct attribute_group nct7802_fan_group = {
 	.is_visible = nct7802_fan_is_visible,
 };
 
+static ssize_t show_temp_type(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{
+	struct nct7802_data *data = dev_get_drvdata(dev);
+	struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(data->regmap, REG_MODE, &val);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%u\n", val >> (2 * sattr->index) & 3);
+}
+
+static ssize_t store_temp_type(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct nct7802_data *data = dev_get_drvdata(dev);
+	struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
+	unsigned int val;
+	int err;
+
+	err = kstrtouint(buf, 0, &val);
+	if (err < 0)
+		return err;
+	if (val > 0xF)
+		return -EINVAL;
+
+	err = regmap_update_bits(data->regmap, REG_MODE,
+			3 << 2 * sattr->index, val << 2 * sattr->index);
+	sysfs_update_group(&dev->kobj, &nct7802_temp_group);
+	return err ? : count;
+}
+
+/* 7.2.32 Mode Selection Register */
+static SENSOR_DEVICE_ATTR(temp1_type, S_IRUGO | S_IWUSR,
+		show_temp_type, store_temp_type, 0);
+static SENSOR_DEVICE_ATTR(temp2_type, S_IRUGO | S_IWUSR,
+		show_temp_type, store_temp_type, 1);
+static SENSOR_DEVICE_ATTR(temp3_type, S_IRUGO | S_IWUSR,
+		show_temp_type, store_temp_type, 2);
+
+static struct attribute *general_attributes[] = {
+	&sensor_dev_attr_temp1_type.dev_attr.attr,
+	&sensor_dev_attr_temp2_type.dev_attr.attr,
+	&sensor_dev_attr_temp3_type.dev_attr.attr,
+	NULL
+};
+
 static const struct attribute_group *nct7802_groups[] = {
+	&(const struct attribute_group){ .attrs = general_attributes },
 	&nct7802_temp_group,
 	&nct7802_in_group,
 	&nct7802_fan_group,
scripts/checkpatch.pl +1 -1
index 4f484ac..bf9a680 100755
@@ -14,7 +14,7 @@ my $V = '0.32';
 
 use Getopt::Long qw(:config no_auto_abbrev);
 
-my $LINELENGTH0;
+my $LINELENGTH€;
 
 my $quiet = 0;
 my $tree = 1;
Signature · 7 lines
1.9.1


_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors
maintainer
#1 lore raw

Hi Constantine,

Quoted 3 lines · Constantine Shulyupin

From: const <[email protected]>

0, 3 - Temperature attributes are hidden

Not really per your code (though they should be hidden).

Quoted 3 lines

1 - Current mode
2 - Thermistor mode

Wrong - that should be 3 for current (diode) mode and 4 for thermistor mode. The values are part of the ABI and must not be changed
(look at the output of the sensors command and you'll understand why).

Quoted 31 lines

Reference:
Nuvoton Hardware Monitoring IC NCT7802Y 7.2.32 Mode Selection Register Location : Index 22h

Signed-off-by Constantine Shulyupin <[email protected]>
---
  drivers/hwmon/nct7802.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-
  scripts/checkpatch.pl   |  2 +-
  2 files changed, 53 insertions(+), 2 deletions(-)
drivers/hwmon/nct7802.c +4 -1
index 60cf5d1..444eedf 100644
@@ -49,7 +49,7 @@ static const u8 REG_VOLTAGE_LIMIT_MSB_SHIFT[2][5] = {
  #define REG_VOLTAGE_LOW		0x0f
  #define REG_FANCOUNT_LOW	0x13
  #define REG_START		0x21
-#define REG_MODE		0x22
+#define REG_MODE		0x22 /* 7.2.32 Mode Selection Register */
  #define REG_PECI_ENABLE		0x23
  #define REG_FAN_ENABLE		0x24
  #define REG_VMON_ENABLE		0x25

@@ -725,7 +725,58 @@ static struct attribute_group nct7802_fan_group = {
  	.is_visible = nct7802_fan_is_visible,
  };
 
+static ssize_t show_temp_type(struct device *dev, struct device_attribute *attr,
+		char *buf)
+{

drivers/hwmon/nct7802.c static struct attribute_group nct7802_fan_group = { · line 740

Quoted 10 lines
drivers/hwmon/nct7802.c +10 -0
+	struct nct7802_data *data = dev_get_drvdata(dev);
+	struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
+	unsigned int val;
+	int ret;
+
+	ret = regmap_read(data->regmap, REG_MODE, &val);
+	if (ret < 0)
+		return ret;
+
+	return sprintf(buf, "%u\n", val >> (2 * sattr->index) & 3);

Please convert to correct ABI values.

Quoted 6 lines
drivers/hwmon/nct7802.c +6 -0
+}
+
+static ssize_t store_temp_type(struct device *dev,
+		struct device_attribute *attr, const char *buf, size_t count)
+{
+	struct nct7802_data *data = dev_get_drvdata(dev);

drivers/hwmon/nct7802.c static struct attribute_group nct7802_fan_group = { · line 756

Quoted 10 lines
drivers/hwmon/nct7802.c +10 -0
+	struct sensor_device_attribute *sattr = to_sensor_dev_attr(attr);
+	unsigned int val;
+	int err;
+
+	err = kstrtouint(buf, 0, &val);
+	if (err < 0)
+		return err;
+	if (val > 0xF)
+		return -EINVAL;
+

Please only accept 3 and 4 for the first two sensors, and only 4 for the third sensor, and convert to register values as needed.

drivers/hwmon/nct7802.c static struct attribute_group nct7802_fan_group = { · line 769

Quoted 13 lines
drivers/hwmon/nct7802.c +13 -0
+	err = regmap_update_bits(data->regmap, REG_MODE,
+			3 << 2 * sattr->index, val << 2 * sattr->index);
+	sysfs_update_group(&dev->kobj, &nct7802_temp_group);
+	return err ? : count;
+}
+
+/* 7.2.32 Mode Selection Register */
+static SENSOR_DEVICE_ATTR(temp1_type, S_IRUGO | S_IWUSR,
+		show_temp_type, store_temp_type, 0);
+static SENSOR_DEVICE_ATTR(temp2_type, S_IRUGO | S_IWUSR,
+		show_temp_type, store_temp_type, 1);
+static SENSOR_DEVICE_ATTR(temp3_type, S_IRUGO | S_IWUSR,
+		show_temp_type, store_temp_type, 2);

Please align continuation lines with '('.

drivers/hwmon/nct7802.c static struct attribute_group nct7802_fan_group = { · line 779

Quoted 10 lines
drivers/hwmon/nct7802.c +9 -0
+
+static struct attribute *general_attributes[] = {
+	&sensor_dev_attr_temp1_type.dev_attr.attr,
+	&sensor_dev_attr_temp2_type.dev_attr.attr,
+	&sensor_dev_attr_temp3_type.dev_attr.attr,
+	NULL
+};
+
  static const struct attribute_group *nct7802_groups[] = {
+	&(const struct attribute_group){ .attrs = general_attributes },

Please merge the new attributes into temp_group to get correct visibility.

Quoted 8 lines
drivers/hwmon/nct7802.c +0 -0
  	&nct7802_temp_group,
  	&nct7802_in_group,
  	&nct7802_fan_group,
scripts/checkpatch.pl +0 -0
index 4f484ac..bf9a680 100755

scripts/checkpatch.pl my $V = '0.32'; · line 18

Quoted 6 lines
scripts/checkpatch.pl +1 -1
@@ -14,7 +14,7 @@ my $V = '0.32';
 
  use Getopt::Long qw(:config no_auto_abbrev);
 
-my $LINELENGTH0;
+my $LINELENGTH€;

That doesn't belong here.

scripts/checkpatch.pl my $V = '0.32'; · line 20

Quoted 3 lines
scripts/checkpatch.pl +0 -0
  my $quiet = 0;
  my $tree = 1;

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors