All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan count
@ 2011-02-11 17:20 Guenter Roeck
  2011-02-12  3:09 ` [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan Andrew Lutomirski
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Guenter Roeck @ 2011-02-11 17:20 UTC (permalink / raw)
  To: lm-sensors

Some of the chips supported by this driver have 13 bit or 16 bit fan count
registers. This patch improves support for those registers, specifically for
NCT6775F. With the changes in this patch, fan speed is reported correctly even
if the fan divider is set to a low value, which results in a fan speed reading
above 0xff.

With this patch, the width of fan count registers is no longer used to determine
if the chip has fan divider register(s) or not. A dedicated flag is used instead
to determine if this is the case.

Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
---
 drivers/hwmon/w83627ehf.c |   86 +++++++++++++++++++++++++++++++--------------
 1 files changed, 59 insertions(+), 27 deletions(-)

diff --git a/drivers/hwmon/w83627ehf.c b/drivers/hwmon/w83627ehf.c
index a6616b7..473e052 100644
--- a/drivers/hwmon/w83627ehf.c
+++ b/drivers/hwmon/w83627ehf.c
@@ -234,7 +234,7 @@ static const u16 NCT6775_REG_FAN_STOP_TIME[] = { 0x107, 0x207, 0x307 };
 static const u16 NCT6775_REG_PWM[] = { 0x109, 0x209, 0x309 };
 static const u16 NCT6775_REG_FAN_MAX_OUTPUT[] = { 0x10a, 0x20a, 0x30a };
 static const u16 NCT6775_REG_FAN_STEP_OUTPUT[] = { 0x10b, 0x20b, 0x30b };
-static const u16 NCT6776_REG_FAN[] = { 0x630, 0x632, 0x634, 0x636, 0x638 };
+static const u16 NCT6775_REG_FAN[] = { 0x630, 0x632, 0x634, 0x636, 0x638 };
 static const u16 NCT6776_REG_FAN_MIN[] = { 0x63a, 0x63c, 0x63e, 0x640, 0x642};
 
 static const u16 NCT6775_REG_TEMP[]
@@ -342,21 +342,36 @@ static inline u8 step_time_to_reg(unsigned int msec, u8 mode)
 						(msec + 200) / 400), 1, 255);
 }
 
-static inline unsigned int
-fan_from_reg(int reg, u16 val, unsigned int div)
+static unsigned int fan_from_reg8(u16 reg, unsigned int divreg)
 {
-	if (val = 0)
+	if (reg = 0 || reg = 255)
 		return 0;
-	if (is_word_sized(reg)) {
-		if ((val & 0xff1f) = 0xff1f)
-			return 0;
-		val = (val & 0x1f) | ((val & 0xff00) >> 3);
-	} else {
-		if (val = 255 || div = 0)
-			return 0;
-		val *= div;
-	}
-	return 1350000U / val;
+	return 1350000U / (reg << divreg);
+}
+
+static unsigned int fan_from_reg13(u16 reg, unsigned int divreg)
+{
+	if ((reg & 0xff1f) = 0xff1f)
+		return 0;
+
+	reg = (reg & 0x1f) | ((reg & 0xff00) >> 3);
+
+	if (reg = 0)
+		return 0;
+
+	return 1350000U / reg;
+}
+
+static unsigned int fan_from_reg16(u16 reg, unsigned int divreg)
+{
+	if (reg = 0 || reg = 0xffff)
+		return 0;
+
+	/*
+	 * Even though the registers are 16 bit wide, the fan divisor
+	 * still applies.
+	 */
+	return 1350000U / (reg << divreg);
 }
 
 static inline unsigned int
@@ -424,6 +439,9 @@ struct w83627ehf_data {
 	const u16 *REG_FAN_MAX_OUTPUT;
 	const u16 *REG_FAN_STEP_OUTPUT;
 
+	unsigned int (*fan_from_reg)(u16 reg, unsigned int divreg);
+	unsigned int (*fan_from_reg_min)(u16 reg, unsigned int divreg);
+
 	struct mutex update_lock;
 	char valid;		/* !=0 if following fields are valid */
 	unsigned long last_updated;	/* In jiffies */
@@ -439,6 +457,7 @@ struct w83627ehf_data {
 	u8 fan_div[5];
 	u8 has_fan;		/* some fan inputs can be disabled */
 	u8 has_fan_min;		/* some fans don't have min register */
+	bool has_fan_div;
 	u8 temp_type[3];
 	s16 temp[9];
 	s16 temp_max[9];
@@ -765,8 +784,8 @@ static struct w83627ehf_data *w83627ehf_update_device(struct device *dev)
 			/* If we failed to measure the fan speed and clock
 			   divider can be increased, let's try that for next
 			   time */
-			if (!is_word_sized(data->REG_FAN[i])
-			    && data->fan[i] = 0xff
+			if (data->has_fan_div
+			    && data->fan[i] >= 0xff
 			    && data->fan_div[i] < 0x07) {
 				dev_dbg(dev, "Increasing fan%d "
 					"clock divider from %u to %u\n",
@@ -960,9 +979,7 @@ show_fan(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;
 	return sprintf(buf, "%d\n",
-		       fan_from_reg(data->REG_FAN[nr],
-				    data->fan[nr],
-				    div_from_reg(data->fan_div[nr])));
+		       data->fan_from_reg(data->fan[nr], data->fan_div[nr]));
 }
 
 static ssize_t
@@ -972,9 +989,8 @@ show_fan_min(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;
 	return sprintf(buf, "%d\n",
-		       fan_from_reg(data->REG_FAN_MIN[nr],
-				    data->fan_min[nr],
-				    div_from_reg(data->fan_div[nr])));
+		       data->fan_from_reg_min(data->fan_min[nr],
+					      data->fan_div[nr]));
 }
 
 static ssize_t
@@ -1004,7 +1020,11 @@ store_fan_min(struct device *dev, struct device_attribute *attr,
 		return err;
 
 	mutex_lock(&data->update_lock);
-	if (is_word_sized(data->REG_FAN_MIN[nr])) {
+	if (!data->has_fan_div) {
+		/*
+		 * Only NCT6776F for now, so we know that this is a 13 bit
+		 * register
+		 */
 		if (!val) {
 			val = 0xff1f;
 		} else {
@@ -1028,7 +1048,7 @@ store_fan_min(struct device *dev, struct device_attribute *attr,
 		new_div = 7; /* 128 = (1 << 7) */
 		dev_warn(dev, "fan%u low limit %lu below minimum %u, set to "
 			 "minimum\n", nr + 1, val,
-			 fan_from_reg(data->REG_FAN_MIN[nr], 254, 128));
+			 data->fan_from_reg_min(254, 7));
 	} else if (!reg) {
 		/* Speed above this value cannot possibly be represented,
 		   even with the lowest divider (1) */
@@ -1036,7 +1056,7 @@ store_fan_min(struct device *dev, struct device_attribute *attr,
 		new_div = 0; /* 1 = (1 << 0) */
 		dev_warn(dev, "fan%u low limit %lu above maximum %u, set to "
 			 "maximum\n", nr + 1, val,
-			 fan_from_reg(data->REG_FAN_MIN[nr], 1, 1));
+			 data->fan_from_reg_min(1, 0));
 	} else {
 		/* Automatically pick the best divider, i.e. the one such
 		   that the min limit will correspond to a register value
@@ -1937,9 +1957,12 @@ static int __devinit w83627ehf_probe(struct platform_device *pdev)
 	}
 
 	if (sio_data->kind = nct6775) {
+		data->has_fan_div = true;
+		data->fan_from_reg = fan_from_reg16;
+		data->fan_from_reg_min = fan_from_reg8;
 		data->REG_PWM = NCT6775_REG_PWM;
 		data->REG_TARGET = NCT6775_REG_TARGET;
-		data->REG_FAN = W83627EHF_REG_FAN;
+		data->REG_FAN = NCT6775_REG_FAN;
 		data->REG_FAN_MIN = W83627EHF_REG_FAN_MIN;
 		data->REG_FAN_START_OUTPUT = NCT6775_REG_FAN_START_OUTPUT;
 		data->REG_FAN_STOP_OUTPUT = NCT6775_REG_FAN_STOP_OUTPUT;
@@ -1947,14 +1970,20 @@ static int __devinit w83627ehf_probe(struct platform_device *pdev)
 		data->REG_FAN_MAX_OUTPUT = NCT6775_REG_FAN_MAX_OUTPUT;
 		data->REG_FAN_STEP_OUTPUT = NCT6775_REG_FAN_STEP_OUTPUT;
 	} else if (sio_data->kind = nct6776) {
+		data->has_fan_div = false;
+		data->fan_from_reg = fan_from_reg13;
+		data->fan_from_reg_min = fan_from_reg13;
 		data->REG_PWM = NCT6775_REG_PWM;
 		data->REG_TARGET = NCT6775_REG_TARGET;
-		data->REG_FAN = NCT6776_REG_FAN;
+		data->REG_FAN = NCT6775_REG_FAN;
 		data->REG_FAN_MIN = NCT6776_REG_FAN_MIN;
 		data->REG_FAN_START_OUTPUT = NCT6775_REG_FAN_START_OUTPUT;
 		data->REG_FAN_STOP_OUTPUT = NCT6775_REG_FAN_STOP_OUTPUT;
 		data->REG_FAN_STOP_TIME = NCT6775_REG_FAN_STOP_TIME;
 	} else if (sio_data->kind = w83667hg_b) {
+		data->has_fan_div = true;
+		data->fan_from_reg = fan_from_reg8;
+		data->fan_from_reg_min = fan_from_reg8;
 		data->REG_PWM = W83627EHF_REG_PWM;
 		data->REG_TARGET = W83627EHF_REG_TARGET;
 		data->REG_FAN = W83627EHF_REG_FAN;
@@ -1967,6 +1996,9 @@ static int __devinit w83627ehf_probe(struct platform_device *pdev)
 		data->REG_FAN_STEP_OUTPUT  		  W83627EHF_REG_FAN_STEP_OUTPUT_W83667_B;
 	} else {
+		data->has_fan_div = true;
+		data->fan_from_reg = fan_from_reg8;
+		data->fan_from_reg_min = fan_from_reg8;
 		data->REG_PWM = W83627EHF_REG_PWM;
 		data->REG_TARGET = W83627EHF_REG_TARGET;
 		data->REG_FAN = W83627EHF_REG_FAN;
-- 
1.7.3.1


_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan
  2011-02-11 17:20 [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan count Guenter Roeck
@ 2011-02-12  3:09 ` Andrew Lutomirski
  2011-02-12  3:45 ` Guenter Roeck
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Andrew Lutomirski @ 2011-02-12  3:09 UTC (permalink / raw)
  To: lm-sensors

On Fri, Feb 11, 2011 at 12:20 PM, Guenter Roeck
<guenter.roeck@ericsson.com> wrote:
> Some of the chips supported by this driver have 13 bit or 16 bit fan count
> registers. This patch improves support for those registers, specifically for
> NCT6775F. With the changes in this patch, fan speed is reported correctly even
> if the fan divider is set to a low value, which results in a fan speed reading
> above 0xff.

I'm not convinced this works.  I have an NCT6775F (or at least the
driver says I do) and, with fan2_min = 0 or 100, I get fan2_div = 128.
 If I set fan2_min\x1000, I get fan2_div = 8 but fan2_input reads 0.
Presumably a low divider should have worked if I really had 16 bits to
play with.

--Andy

>
> With this patch, the width of fan count registers is no longer used to determine
> if the chip has fan divider register(s) or not. A dedicated flag is used instead
> to determine if this is the case.
>
> Signed-off-by: Guenter Roeck <guenter.roeck@ericsson.com>
> ---
>  drivers/hwmon/w83627ehf.c |   86 +++++++++++++++++++++++++++++++--------------
>  1 files changed, 59 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/hwmon/w83627ehf.c b/drivers/hwmon/w83627ehf.c
> index a6616b7..473e052 100644
> --- a/drivers/hwmon/w83627ehf.c
> +++ b/drivers/hwmon/w83627ehf.c
> @@ -234,7 +234,7 @@ static const u16 NCT6775_REG_FAN_STOP_TIME[] = { 0x107, 0x207, 0x307 };
>  static const u16 NCT6775_REG_PWM[] = { 0x109, 0x209, 0x309 };
>  static const u16 NCT6775_REG_FAN_MAX_OUTPUT[] = { 0x10a, 0x20a, 0x30a };
>  static const u16 NCT6775_REG_FAN_STEP_OUTPUT[] = { 0x10b, 0x20b, 0x30b };
> -static const u16 NCT6776_REG_FAN[] = { 0x630, 0x632, 0x634, 0x636, 0x638 };
> +static const u16 NCT6775_REG_FAN[] = { 0x630, 0x632, 0x634, 0x636, 0x638 };
>  static const u16 NCT6776_REG_FAN_MIN[] = { 0x63a, 0x63c, 0x63e, 0x640, 0x642};
>
>  static const u16 NCT6775_REG_TEMP[]
> @@ -342,21 +342,36 @@ static inline u8 step_time_to_reg(unsigned int msec, u8 mode)
>                                                (msec + 200) / 400), 1, 255);
>  }
>
> -static inline unsigned int
> -fan_from_reg(int reg, u16 val, unsigned int div)
> +static unsigned int fan_from_reg8(u16 reg, unsigned int divreg)
>  {
> -       if (val = 0)
> +       if (reg = 0 || reg = 255)
>                return 0;
> -       if (is_word_sized(reg)) {
> -               if ((val & 0xff1f) = 0xff1f)
> -                       return 0;
> -               val = (val & 0x1f) | ((val & 0xff00) >> 3);
> -       } else {
> -               if (val = 255 || div = 0)
> -                       return 0;
> -               val *= div;
> -       }
> -       return 1350000U / val;
> +       return 1350000U / (reg << divreg);
> +}
> +
> +static unsigned int fan_from_reg13(u16 reg, unsigned int divreg)
> +{
> +       if ((reg & 0xff1f) = 0xff1f)
> +               return 0;
> +
> +       reg = (reg & 0x1f) | ((reg & 0xff00) >> 3);
> +
> +       if (reg = 0)
> +               return 0;
> +
> +       return 1350000U / reg;
> +}
> +
> +static unsigned int fan_from_reg16(u16 reg, unsigned int divreg)
> +{
> +       if (reg = 0 || reg = 0xffff)
> +               return 0;
> +
> +       /*
> +        * Even though the registers are 16 bit wide, the fan divisor
> +        * still applies.
> +        */
> +       return 1350000U / (reg << divreg);
>  }
>
>  static inline unsigned int
> @@ -424,6 +439,9 @@ struct w83627ehf_data {
>        const u16 *REG_FAN_MAX_OUTPUT;
>        const u16 *REG_FAN_STEP_OUTPUT;
>
> +       unsigned int (*fan_from_reg)(u16 reg, unsigned int divreg);
> +       unsigned int (*fan_from_reg_min)(u16 reg, unsigned int divreg);
> +
>        struct mutex update_lock;
>        char valid;             /* !=0 if following fields are valid */
>        unsigned long last_updated;     /* In jiffies */
> @@ -439,6 +457,7 @@ struct w83627ehf_data {
>        u8 fan_div[5];
>        u8 has_fan;             /* some fan inputs can be disabled */
>        u8 has_fan_min;         /* some fans don't have min register */
> +       bool has_fan_div;
>        u8 temp_type[3];
>        s16 temp[9];
>        s16 temp_max[9];
> @@ -765,8 +784,8 @@ static struct w83627ehf_data *w83627ehf_update_device(struct device *dev)
>                        /* If we failed to measure the fan speed and clock
>                           divider can be increased, let's try that for next
>                           time */
> -                       if (!is_word_sized(data->REG_FAN[i])
> -                           && data->fan[i] = 0xff
> +                       if (data->has_fan_div
> +                           && data->fan[i] >= 0xff
>                            && data->fan_div[i] < 0x07) {
>                                dev_dbg(dev, "Increasing fan%d "
>                                        "clock divider from %u to %u\n",
> @@ -960,9 +979,7 @@ show_fan(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;
>        return sprintf(buf, "%d\n",
> -                      fan_from_reg(data->REG_FAN[nr],
> -                                   data->fan[nr],
> -                                   div_from_reg(data->fan_div[nr])));
> +                      data->fan_from_reg(data->fan[nr], data->fan_div[nr]));
>  }
>
>  static ssize_t
> @@ -972,9 +989,8 @@ show_fan_min(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;
>        return sprintf(buf, "%d\n",
> -                      fan_from_reg(data->REG_FAN_MIN[nr],
> -                                   data->fan_min[nr],
> -                                   div_from_reg(data->fan_div[nr])));
> +                      data->fan_from_reg_min(data->fan_min[nr],
> +                                             data->fan_div[nr]));
>  }
>
>  static ssize_t
> @@ -1004,7 +1020,11 @@ store_fan_min(struct device *dev, struct device_attribute *attr,
>                return err;
>
>        mutex_lock(&data->update_lock);
> -       if (is_word_sized(data->REG_FAN_MIN[nr])) {
> +       if (!data->has_fan_div) {
> +               /*
> +                * Only NCT6776F for now, so we know that this is a 13 bit
> +                * register
> +                */
>                if (!val) {
>                        val = 0xff1f;
>                } else {
> @@ -1028,7 +1048,7 @@ store_fan_min(struct device *dev, struct device_attribute *attr,
>                new_div = 7; /* 128 = (1 << 7) */
>                dev_warn(dev, "fan%u low limit %lu below minimum %u, set to "
>                         "minimum\n", nr + 1, val,
> -                        fan_from_reg(data->REG_FAN_MIN[nr], 254, 128));
> +                        data->fan_from_reg_min(254, 7));
>        } else if (!reg) {
>                /* Speed above this value cannot possibly be represented,
>                   even with the lowest divider (1) */
> @@ -1036,7 +1056,7 @@ store_fan_min(struct device *dev, struct device_attribute *attr,
>                new_div = 0; /* 1 = (1 << 0) */
>                dev_warn(dev, "fan%u low limit %lu above maximum %u, set to "
>                         "maximum\n", nr + 1, val,
> -                        fan_from_reg(data->REG_FAN_MIN[nr], 1, 1));
> +                        data->fan_from_reg_min(1, 0));
>        } else {
>                /* Automatically pick the best divider, i.e. the one such
>                   that the min limit will correspond to a register value
> @@ -1937,9 +1957,12 @@ static int __devinit w83627ehf_probe(struct platform_device *pdev)
>        }
>
>        if (sio_data->kind = nct6775) {
> +               data->has_fan_div = true;
> +               data->fan_from_reg = fan_from_reg16;
> +               data->fan_from_reg_min = fan_from_reg8;
>                data->REG_PWM = NCT6775_REG_PWM;
>                data->REG_TARGET = NCT6775_REG_TARGET;
> -               data->REG_FAN = W83627EHF_REG_FAN;
> +               data->REG_FAN = NCT6775_REG_FAN;
>                data->REG_FAN_MIN = W83627EHF_REG_FAN_MIN;
>                data->REG_FAN_START_OUTPUT = NCT6775_REG_FAN_START_OUTPUT;
>                data->REG_FAN_STOP_OUTPUT = NCT6775_REG_FAN_STOP_OUTPUT;
> @@ -1947,14 +1970,20 @@ static int __devinit w83627ehf_probe(struct platform_device *pdev)
>                data->REG_FAN_MAX_OUTPUT = NCT6775_REG_FAN_MAX_OUTPUT;
>                data->REG_FAN_STEP_OUTPUT = NCT6775_REG_FAN_STEP_OUTPUT;
>        } else if (sio_data->kind = nct6776) {
> +               data->has_fan_div = false;
> +               data->fan_from_reg = fan_from_reg13;
> +               data->fan_from_reg_min = fan_from_reg13;
>                data->REG_PWM = NCT6775_REG_PWM;
>                data->REG_TARGET = NCT6775_REG_TARGET;
> -               data->REG_FAN = NCT6776_REG_FAN;
> +               data->REG_FAN = NCT6775_REG_FAN;
>                data->REG_FAN_MIN = NCT6776_REG_FAN_MIN;
>                data->REG_FAN_START_OUTPUT = NCT6775_REG_FAN_START_OUTPUT;
>                data->REG_FAN_STOP_OUTPUT = NCT6775_REG_FAN_STOP_OUTPUT;
>                data->REG_FAN_STOP_TIME = NCT6775_REG_FAN_STOP_TIME;
>        } else if (sio_data->kind = w83667hg_b) {
> +               data->has_fan_div = true;
> +               data->fan_from_reg = fan_from_reg8;
> +               data->fan_from_reg_min = fan_from_reg8;
>                data->REG_PWM = W83627EHF_REG_PWM;
>                data->REG_TARGET = W83627EHF_REG_TARGET;
>                data->REG_FAN = W83627EHF_REG_FAN;
> @@ -1967,6 +1996,9 @@ static int __devinit w83627ehf_probe(struct platform_device *pdev)
>                data->REG_FAN_STEP_OUTPUT >                  W83627EHF_REG_FAN_STEP_OUTPUT_W83667_B;
>        } else {
> +               data->has_fan_div = true;
> +               data->fan_from_reg = fan_from_reg8;
> +               data->fan_from_reg_min = fan_from_reg8;
>                data->REG_PWM = W83627EHF_REG_PWM;
>                data->REG_TARGET = W83627EHF_REG_TARGET;
>                data->REG_FAN = W83627EHF_REG_FAN;
> --
> 1.7.3.1
>
>

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan
  2011-02-11 17:20 [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan count Guenter Roeck
  2011-02-12  3:09 ` [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan Andrew Lutomirski
@ 2011-02-12  3:45 ` Guenter Roeck
  2011-02-12  4:08 ` Andrew Lutomirski
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2011-02-12  3:45 UTC (permalink / raw)
  To: lm-sensors

On Fri, Feb 11, 2011 at 10:09:23PM -0500, Andrew Lutomirski wrote:
> On Fri, Feb 11, 2011 at 12:20 PM, Guenter Roeck
> <guenter.roeck@ericsson.com> wrote:
> > Some of the chips supported by this driver have 13 bit or 16 bit fan count
> > registers. This patch improves support for those registers, specifically for
> > NCT6775F. With the changes in this patch, fan speed is reported correctly even
> > if the fan divider is set to a low value, which results in a fan speed reading
> > above 0xff.
> 
> I'm not convinced this works.  I have an NCT6775F (or at least the
> driver says I do) and, with fan2_min = 0 or 100, I get fan2_div = 128.
>  If I set fan2_min\x1000, I get fan2_div = 8 but fan2_input reads 0.
> Presumably a low divider should have worked if I really had 16 bits to
> play with.
> 
Yes, you are right. Can you dump the raw contents of the registers at 0x28..0x2a
as well as 0x630..0x637 ?

According to the datasheet, register 0x28 and 0x631 should match, 0x29 should
match 0x633, 0x2a should match 0x635, and 0x3f should match 0x637.

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] 13+ messages in thread

* Re: [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan
  2011-02-11 17:20 [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan count Guenter Roeck
  2011-02-12  3:09 ` [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan Andrew Lutomirski
  2011-02-12  3:45 ` Guenter Roeck
@ 2011-02-12  4:08 ` Andrew Lutomirski
  2011-02-12  5:08 ` Guenter Roeck
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Andrew Lutomirski @ 2011-02-12  4:08 UTC (permalink / raw)
  To: lm-sensors

On Fri, Feb 11, 2011 at 10:45 PM, Guenter Roeck
<guenter.roeck@ericsson.com> wrote:
> On Fri, Feb 11, 2011 at 10:09:23PM -0500, Andrew Lutomirski wrote:
>> On Fri, Feb 11, 2011 at 12:20 PM, Guenter Roeck
>> <guenter.roeck@ericsson.com> wrote:
>> > Some of the chips supported by this driver have 13 bit or 16 bit fan count
>> > registers. This patch improves support for those registers, specifically for
>> > NCT6775F. With the changes in this patch, fan speed is reported correctly even
>> > if the fan divider is set to a low value, which results in a fan speed reading
>> > above 0xff.
>>
>> I'm not convinced this works.  I have an NCT6775F (or at least the
>> driver says I do) and, with fan2_min = 0 or 100, I get fan2_div = 128.
>>  If I set fan2_min\x1000, I get fan2_div = 8 but fan2_input reads 0.
>> Presumably a low divider should have worked if I really had 16 bits to
>> play with.
>>
> Yes, you are right. Can you dump the raw contents of the registers at 0x28..0x2a
> as well as 0x630..0x637 ?

[13561.120768] w83627ehf: Found NCT6775F chip at 0x290
[13561.121084] Reg 0028 = 45
[13561.121097] Reg 0029 = 4c
[13561.121108] Reg 002a = 00
[13561.121119] Reg 003f = 00
[13561.121147] Reg 0630 = 0045
[13561.121166] Reg 0631 = 4500
[13561.121186] Reg 0632 = 004c
[13561.121205] Reg 0633 = 4c00
[13561.121225] Reg 0634 = 0000
[13561.121244] Reg 0635 = 0000
[13561.121263] Reg 0636 = 0000
[13561.121283] Reg 0637 = 0000

>
> According to the datasheet, register 0x28 and 0x631 should match, 0x29 should
> match 0x633, 0x2a should match 0x635, and 0x3f should match 0x637.

Doesn't look like it.  You might be off by one, or maybe there's an
endianness issue.

FWIW, here's the code I used:

	int regs_to_dump[] = {0x28, 0x29, 0x2a, 0x3f, 0x630, 0x631,
			      0x632, 0x633, 0x634, 0x635, 0x636, 0x637};
	for (i = 0; i < sizeof(regs_to_dump) / sizeof(regs_to_dump[0]); i++)
	{
		unsigned int val;
		val = w83627ehf_read_value(data, regs_to_dump[i]);
		if (is_word_sized(regs_to_dump[i])) {
			printk(KERN_ERR "Reg %04x = %04x\n",
			       regs_to_dump[i], val);
		} else {
			printk(KERN_ERR "Reg %04x = %02x\n",
			       regs_to_dump[i], val);
		}
	}

--Andy

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan
  2011-02-11 17:20 [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan count Guenter Roeck
                   ` (2 preceding siblings ...)
  2011-02-12  4:08 ` Andrew Lutomirski
@ 2011-02-12  5:08 ` Guenter Roeck
  2011-02-12 13:53 ` Andrew Lutomirski
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2011-02-12  5:08 UTC (permalink / raw)
  To: lm-sensors

On Fri, Feb 11, 2011 at 11:08:54PM -0500, Andrew Lutomirski wrote:
> On Fri, Feb 11, 2011 at 10:45 PM, Guenter Roeck
> <guenter.roeck@ericsson.com> wrote:
> > On Fri, Feb 11, 2011 at 10:09:23PM -0500, Andrew Lutomirski wrote:
> >> On Fri, Feb 11, 2011 at 12:20 PM, Guenter Roeck
> >> <guenter.roeck@ericsson.com> wrote:
> >> > Some of the chips supported by this driver have 13 bit or 16 bit fan count
> >> > registers. This patch improves support for those registers, specifically for
> >> > NCT6775F. With the changes in this patch, fan speed is reported correctly even
> >> > if the fan divider is set to a low value, which results in a fan speed reading
> >> > above 0xff.
> >>
> >> I'm not convinced this works.  I have an NCT6775F (or at least the
> >> driver says I do) and, with fan2_min = 0 or 100, I get fan2_div = 128.
> >>  If I set fan2_min\x1000, I get fan2_div = 8 but fan2_input reads 0.
> >> Presumably a low divider should have worked if I really had 16 bits to
> >> play with.
> >>
> > Yes, you are right. Can you dump the raw contents of the registers at 0x28..0x2a
> > as well as 0x630..0x637 ?
> 
> [13561.120768] w83627ehf: Found NCT6775F chip at 0x290
> [13561.121084] Reg 0028 = 45
> [13561.121097] Reg 0029 = 4c
> [13561.121108] Reg 002a = 00
> [13561.121119] Reg 003f = 00
> [13561.121147] Reg 0630 = 0045
> [13561.121166] Reg 0631 = 4500
> [13561.121186] Reg 0632 = 004c
> [13561.121205] Reg 0633 = 4c00
> [13561.121225] Reg 0634 = 0000
> [13561.121244] Reg 0635 = 0000
> [13561.121263] Reg 0636 = 0000
> [13561.121283] Reg 0637 = 0000
> 
> >
> > According to the datasheet, register 0x28 and 0x631 should match, 0x29 should
> > match 0x633, 0x2a should match 0x635, and 0x3f should match 0x637.
> 
> Doesn't look like it.  You might be off by one, or maybe there's an
> endianness issue.
> 
No, it is ok, since the registers are detected as 16 bit registers. Reading from 0x630 
really reads from 0x630 and 0x631, and the result is as expected (ie the read from 0x630
returns the same as the read from 0x28, and both is 0x45).

But then this should translate to a fan1 value of (1350000 / (0x45 * divisor)),
ie be reported as a value somewhere between 19565 if the divisor is 1 to
153 if the divisor is 128. I must be missing something.

Odd is that if I simulate fan speed reading, ie just assign 0x45 to fan[i] when
reading the fan speeds, I do get the expected speeds for the various divisor values.
So I don't really understand what is going on in your system.

Can you possibly dump the raw values of nr, data->fan[nr], data->fan_div[nr], and 
data->fan_from_reg(data->fan[nr], data->fan_div[nr]) in show_fan() ?

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] 13+ messages in thread

* Re: [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan
  2011-02-11 17:20 [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan count Guenter Roeck
                   ` (3 preceding siblings ...)
  2011-02-12  5:08 ` Guenter Roeck
@ 2011-02-12 13:53 ` Andrew Lutomirski
  2011-02-12 15:48 ` Guenter Roeck
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Andrew Lutomirski @ 2011-02-12 13:53 UTC (permalink / raw)
  To: lm-sensors

On Sat, Feb 12, 2011 at 12:08 AM, Guenter Roeck
<guenter.roeck@ericsson.com> wrote:
>>
>> [13561.120768] w83627ehf: Found NCT6775F chip at 0x290
>> [13561.121084] Reg 0028 = 45
>> [13561.121097] Reg 0029 = 4c
>> [13561.121108] Reg 002a = 00
>> [13561.121119] Reg 003f = 00
>> [13561.121147] Reg 0630 = 0045
>> [13561.121166] Reg 0631 = 4500
>> [13561.121186] Reg 0632 = 004c
>> [13561.121205] Reg 0633 = 4c00
>> [13561.121225] Reg 0634 = 0000
>> [13561.121244] Reg 0635 = 0000
>> [13561.121263] Reg 0636 = 0000
>> [13561.121283] Reg 0637 = 0000
>>
>> >
>> > According to the datasheet, register 0x28 and 0x631 should match, 0x29 should
>> > match 0x633, 0x2a should match 0x635, and 0x3f should match 0x637.
>>
>> Doesn't look like it.  You might be off by one, or maybe there's an
>> endianness issue.
>>
> No, it is ok, since the registers are detected as 16 bit registers. Reading from 0x630
> really reads from 0x630 and 0x631, and the result is as expected (ie the read from 0x630
> returns the same as the read from 0x28, and both is 0x45).
>
> But then this should translate to a fan1 value of (1350000 / (0x45 * divisor)),
> ie be reported as a value somewhere between 19565 if the divisor is 1 to
> 153 if the divisor is 128. I must be missing something.
>
> Odd is that if I simulate fan speed reading, ie just assign 0x45 to fan[i] when
> reading the fan speeds, I do get the expected speeds for the various divisor values.
> So I don't really understand what is going on in your system.
>
> Can you possibly dump the raw values of nr, data->fan[nr], data->fan_div[nr], and
> data->fan_from_reg(data->fan[nr], data->fan_div[nr]) in show_fan() ?

With fan2_div = 32 (b/c fan2_min = 502), I get:

[48429.734521] w83627ehf: Found NCT6775F chip at 0x290
[48429.734818] Reg 0028 = 47
[48429.734832] Reg 0029 = 00
[48429.734843] Reg 002a = 00
[48429.734855] Reg 003f = 00
[48429.734884] Reg 0630 = 0047
[48429.734905] Reg 0631 = 4700
[48429.734925] Reg 0632 = 0000
[48429.734945] Reg 0633 = 0000
[48429.734965] Reg 0634 = 0000
[48429.734986] Reg 0635 = 0000
[48429.735006] Reg 0636 = 0000
[48429.735026] Reg 0637 = 0000
[48437.765900] nr=1, fan=0, fan_div=4, fan_from_reg=0  <<< reading fan2_input

With fan2_min = 100 (i.e. fan2_div = 128) I get:

[48525.613069] w83627ehf: Found NCT6775F chip at 0x290
[48525.613686] Reg 0028 = 47
[48525.613698] Reg 0029 = 4c
[48525.613708] Reg 002a = 00
[48525.613719] Reg 003f = 00
[48525.613754] Reg 0630 = 0047
[48525.613772] Reg 0631 = 4700
[48525.613796] Reg 0632 = 004c
[48525.613813] Reg 0633 = 4c00
[48525.613829] Reg 0634 = 0000
[48525.613846] Reg 0635 = 0000
[48525.613863] Reg 0636 = 0000
[48525.613880] Reg 0637 = 0000
[48533.156472] nr=1, fanw, fan_div=7, fan_from_reg\x136

Maybe those 8 high bits just don't work?  Is there some flag to set to
put the chip in 16-bit mode?

--Andy

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan
  2011-02-11 17:20 [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan count Guenter Roeck
                   ` (4 preceding siblings ...)
  2011-02-12 13:53 ` Andrew Lutomirski
@ 2011-02-12 15:48 ` Guenter Roeck
  2011-02-12 16:01 ` Andrew Lutomirski
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2011-02-12 15:48 UTC (permalink / raw)
  To: lm-sensors

On Sat, Feb 12, 2011 at 08:53:51AM -0500, Andrew Lutomirski wrote:
[ ...]
> 
> With fan2_div = 32 (b/c fan2_min = 502), I get:
> 
> [48429.734521] w83627ehf: Found NCT6775F chip at 0x290
> [48429.734818] Reg 0028 = 47
> [48429.734832] Reg 0029 = 00
> [48429.734843] Reg 002a = 00
> [48429.734855] Reg 003f = 00
> [48429.734884] Reg 0630 = 0047
> [48429.734905] Reg 0631 = 4700
> [48429.734925] Reg 0632 = 0000
> [48429.734945] Reg 0633 = 0000
> [48429.734965] Reg 0634 = 0000
> [48429.734986] Reg 0635 = 0000
> [48429.735006] Reg 0636 = 0000
> [48429.735026] Reg 0637 = 0000
> [48437.765900] nr=1, fan=0, fan_div=4, fan_from_reg=0  <<< reading fan2_input
> 
> With fan2_min = 100 (i.e. fan2_div = 128) I get:
> 
> [48525.613069] w83627ehf: Found NCT6775F chip at 0x290
> [48525.613686] Reg 0028 = 47
> [48525.613698] Reg 0029 = 4c
> [48525.613708] Reg 002a = 00
> [48525.613719] Reg 003f = 00
> [48525.613754] Reg 0630 = 0047
> [48525.613772] Reg 0631 = 4700
> [48525.613796] Reg 0632 = 004c
> [48525.613813] Reg 0633 = 4c00
> [48525.613829] Reg 0634 = 0000
> [48525.613846] Reg 0635 = 0000
> [48525.613863] Reg 0636 = 0000
> [48525.613880] Reg 0637 = 0000
> [48533.156472] nr=1, fanw, fan_div=7, fan_from_reg\x136
> 
> Maybe those 8 high bits just don't work?  Is there some flag to set to
> put the chip in 16-bit mode?
> 
16 bit mode is always active.

Register 0x29 and 0x632/0x0633 are in sync, though. What is odd is that 
the value is 0x0 in both registers when it should be 0xff or above.
This is a problem, because if it returns 0x0 in register 0x29 if the divisor
value is too low, we can not detect that it is too low, and the code
to automatically increase the divisor does not work.

Can you check what you get for other fan_min values, specifically for 200 ?
Also, can you check the return values of registers 0x656..0x65d ?
That should be the direct RPM values. Maybe we have to use the RPM registers
for the new chips instead of using the fan count.

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] 13+ messages in thread

* Re: [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan
  2011-02-11 17:20 [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan count Guenter Roeck
                   ` (5 preceding siblings ...)
  2011-02-12 15:48 ` Guenter Roeck
@ 2011-02-12 16:01 ` Andrew Lutomirski
  2011-02-12 17:47 ` Guenter Roeck
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Andrew Lutomirski @ 2011-02-12 16:01 UTC (permalink / raw)
  To: lm-sensors

On Sat, Feb 12, 2011 at 10:48 AM, Guenter Roeck
<guenter.roeck@ericsson.com> wrote:
> On Sat, Feb 12, 2011 at 08:53:51AM -0500, Andrew Lutomirski wrote:
> [ ...]
> 16 bit mode is always active.
>
> Register 0x29 and 0x632/0x0633 are in sync, though. What is odd is that
> the value is 0x0 in both registers when it should be 0xff or above.
> This is a problem, because if it returns 0x0 in register 0x29 if the divisor
> value is too low, we can not detect that it is too low, and the code
> to automatically increase the divisor does not work.
>
> Can you check what you get for other fan_min values, specifically for 200 ?
> Also, can you check the return values of registers 0x656..0x65d ?
> That should be the direct RPM values. Maybe we have to use the RPM registers
> for the new chips instead of using the fan count.

fan2_min = 200 (div = 64):
fan2_input = 138 (read at a different time, so may differ a little)

[56013.556233] Reg 0028 = 46
[56013.556243] Reg 0029 = 99
[56013.556252] Reg 002a = 00
[56013.556261] Reg 003f = 00
[56013.556286] Reg 0630 = 0046
[56013.556302] Reg 0631 = 4600
[56013.556319] Reg 0632 = 0099
[56013.556336] Reg 0633 = 9900
[56013.556352] Reg 0634 = 0000
[56013.556369] Reg 0635 = 0000
[56013.556386] Reg 0636 = 0000
[56013.556403] Reg 0637 = 0000
[56013.556419] Reg 0656 = 025a
[56013.556436] Reg 0657 = 5a00
[56013.556453] Reg 0658 = 0089
[56013.556469] Reg 0659 = 8900
[56013.556486] Reg 065a = 0000
[56013.556502] Reg 065b = 0000
[56013.556519] Reg 065c = 0000
[56013.556536] Reg 065d = 0000


fan2_min = 502 (div = 32):
fan2_input = 0 (read at a different time, so may differ a little)

[56085.471272] w83627ehf: Found NCT6775F chip at 0x290
[56085.471557] Reg 0028 = 45
[56085.471567] Reg 0029 = 00
[56085.471575] Reg 002a = 00
[56085.471584] Reg 003f = 00
[56085.471610] Reg 0630 = 0045
[56085.471627] Reg 0631 = 4500
[56085.471644] Reg 0632 = 0000
[56085.471661] Reg 0633 = 0000
[56085.471677] Reg 0634 = 0000
[56085.471694] Reg 0635 = 0000
[56085.471711] Reg 0636 = 0000
[56085.471728] Reg 0637 = 0000
[56085.471751] Reg 0656 = 0263
[56085.471780] Reg 0657 = 6300
[56085.471804] Reg 0658 = 0000
[56085.471829] Reg 0659 = 0000
[56085.471846] Reg 065a = 0000
[56085.471864] Reg 065b = 0000
[56085.471881] Reg 065c = 0000
[56085.471898] Reg 065d = 0000

I assume it's 0x658 that should hold the RPM value.

If you haven't solved it remotely by Monday, I'll email Nuvoton and
ask for that datasheet...

--Andy

>
> 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] 13+ messages in thread

* Re: [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan
  2011-02-11 17:20 [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan count Guenter Roeck
                   ` (6 preceding siblings ...)
  2011-02-12 16:01 ` Andrew Lutomirski
@ 2011-02-12 17:47 ` Guenter Roeck
  2011-02-12 17:55 ` Guenter Roeck
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2011-02-12 17:47 UTC (permalink / raw)
  To: lm-sensors

On Sat, Feb 12, 2011 at 11:01:11AM -0500, Andrew Lutomirski wrote:
[ ... ]
> 
> fan2_min = 200 (div = 64):
> fan2_input = 138 (read at a different time, so may differ a little)
> 
> [56013.556233] Reg 0028 = 46
> [56013.556243] Reg 0029 = 99
> [56013.556252] Reg 002a = 00
> [56013.556261] Reg 003f = 00
> [56013.556286] Reg 0630 = 0046
> [56013.556302] Reg 0631 = 4600
> [56013.556319] Reg 0632 = 0099
> [56013.556336] Reg 0633 = 9900
> [56013.556352] Reg 0634 = 0000
> [56013.556369] Reg 0635 = 0000
> [56013.556386] Reg 0636 = 0000
> [56013.556403] Reg 0637 = 0000
> [56013.556419] Reg 0656 = 025a
> [56013.556436] Reg 0657 = 5a00
> [56013.556453] Reg 0658 = 0089
> [56013.556469] Reg 0659 = 8900
> [56013.556486] Reg 065a = 0000
> [56013.556502] Reg 065b = 0000
> [56013.556519] Reg 065c = 0000
> [56013.556536] Reg 065d = 0000
> 
> 
> fan2_min = 502 (div = 32):
> fan2_input = 0 (read at a different time, so may differ a little)
> 
> [56085.471272] w83627ehf: Found NCT6775F chip at 0x290
> [56085.471557] Reg 0028 = 45
> [56085.471567] Reg 0029 = 00
> [56085.471575] Reg 002a = 00
> [56085.471584] Reg 003f = 00
> [56085.471610] Reg 0630 = 0045
> [56085.471627] Reg 0631 = 4500
> [56085.471644] Reg 0632 = 0000
> [56085.471661] Reg 0633 = 0000
> [56085.471677] Reg 0634 = 0000
> [56085.471694] Reg 0635 = 0000
> [56085.471711] Reg 0636 = 0000
> [56085.471728] Reg 0637 = 0000
> [56085.471751] Reg 0656 = 0263
> [56085.471780] Reg 0657 = 6300
> [56085.471804] Reg 0658 = 0000
> [56085.471829] Reg 0659 = 0000
> [56085.471846] Reg 065a = 0000
> [56085.471864] Reg 065b = 0000
> [56085.471881] Reg 065c = 0000
> [56085.471898] Reg 065d = 0000
> 
> I assume it's 0x658 that should hold the RPM value.
> 
Yes, for fan2. 0x656 would be fan1. Looks like that works.

> If you haven't solved it remotely by Monday, I'll email Nuvoton and
> ask for that datasheet...
> 
Might be a good idea anyway.

My take is that we might have to use the rpm registers directly. I'll prepare 
a patch for that. I'll also ask a contact at Nuvoton if they have an idea 
what is going on.

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] 13+ messages in thread

* Re: [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan
  2011-02-11 17:20 [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan count Guenter Roeck
                   ` (7 preceding siblings ...)
  2011-02-12 17:47 ` Guenter Roeck
@ 2011-02-12 17:55 ` Guenter Roeck
  2011-02-12 18:56 ` Andrew Lutomirski
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2011-02-12 17:55 UTC (permalink / raw)
  To: lm-sensors

On Sat, Feb 12, 2011 at 11:01:11AM -0500, Andrew Lutomirski wrote:
[ ... ]

Actually,

> fan2_min = 200 (div = 64):
> fan2_input = 138 (read at a different time, so may differ a little)
> 
> [56013.556243] Reg 0029 = 99
> [56013.556286] Reg 0630 = 0046
> [56013.556453] Reg 0658 = 0089
> 
> fan2_min = 502 (div = 32):
> fan2_input = 0 (read at a different time, so may differ a little)
> 
> [56085.471567] Reg 0029 = 00
> [56085.471644] Reg 0632 = 0000
> [56085.471804] Reg 0658 = 0000

Must have been blind. So the RPM register returns 0 as well. Not good.
Wonder what is going on.

Guenter

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan
  2011-02-11 17:20 [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan count Guenter Roeck
                   ` (8 preceding siblings ...)
  2011-02-12 17:55 ` Guenter Roeck
@ 2011-02-12 18:56 ` Andrew Lutomirski
  2011-02-12 21:26 ` Guenter Roeck
  2011-02-14  6:03 ` Guenter Roeck
  11 siblings, 0 replies; 13+ messages in thread
From: Andrew Lutomirski @ 2011-02-12 18:56 UTC (permalink / raw)
  To: lm-sensors

On Sat, Feb 12, 2011 at 12:55 PM, Guenter Roeck
<guenter.roeck@ericsson.com> wrote:
> On Sat, Feb 12, 2011 at 11:01:11AM -0500, Andrew Lutomirski wrote:
> [ ... ]
>
> Actually,
>
>> fan2_min = 200 (div = 64):
>> fan2_input = 138 (read at a different time, so may differ a little)
>>
>> [56013.556243] Reg 0029 = 99
>> [56013.556286] Reg 0630 = 0046
>> [56013.556453] Reg 0658 = 0089
>>
>> fan2_min = 502 (div = 32):
>> fan2_input = 0 (read at a different time, so may differ a little)
>>
>> [56085.471567] Reg 0029 = 00
>> [56085.471644] Reg 0632 = 0000
>> [56085.471804] Reg 0658 = 0000
>
> Must have been blind. So the RPM register returns 0 as well. Not good.
> Wonder what is going on.

So far, the highest number I've been able to get out of the counter
register is 0x2e8 by setting div = 16 and playing with the pwm.

If I set div = 8, then I can't get much above 0x10b.

I have no idea what's going on either.

I tried writing 0x80 to 0x508 (which is supposed to turn the whole
thing on), but the only bits that seem to stick are 0x7, so I have no
idea what's going on.

--Andy

>
> Guenter
>

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan
  2011-02-11 17:20 [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan count Guenter Roeck
                   ` (9 preceding siblings ...)
  2011-02-12 18:56 ` Andrew Lutomirski
@ 2011-02-12 21:26 ` Guenter Roeck
  2011-02-14  6:03 ` Guenter Roeck
  11 siblings, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2011-02-12 21:26 UTC (permalink / raw)
  To: lm-sensors

On Sat, Feb 12, 2011 at 01:56:09PM -0500, Andrew Lutomirski wrote:
> On Sat, Feb 12, 2011 at 12:55 PM, Guenter Roeck
> <guenter.roeck@ericsson.com> wrote:
> > On Sat, Feb 12, 2011 at 11:01:11AM -0500, Andrew Lutomirski wrote:
> > [ ... ]
> >
> > Actually,
> >
> >> fan2_min = 200 (div = 64):
> >> fan2_input = 138 (read at a different time, so may differ a little)
> >>
> >> [56013.556243] Reg 0029 = 99
> >> [56013.556286] Reg 0630 = 0046
> >> [56013.556453] Reg 0658 = 0089
> >>
> >> fan2_min = 502 (div = 32):
> >> fan2_input = 0 (read at a different time, so may differ a little)
> >>
> >> [56085.471567] Reg 0029 = 00
> >> [56085.471644] Reg 0632 = 0000
> >> [56085.471804] Reg 0658 = 0000
> >
> > Must have been blind. So the RPM register returns 0 as well. Not good.
> > Wonder what is going on.
> 
> So far, the highest number I've been able to get out of the counter
> register is 0x2e8 by setting div = 16 and playing with the pwm.
> 
> If I set div = 8, then I can't get much above 0x10b.
> 
> I have no idea what's going on either.
> 
Do you always get an output with div=8, or was that at a higher fan speed ?

Also, does it ever report 0xff in the 8-bit register, or does it always 
switch to 0x00 ?

> I tried writing 0x80 to 0x508 (which is supposed to turn the whole
> thing on), but the only bits that seem to stick are 0x7, so I have no
> idea what's going on.
> 
If you set 0x508 to 0, you might see 0xff in the 8-bit register; at least
this is what I see. The register seems to be write-only.


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] 13+ messages in thread

* Re: [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan
  2011-02-11 17:20 [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan count Guenter Roeck
                   ` (10 preceding siblings ...)
  2011-02-12 21:26 ` Guenter Roeck
@ 2011-02-14  6:03 ` Guenter Roeck
  11 siblings, 0 replies; 13+ messages in thread
From: Guenter Roeck @ 2011-02-14  6:03 UTC (permalink / raw)
  To: lm-sensors

On Sat, Feb 12, 2011 at 01:56:09PM -0500, Andrew Lutomirski wrote:
> On Sat, Feb 12, 2011 at 12:55 PM, Guenter Roeck
> <guenter.roeck@ericsson.com> wrote:
> > On Sat, Feb 12, 2011 at 11:01:11AM -0500, Andrew Lutomirski wrote:
> > [ ... ]
> >
> > Actually,
> >
> >> fan2_min = 200 (div = 64):
> >> fan2_input = 138 (read at a different time, so may differ a little)
> >>
> >> [56013.556243] Reg 0029 = 99
> >> [56013.556286] Reg 0630 = 0046
> >> [56013.556453] Reg 0658 = 0089
> >>
> >> fan2_min = 502 (div = 32):
> >> fan2_input = 0 (read at a different time, so may differ a little)
> >>
> >> [56085.471567] Reg 0029 = 00
> >> [56085.471644] Reg 0632 = 0000
> >> [56085.471804] Reg 0658 = 0000
> >
> > Must have been blind. So the RPM register returns 0 as well. Not good.
> > Wonder what is going on.
> 
> So far, the highest number I've been able to get out of the counter
> register is 0x2e8 by setting div = 16 and playing with the pwm.
> 
> If I set div = 8, then I can't get much above 0x10b.
> 
> I have no idea what's going on either.
> 
> I tried writing 0x80 to 0x508 (which is supposed to turn the whole
> thing on), but the only bits that seem to stick are 0x7, so I have no
> idea what's going on.
> 
Here is what I got from Nuvoton.

The chip has an internal "abort time" when waiting for the next pulse from the fan.
If this abort time expires and no pulse was received, all fan count and rpm registers
will return 0. This can happen if 1) the fan speed is very low and/or 2) if
the divisor setting is too low.

This means we can not depend on a reading of 0xff to determine if the divisor
is too low. A reading of 0x0 (for 8 bit and 16 bit fan count registers as well
as RPM) can also indicate that the divisor was too low.

I'll have to think about how to proceed. Best approach might be to increase the divisor
if the fan count returns 0. If anyone has a better idea, please let me know.

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] 13+ messages in thread

end of thread, other threads:[~2011-02-14  6:03 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-02-11 17:20 [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan count Guenter Roeck
2011-02-12  3:09 ` [lm-sensors] [PATCH v3 7/7] hwmon: (w83627ehf) Use 16 bit fan Andrew Lutomirski
2011-02-12  3:45 ` Guenter Roeck
2011-02-12  4:08 ` Andrew Lutomirski
2011-02-12  5:08 ` Guenter Roeck
2011-02-12 13:53 ` Andrew Lutomirski
2011-02-12 15:48 ` Guenter Roeck
2011-02-12 16:01 ` Andrew Lutomirski
2011-02-12 17:47 ` Guenter Roeck
2011-02-12 17:55 ` Guenter Roeck
2011-02-12 18:56 ` Andrew Lutomirski
2011-02-12 21:26 ` Guenter Roeck
2011-02-14  6:03 ` Guenter Roeck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.