hwmon: some jc42 changes

In set_temp_crit_hyst(), make the variable 'val' have the correct type for strict_strtoul().

Signed-off-by Clemens Ladisch <[email protected]>

The documentation lists standard numbers and chip names in excruciating detail, but that's all it does. To help mere mortals in deciding whether to enable this driver, mention what this sensor is for and in which systems it might be found.

Also add a link to the actual JC 42.4 specification.

Signed-off-by Clemens Ladisch <[email protected]>

On systems where the temperature sensor is actually used, the BIOS is likely to have locked the alarm registers. In that case, all writes through the corresponding sysfs files would be silently ignored.

To prevent this, detect the locks and make the affected sysfs files read-only.

Signed-off-by Clemens Ladisch <[email protected]>

---
 Documentation/hwmon/jc42 |   12 ++++++++----
 drivers/hwmon/jc42.c     |   33 +++++++++++++++++++++++++++++----
 2 files changed, 37 insertions(+), 8 deletions(-)

Documentation/hwmon/jc42 +8 -4

@@ -86,15 +86,19 @@ limits. The chip supports only a single
 which applies to all limits. This register can be written by writing into
 temp1_crit_hyst. Other hysteresis attributes are read-only.
 
+If the BIOS has configured the sensor for automatic temperature management, it
+is likely that it has locked the registers, i.e., that the temperature limits
+cannot be changed.
+
 Sysfs entries
 -------------
 
 temp1_input		Temperature (RO)
-temp1_min		Minimum temperature (RW)
-temp1_max		Maximum temperature (RW)
-temp1_crit		Critical high temperature (RW)
+temp1_min		Minimum temperature (RO or RW)
+temp1_max		Maximum temperature (RO or RW)
+temp1_crit		Critical high temperature (RO or RW)
 
-temp1_crit_hyst		Critical hysteresis temperature (RW)
+temp1_crit_hyst		Critical hysteresis temperature (RO or RW)
 temp1_max_hyst		Maximum hysteresis temperature (RO)
 
 temp1_min_alarm		Temperature low alarm

drivers/hwmon/jc42.c +7 -4

@@ -53,6 +53,8 @@ static const unsigned short normal_i2c[]
 
 /* Configuration register defines */
 #define JC42_CFG_CRIT_ONLY	(1 << 2)
+#define JC42_CFG_TCRIT_LOCK	(1 << 6)
+#define JC42_CFG_EVENT_LOCK	(1 << 7)
 #define JC42_CFG_SHUTDOWN	(1 << 8)
 #define JC42_CFG_HYST_SHIFT	9
 #define JC42_CFG_HYST_MASK	0x03

@@ -380,14 +382,14 @@ static ssize_t show_alarm(struct device
 
 static DEVICE_ATTR(temp1_input, S_IRUGO,
 		   show_temp_input, NULL);
-static DEVICE_ATTR(temp1_crit, S_IWUSR | S_IRUGO,
+static DEVICE_ATTR(temp1_crit, S_IRUGO,
 		   show_temp_crit, set_temp_crit);
-static DEVICE_ATTR(temp1_min, S_IWUSR | S_IRUGO,
+static DEVICE_ATTR(temp1_min, S_IRUGO,
 		   show_temp_min, set_temp_min);
-static DEVICE_ATTR(temp1_max, S_IWUSR | S_IRUGO,
+static DEVICE_ATTR(temp1_max, S_IRUGO,
 		   show_temp_max, set_temp_max);
 
-static DEVICE_ATTR(temp1_crit_hyst, S_IWUSR | S_IRUGO,
+static DEVICE_ATTR(temp1_crit_hyst, S_IRUGO,
 		   show_temp_crit_hyst, set_temp_crit_hyst);
 static DEVICE_ATTR(temp1_max_hyst, S_IRUGO,
 		   show_temp_max_hyst, NULL);

@@ -412,8 +414,31 @@ static struct attribute *jc42_attributes
 	NULL
 };
 
+static mode_t jc42_attribute_mode(struct kobject *kobj,

drivers/hwmon/jc42.c +12 -0

+	else if (attr = &dev_attr_temp1_min.attr ||
+		 attr = &dev_attr_temp1_max.attr)
+		readonly = config & JC42_CFG_EVENT_LOCK;
+	else if (attr = &dev_attr_temp1_crit_hyst.attr)
+		readonly = config & (JC42_CFG_EVENT_LOCK | JC42_CFG_TCRIT_LOCK);
+	else
+		readonly = true;
+
+	return S_IRUGO | (readonly ? 0 : S_IWUSR);
+}
+
 static const struct attribute_group jc42_group = {
 	.attrs = jc42_attributes,
+	.is_visible = jc42_attribute_mode,
 };
 
 /* Return 0 if detection is successful, -ENODEV otherwise */

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

drivers/hwmon/jc42.c static struct attribute *jc42_attributes · line 427

Quoted 1 line · Clemens Ladisch

drivers/hwmon/jc42.c +1 -0

+		readonly = config & JC42_CFG_TCRIT_LOCK;

You are assigning a non-bool to a bool. I can see that recent C compilers do the right thing, but I am not sure if that is always the case. So I would prefer

		readonly = !!(config & JC42_CFG_TCRIT_LOCK));

Same for the assignments below. I can make that change if you are ok with it.

Quoted 8 lines · Guenter Roeck

drivers/hwmon/jc42.c static struct attribute *jc42_attributes · line 427

Quoted 1 line · Clemens Ladisch

drivers/hwmon/jc42.c +1 -0

+		readonly = config & JC42_CFG_TCRIT_LOCK;

You are assigning a non-bool to a bool. I can see that recent C compilers do the right thing, but I am not sure if that is always the case. So I would prefer

		readonly = !!(config & JC42_CFG_TCRIT_LOCK);

Quoted 1 line

Same for the assignments below. I can make that change if you are ok with it.

I cannot imagine how a compiler could get this wrong even if it tried to, but if you think so, go ahead. :)

Quoted 9 lines · Clemens Ladisch

Quoted 8 lines · Guenter Roeck

drivers/hwmon/jc42.c static struct attribute *jc42_attributes · line 427

Quoted 1 line · Clemens Ladisch

drivers/hwmon/jc42.c +1 -0

+		readonly = config & JC42_CFG_TCRIT_LOCK;

You are assigning a non-bool to a bool. I can see that recent C compilers do the right thing, but I am not sure if that is always the case. So I would prefer

		readonly = !!(config & JC42_CFG_TCRIT_LOCK);

Quoted 4 lines

Quoted 1 line

Same for the assignments below. I can make that change if you are ok with it.

I cannot imagine how a compiler could get this wrong even if it tried to, but if you think so, go ahead. :)

I don't know. Maybe I am just paranoid. Using !! is how I usually see it done.

Quoted 15 lines · Guenter Roeck

Quoted 13 lines · Clemens Ladisch

Quoted 9 lines · Guenter Roeck

drivers/hwmon/jc42.c static struct attribute *jc42_attributes · line 427

Quoted 1 line · Clemens Ladisch

drivers/hwmon/jc42.c +1 -0

+		readonly = config & JC42_CFG_TCRIT_LOCK;

You are assigning a non-bool to a bool. I can see that recent C compilers do the right thing, but I am not sure if that is always the case. So I would prefer

		readonly = !!(config & JC42_CFG_TCRIT_LOCK);

Same for the assignments below. I can make that change if you are ok with it.

I cannot imagine how a compiler could get this wrong even if it tried to, but if you think so, go ahead. :)

Quoted 1 line

I don't know. Maybe I am just paranoid. Using !! is how I usually see it done.

Usually, !! is used to convert non-zero to the _integer_ 1. With a compiler that implements bool, this conversion is already implied. On older compilers, someone might be tempted to do "#define bool int", but this is not an issue with the compilers required by Linux.