Re: [PATCH v1] hwmon: (it87) Separate temperature type to separate funtion and detect AMDTSI

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 6/25/23 04:18, Frank Crawford wrote:
The temperature sensor type needs to be used in multiple places, so
split it out into its own function.  It is used both in show_temp_type
attribute and testing if the attribute should be visible (i.e. is
defined).

The sensor type also reports the type AMDTSI on certain chipsets.

Signed-off-by: Frank Crawford <frank@xxxxxxxxxxxxxxxxxx>
---
  drivers/hwmon/it87.c | 78 ++++++++++++++++++++++++++++++++++++--------
  1 file changed, 65 insertions(+), 13 deletions(-)

diff --git a/drivers/hwmon/it87.c b/drivers/hwmon/it87.c
index 5deff5e5f693..ba75f33301ce 100644
--- a/drivers/hwmon/it87.c
+++ b/drivers/hwmon/it87.c
@@ -221,6 +221,10 @@ static bool fix_pwm_polarity;
   * Super-I/O configuration space.
   */
  #define IT87_REG_VID           0x0a
+
+/* Interface Selection register on other chips */
+#define IT87_REG_IFSEL         0x0a
+
  /*
   * The IT8705F and IT8712F earlier than revision 0x08 use register 0x0b
   * for fan divisors. Later IT8712F revisions must use 16-bit tachometer
@@ -1159,28 +1163,68 @@ static SENSOR_DEVICE_ATTR_2(temp4_input, S_IRUGO, show_temp, NULL, 3, 0);
  static SENSOR_DEVICE_ATTR_2(temp5_input, S_IRUGO, show_temp, NULL, 4, 0);
  static SENSOR_DEVICE_ATTR_2(temp6_input, S_IRUGO, show_temp, NULL, 5, 0);
+static int get_temp_type(struct it87_data *data, int index)
+{
+	/*
+	 * 2 is deprecated;
+	 * 3 = thermal diode;
+	 * 4 = thermistor;
+	 * 5 = AMDTSI;
+	 * 6 = Intel PECI;
+	 * 0 = disabled
+	 */
+	u8 reg, extra;
+	int ttype, type = 0;
+
+	/* Detect PECI vs. AMDTSI */
+	ttype = 6;
+	if ((has_temp_peci(data, index)) || data->type == it8721 ||
+	    data->type == it8720) {
+		extra = it87_read_value(data, IT87_REG_IFSEL);
+		if ((extra & 0x70) == 0x40)
+			ttype = 5;
+	}
+
+	reg = it87_read_value(data, IT87_REG_TEMP_ENABLE);
+
+	/* Per chip special detection */
+	switch (data->type) {
+	case it8622:
+		if (!(reg & 0xc0) && index == 3)
+			type = ttype;
+		break;
+	default:
+		break;
+	}
+
+	if (type || index >= 3)
+		return type;
+

The above is really all new code, not just refactoring,
and needs to be explained separately. Please split the patches
into refactoring and separate patch(es) for functional changes.

Thanks,
Guenter

+	extra = it87_read_value(data, IT87_REG_TEMP_EXTRA);
+
+	if ((has_temp_peci(data, index) && (reg >> 6 == index + 1)) ||
+	    (has_temp_old_peci(data, index) && (extra & 0x80)))
+		type = ttype;	/* Intel PECI or AMDTSI */
+	else if (reg & BIT(index))
+		type = 3;	/* thermal diode */
+	else if (reg & BIT(index + 3))
+		type = 4;	/* thermistor */
+
+	return type;
+}
+
  static ssize_t show_temp_type(struct device *dev, struct device_attribute *attr,
  			      char *buf)
  {
  	struct sensor_device_attribute *sensor_attr = to_sensor_dev_attr(attr);
-	int nr = sensor_attr->index;
  	struct it87_data *data = it87_update_device(dev);
-	u8 reg, extra;
+	int type;
if (IS_ERR(data))
  		return PTR_ERR(data);
- reg = data->sensor; /* In case value is updated while used */
-	extra = data->extra;
-
-	if ((has_temp_peci(data, nr) && (reg >> 6 == nr + 1)) ||
-	    (has_temp_old_peci(data, nr) && (extra & 0x80)))
-		return sprintf(buf, "6\n");  /* Intel PECI */
-	if (reg & (1 << nr))
-		return sprintf(buf, "3\n");  /* thermal diode */
-	if (reg & (8 << nr))
-		return sprintf(buf, "4\n");  /* thermistor */
-	return sprintf(buf, "0\n");      /* disabled */
+	type = get_temp_type(data, sensor_attr->index);
+	return sprintf(buf, "%d\n", type);
  }
static ssize_t set_temp_type(struct device *dev, struct device_attribute *attr,
@@ -2313,6 +2357,14 @@ static umode_t it87_temp_is_visible(struct kobject *kobj,
  	if (!(data->has_temp & BIT(i)))
  		return 0;
+ if (a == 3) {
+		int type = get_temp_type(data, i);
+
+		if (type == 0)
+			return 0;
+		return attr->mode;
+	}
+
  	if (a == 5 && !has_temp_offset(data))
  		return 0;




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux