All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors] hwmon/w83627hf pwm_freq support
@ 2007-05-20 13:52 Carlos Olalla Martínez
  2007-05-21 11:57 ` Jean Delvare
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Carlos Olalla Martínez @ 2007-05-20 13:52 UTC (permalink / raw)
  To: lm-sensors

Couldnt test it, but w83697hf was OK in 2.6.21.1; any tester with a w83627hf?

Signed-off-by: Carlos Olalla <com.ea@tinet.org>
diff -uprN -X linux-2.6.22-rc1/Documentation/dontdiff
linux-2.6.22-rc1.orig/drivers/hwmon/w83627hf.c
linux-2.6.22-rc1/drivers/hwmon/w83627hf.c
--- linux-2.6.22-rc1.orig/drivers/hwmon/w83627hf.c	2007-05-13
03:45:56.000000000 +0200
+++ linux-2.6.22-rc1/drivers/hwmon/w83627hf.c	2007-05-19
18:31:55.000000000 +0200
@@ -25,12 +25,12 @@
 /*
     Supports following chips:

-    Chip	#vin	#fanin	#pwm	#temp	wchipid	vendid	i2c	ISA
-    w83627hf	9	3	2	3	0x20	0x5ca3	no	yes(LPC)
-    w83627thf	7	3	3	3	0x90	0x5ca3	no	yes(LPC)
-    w83637hf	7	3	3	3	0x80	0x5ca3	no	yes(LPC)
-    w83687thf	7	3	3	3	0x90	0x5ca3	no	yes(LPC)
-    w83697hf	8	2	2	2	0x60	0x5ca3	no	yes(LPC)
+    Chip	#vin	#fanin	#pwm	#pwm_freq	#temp	wchipid	vendid	i2c	ISA
+    w83627hf	9	3	2	2		3	0x20	0x5ca3	no	yes(LPC)
+    w83627thf	7	3	3	X		3	0x90	0x5ca3	no	yes(LPC)
+    w83637hf	7	3	3	3		3	0x80	0x5ca3	no	yes(LPC)
+    w83687thf	7	3	3	3		3	0x90	0x5ca3	no	yes(LPC)
+    w83697hf	8	2	2	2		2	0x60	0x5ca3	no	yes(LPC)

     For other winbond chips, and for i2c support in the above chips,
     use w83781d.c.
@@ -220,6 +220,25 @@ static const u8 regpwm[] = { W83627THF_R
 #define W836X7HF_REG_PWM(type, nr) (((type) = w83627hf) ? \
                                      regpwm_627hf[(nr) - 1] : regpwm[(nr)
- 1])

+#define W83627HF_REG_PWM_FREQ1		0x5C	/* Only for the 627HF */
+#define W83627HF_REG_PWM_FREQ2		0x5C	/* Only for the 627HF */
+
+#define W83637HF_REG_PWM_FREQ1		0x00	/* 697HF/687THF too */
+#define W83637HF_REG_PWM_FREQ2		0x02	/* 697HF/687THF too */
+#define W83637HF_REG_PWM_FREQ3		0x10	/* 687THF too */
+
+/* 687thf uses DC output by default and 627thf only uses DC output.
+ PWMCLK is useless for DC output */
+static const u8 regpwmfreq_627hf[] = { W83627HF_REG_PWM_FREQ1,
W83627HF_REG_PWM_FREQ2 };
+static const u8 regpwmfreq[] = { W83637HF_REG_PWM_FREQ1,
W83637HF_REG_PWM_FREQ2,
+				 W83637HF_REG_PWM_FREQ3 };
+#define W836X7HF_REG_PWM_FREQ(type, nr) (((type) = w83627hf) ? \
+                                     regpwmfreq_627hf[(nr) - 1] :
regpwmfreq[(nr) - 1])
+
+/* From manual 627hf -> 46870 Hz is the base,
+	we take 46880 to avoid float results with divisors. */
+#define W83627HF_DEFAULT_PWM_FREQ	46880
+
 #define W83781D_REG_I2C_ADDR 0x48
 #define W83781D_REG_I2C_SUBADDR 0x4A

@@ -267,6 +286,49 @@ static int TEMP_FROM_REG(u8 reg)

 #define PWM_TO_REG(val) (SENSORS_LIMIT((val),0,255))

+static inline unsigned long pwm_freq_from_reg_627hf(u8 reg, int nr)
+{
+	unsigned long freq;
+	/* bits 6..4 pwmclk2 (nr=2) .. bits 2..0 pwmclk1 (nr=1) */
+	reg = (reg >> ((nr-1)*4)) & 0x07;
+	freq = W83627HF_DEFAULT_PWM_FREQ/(2^reg);
+	return freq;
+}
+static inline u8 pwm_freq_to_reg_627hf(unsigned long val, int nr)
+{
+	u8 i;
+	/* Only 5 dividers (1 2 4 8 16)... Search for the nearest available
frequency */
+	for (i = 0; i < 5; i++) {
+		if (val > (W83627HF_DEFAULT_PWM_FREQ/(2^i) +
W83627HF_DEFAULT_PWM_FREQ/(2^(i+1))) / 2)
+			break;
+	}
+	return (i << ((nr-1)*4));
+}
+
+static inline unsigned long pwm_freq_from_reg(u8 reg)
+{
+	/* Clock bit 8 -> 180 kHz or 24 MHz */
+	unsigned long clock = (reg & 0x80) ? 180000UL : 24000000UL;
+
+	reg &= 0x7f;
+	/* This should not happen but anyway... */
+	if (reg = 0)
+		reg++;
+	return clock / ( reg << 8 );
+}
+static inline u8 pwm_freq_to_reg(unsigned long val)
+{
+	/* Minimum divider value is 0x01 and maximum is 0x7F */
+	if (val >= 93750)	/* The highest we can do */
+		return 0x01;
+	if (val >= 738)	/* Use 24 MHz clock */
+		return (24000000UL / (val << 8));
+	if (val < 6)		/* The lowest we can do */
+		return 0xFF;
+	else			/* Use 180 kHz clock */
+		return 0x80 | (180000UL / (val << 8));
+}
+
 #define BEEP_MASK_FROM_REG(val)		 (val)
 #define BEEP_MASK_TO_REG(val)		((val) & 0xffffff)
 #define BEEP_ENABLE_TO_REG(val)		((val)?1:0)
@@ -316,6 +378,7 @@ struct w83627hf_data {
 	u32 beep_mask;		/* Register encoding, combined */
 	u8 beep_enable;		/* Boolean */
 	u8 pwm[3];		/* Register value */
+	u8 pwm_freq[3];		/* Register value */
 	u16 sens[3];		/* 782D/783S only.
 				   1 = pentium diode; 2 = 3904 diode;
 				   3000-5000 = thermistor beta.
@@ -852,6 +915,61 @@ sysfs_pwm(2);
 sysfs_pwm(3);

 static ssize_t
+show_pwm_freq_reg(struct device *dev, char *buf, int nr)
+{
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+//	return sprintf(buf, "%ld\n", (long) data->pwm_freq[nr - 1]);
+	if (data->type = w83627hf)
+		return sprintf(buf, "%ld\n",
pwm_freq_from_reg_627hf(data->pwm_freq[nr-1],nr));
+	else
+		return sprintf(buf, "%ld\n", pwm_freq_from_reg(data->pwm_freq[nr-1]));
+}
+
+static ssize_t
+store_pwm_freq_reg(struct device *dev, const char *buf, size_t count, int
nr)
+{
+	struct w83627hf_data *data = dev_get_drvdata(dev);
+	u32 val;
+	u8 mask[]={0xF8, 0x8F};
+
+	val = simple_strtoul(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
+
+	if (data->type = w83627hf) {
+		data->pwm_freq[nr - 1] = pwm_freq_to_reg_627hf(val,nr);
+		w83627hf_write_value( data, regpwmfreq_627hf[nr - 1],
+					data->pwm_freq[nr - 1] |
+					(w83627hf_read_value(data,
+					regpwmfreq_627hf[nr - 1]) & mask[nr - 1]) );
+	} else if ( (data->type = w83637hf) || (data->type = w83697hf) ||
(data->type = w83687thf) ) {
+		data->pwm_freq[nr - 1] = pwm_freq_to_reg(val);
+		w83627hf_write_value(data, regpwmfreq[nr - 1],
+					data->pwm_freq[nr - 1]);
+	}
+
+	mutex_unlock(&data->update_lock);
+	return count;
+}
+
+#define sysfs_pwm_freq(offset) \
+static ssize_t show_regs_pwm_freq_##offset (struct device *dev, struct
device_attribute *attr, char *buf) \
+{ \
+	return show_pwm_freq_reg(dev, buf, offset); \
+} \
+static ssize_t \
+store_regs_pwm_freq_##offset (struct device *dev, struct device_attribute
*attr, const char *buf, size_t count) \
+{ \
+	return store_pwm_freq_reg(dev, buf, count, offset); \
+} \
+static DEVICE_ATTR(pwm##offset##_freq, S_IRUGO | S_IWUSR, \
+		  show_regs_pwm_freq_##offset, store_regs_pwm_freq_##offset);
+
+sysfs_pwm_freq(1);
+sysfs_pwm_freq(2);
+sysfs_pwm_freq(3);
+
+static ssize_t
 show_sensor_reg(struct device *dev, char *buf, int nr)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
@@ -1044,6 +1162,8 @@ static struct attribute *w83627hf_attrib

 	&dev_attr_pwm1.attr,
 	&dev_attr_pwm2.attr,
+	&dev_attr_pwm1_freq.attr,
+	&dev_attr_pwm2_freq.attr,

 	&dev_attr_name.attr,
 	NULL
@@ -1074,6 +1194,7 @@ static struct attribute *w83627hf_attrib
 	&dev_attr_temp3_type.attr,

 	&dev_attr_pwm3.attr,
+	&dev_attr_pwm3_freq.attr,

 	NULL
 };
@@ -1167,6 +1288,10 @@ static int __devinit w83627hf_probe(stru
 		if ((err = device_create_file(dev, &dev_attr_pwm3)))
 			goto ERROR4;

+	if (data->type = w83637hf || data->type = w83687thf)
+		if ((err = device_create_file(dev, &dev_attr_pwm3_freq)))
+			goto ERROR4;
+
 	data->class_dev = hwmon_device_register(dev);
 	if (IS_ERR(data->class_dev)) {
 		err = PTR_ERR(data->class_dev);
@@ -1470,6 +1595,14 @@ static struct w83627hf_data *w83627hf_up
 			   (data->type = w83627hf || data->type = w83697hf))
 				break;
 		}
+		for (i = 1; i <= 3; i++) {
+			u8 tmp = w83627hf_read_value(data,
+				W836X7HF_REG_PWM_FREQ(data->type, i));
+			data->pwm_freq[i - 1] = tmp;
+			if(i = 2 &&
+			   (data->type != w83637hf || data->type != w83687thf))
+				break;
+		}

 		data->temp = w83627hf_read_value(data, W83781D_REG_TEMP(1));
 		data->temp_max 
--

Carlos


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

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

* Re: [lm-sensors] hwmon/w83627hf pwm_freq support
  2007-05-20 13:52 [lm-sensors] hwmon/w83627hf pwm_freq support Carlos Olalla Martínez
@ 2007-05-21 11:57 ` Jean Delvare
  2007-05-26 12:55 ` Carlos Olalla Martínez
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jean Delvare @ 2007-05-21 11:57 UTC (permalink / raw)
  To: lm-sensors

Hi Carlos,

On Sun, 20 May 2007 15:52:21 +0200 (CEST), Carlos Olalla Martínez wrote:
> Couldnt test it, but w83697hf was OK in 2.6.21.1; any tester with a w83627hf?
> 
> Signed-off-by: Carlos Olalla <com.ea@tinet.org>
> diff -uprN -X linux-2.6.22-rc1/Documentation/dontdiff
> linux-2.6.22-rc1.orig/drivers/hwmon/w83627hf.c
> linux-2.6.22-rc1/drivers/hwmon/w83627hf.c
> --- linux-2.6.22-rc1.orig/drivers/hwmon/w83627hf.c	2007-05-13
> 03:45:56.000000000 +0200
> +++ linux-2.6.22-rc1/drivers/hwmon/w83627hf.c	2007-05-19
> 18:31:55.000000000 +0200

You mailer wrapped long lines -> I can't apply your patch. Please
resend and make sure the patch will remain untouched so that I can
apply it.

> @@ -25,12 +25,12 @@
>  /*
>      Supports following chips:
> 
> -    Chip	#vin	#fanin	#pwm	#temp	wchipid	vendid	i2c	ISA
> -    w83627hf	9	3	2	3	0x20	0x5ca3	no	yes(LPC)
> -    w83627thf	7	3	3	3	0x90	0x5ca3	no	yes(LPC)
> -    w83637hf	7	3	3	3	0x80	0x5ca3	no	yes(LPC)
> -    w83687thf	7	3	3	3	0x90	0x5ca3	no	yes(LPC)
> -    w83697hf	8	2	2	2	0x60	0x5ca3	no	yes(LPC)
> +    Chip	#vin	#fanin	#pwm	#pwm_freq	#temp	wchipid	vendid	i2c	ISA
> +    w83627hf	9	3	2	2		3	0x20	0x5ca3	no	yes(LPC)
> +    w83627thf	7	3	3	X		3	0x90	0x5ca3	no	yes(LPC)
> +    w83637hf	7	3	3	3		3	0x80	0x5ca3	no	yes(LPC)
> +    w83687thf	7	3	3	3		3	0x90	0x5ca3	no	yes(LPC)
> +    w83697hf	8	2	2	2		2	0x60	0x5ca3	no	yes(LPC)

Huh, please don't touch this table. We don't list every possible
feature here, only the core properties.

> 
>      For other winbond chips, and for i2c support in the above chips,
>      use w83781d.c.
> @@ -220,6 +220,25 @@ static const u8 regpwm[] = { W83627THF_R
>  #define W836X7HF_REG_PWM(type, nr) (((type) = w83627hf) ? \
>                                       regpwm_627hf[(nr) - 1] : regpwm[(nr)
> - 1])
> 
> +#define W83627HF_REG_PWM_FREQ1		0x5C	/* Only for the 627HF */
> +#define W83627HF_REG_PWM_FREQ2		0x5C	/* Only for the 627HF */
> +
> +#define W83637HF_REG_PWM_FREQ1		0x00	/* 697HF/687THF too */
> +#define W83637HF_REG_PWM_FREQ2		0x02	/* 697HF/687THF too */
> +#define W83637HF_REG_PWM_FREQ3		0x10	/* 687THF too */
> +
> +/* 687thf uses DC output by default and 627thf only uses DC output.
> + PWMCLK is useless for DC output */
> +static const u8 regpwmfreq_627hf[] = { W83627HF_REG_PWM_FREQ1,
> W83627HF_REG_PWM_FREQ2 };
> +static const u8 regpwmfreq[] = { W83637HF_REG_PWM_FREQ1,
> W83637HF_REG_PWM_FREQ2,
> +				 W83637HF_REG_PWM_FREQ3 };
> +#define W836X7HF_REG_PWM_FREQ(type, nr) (((type) = w83627hf) ? \
> +                                     regpwmfreq_627hf[(nr) - 1] :
> regpwmfreq[(nr) - 1])

You're trying to put together things which are too different. The
W83627HF handles the PWM frequencies in a completely different way, so
there is no point in having a single macro to handle all the chips.
This makes things more complex with no benefit. In fact your current
code even ends up reading the same register twice for the W83627HF -
that's inefficient.

So I suggest that you simply define:

#define W83627HF_REG_PWM_FREQ		0x5C			/* Only for the 627HF */
static const u8 W83637HF_REG_PWM_FREQ = { 0x00, 0x02, 0x10 };	/* 697HF/687THF too */

And use these directly in the code. This will be much more simple that
way.

> +
> +/* From manual 627hf -> 46870 Hz is the base,
> +	we take 46880 to avoid float results with divisors. */
> +#define W83627HF_DEFAULT_PWM_FREQ	46880

There are no floats in the kernel. Integer division will strip the
non-integer part for you. So just use 46870.

And the name is badly chosen, as the datasheet says that the default is
23.43 kHz. 46.87 kHz is more like the base PWM clock, or maximum
frequency.

> +
>  #define W83781D_REG_I2C_ADDR 0x48
>  #define W83781D_REG_I2C_SUBADDR 0x4A
> 
> @@ -267,6 +286,49 @@ static int TEMP_FROM_REG(u8 reg)
> 
>  #define PWM_TO_REG(val) (SENSORS_LIMIT((val),0,255))
> 
> +static inline unsigned long pwm_freq_from_reg_627hf(u8 reg, int nr)
> +{
> +	unsigned long freq;
> +	/* bits 6..4 pwmclk2 (nr=2) .. bits 2..0 pwmclk1 (nr=1) */
> +	reg = (reg >> ((nr-1)*4)) & 0x07;
> +	freq = W83627HF_DEFAULT_PWM_FREQ/(2^reg);

Eek! 2^reg is NOT "2 at the power reg". ^ is the bitwise XOR operator.

This should be instead written:

	freq = W83627HF_DEFAULT_PWM_FREQ >> reg;

> +	return freq;
> +}
> +static inline u8 pwm_freq_to_reg_627hf(unsigned long val, int nr)
> +{
> +	u8 i;
> +	/* Only 5 dividers (1 2 4 8 16)... Search for the nearest available
> frequency */
> +	for (i = 0; i < 5; i++) {
> +		if (val > (W83627HF_DEFAULT_PWM_FREQ/(2^i) +
> W83627HF_DEFAULT_PWM_FREQ/(2^(i+1))) / 2)

Again this isn't correct and should be written >> i and >> (i + 1).

> +			break;
> +	}
> +	return (i << ((nr-1)*4));
> +}
> +
> +static inline unsigned long pwm_freq_from_reg(u8 reg)
> +{
> +	/* Clock bit 8 -> 180 kHz or 24 MHz */
> +	unsigned long clock = (reg & 0x80) ? 180000UL : 24000000UL;
> +
> +	reg &= 0x7f;
> +	/* This should not happen but anyway... */
> +	if (reg = 0)
> +		reg++;
> +	return clock / ( reg << 8 );
> +}
> +static inline u8 pwm_freq_to_reg(unsigned long val)
> +{
> +	/* Minimum divider value is 0x01 and maximum is 0x7F */
> +	if (val >= 93750)	/* The highest we can do */
> +		return 0x01;
> +	if (val >= 738)	/* Use 24 MHz clock */

Should be > 720, so that values between 703 (highest with 180 kHz
clock) and 738 (lowest with 24 MHz clock) are redirected to the nearest
possible value.

> +		return (24000000UL / (val << 8));
> +	if (val < 6)		/* The lowest we can do */
> +		return 0xFF;
> +	else			/* Use 180 kHz clock */
> +		return 0x80 | (180000UL / (val << 8));
> +}
> +
>  #define BEEP_MASK_FROM_REG(val)		 (val)
>  #define BEEP_MASK_TO_REG(val)		((val) & 0xffffff)
>  #define BEEP_ENABLE_TO_REG(val)		((val)?1:0)
> @@ -316,6 +378,7 @@ struct w83627hf_data {
>  	u32 beep_mask;		/* Register encoding, combined */
>  	u8 beep_enable;		/* Boolean */
>  	u8 pwm[3];		/* Register value */
> +	u8 pwm_freq[3];		/* Register value */
>  	u16 sens[3];		/* 782D/783S only.
>  				   1 = pentium diode; 2 = 3904 diode;
>  				   3000-5000 = thermistor beta.
> @@ -852,6 +915,61 @@ sysfs_pwm(2);
>  sysfs_pwm(3);
> 
>  static ssize_t
> +show_pwm_freq_reg(struct device *dev, char *buf, int nr)
> +{
> +	struct w83627hf_data *data = w83627hf_update_device(dev);
> +//	return sprintf(buf, "%ld\n", (long) data->pwm_freq[nr - 1]);

Please clean up your code before sending patches.

> +	if (data->type = w83627hf)
> +		return sprintf(buf, "%ld\n",
> pwm_freq_from_reg_627hf(data->pwm_freq[nr-1],nr));
> +	else
> +		return sprintf(buf, "%ld\n", pwm_freq_from_reg(data->pwm_freq[nr-1]));
> +}
> +
> +static ssize_t
> +store_pwm_freq_reg(struct device *dev, const char *buf, size_t count, int
> nr)
> +{
> +	struct w83627hf_data *data = dev_get_drvdata(dev);
> +	u32 val;
> +	u8 mask[]={0xF8, 0x8F};

static const.

> +
> +	val = simple_strtoul(buf, NULL, 10);
> +
> +	mutex_lock(&data->update_lock);
> +
> +	if (data->type = w83627hf) {
> +		data->pwm_freq[nr - 1] = pwm_freq_to_reg_627hf(val,nr);
> +		w83627hf_write_value( data, regpwmfreq_627hf[nr - 1],
> +					data->pwm_freq[nr - 1] |
> +					(w83627hf_read_value(data,
> +					regpwmfreq_627hf[nr - 1]) & mask[nr - 1]) );

Coding style: no space inside parentheses.

> +	} else if ( (data->type = w83637hf) || (data->type = w83697hf) ||
> (data->type = w83687thf) ) {

This could be a simple "else".

> +		data->pwm_freq[nr - 1] = pwm_freq_to_reg(val);
> +		w83627hf_write_value(data, regpwmfreq[nr - 1],
> +					data->pwm_freq[nr - 1]);
> +	}
> +
> +	mutex_unlock(&data->update_lock);
> +	return count;
> +}
> +
> +#define sysfs_pwm_freq(offset) \
> +static ssize_t show_regs_pwm_freq_##offset (struct device *dev, struct
> device_attribute *attr, char *buf) \
> +{ \
> +	return show_pwm_freq_reg(dev, buf, offset); \
> +} \
> +static ssize_t \
> +store_regs_pwm_freq_##offset (struct device *dev, struct device_attribute
> *attr, const char *buf, size_t count) \
> +{ \
> +	return store_pwm_freq_reg(dev, buf, count, offset); \
> +} \
> +static DEVICE_ATTR(pwm##offset##_freq, S_IRUGO | S_IWUSR, \
> +		  show_regs_pwm_freq_##offset, store_regs_pwm_freq_##offset);
> +
> +sysfs_pwm_freq(1);
> +sysfs_pwm_freq(2);
> +sysfs_pwm_freq(3);
> +
> +static ssize_t
>  show_sensor_reg(struct device *dev, char *buf, int nr)
>  {
>  	struct w83627hf_data *data = w83627hf_update_device(dev);
> @@ -1044,6 +1162,8 @@ static struct attribute *w83627hf_attrib
> 
>  	&dev_attr_pwm1.attr,
>  	&dev_attr_pwm2.attr,
> +	&dev_attr_pwm1_freq.attr,
> +	&dev_attr_pwm2_freq.attr,

These files shouldn't be created for the W83627THF, so they should go
to the optional attributes group.

> 
>  	&dev_attr_name.attr,
>  	NULL
> @@ -1074,6 +1194,7 @@ static struct attribute *w83627hf_attrib
>  	&dev_attr_temp3_type.attr,
> 
>  	&dev_attr_pwm3.attr,
> +	&dev_attr_pwm3_freq.attr,
> 
>  	NULL
>  };
> @@ -1167,6 +1288,10 @@ static int __devinit w83627hf_probe(stru
>  		if ((err = device_create_file(dev, &dev_attr_pwm3)))
>  			goto ERROR4;
> 
> +	if (data->type = w83637hf || data->type = w83687thf)
> +		if ((err = device_create_file(dev, &dev_attr_pwm3_freq)))
> +			goto ERROR4;
> +
>  	data->class_dev = hwmon_device_register(dev);
>  	if (IS_ERR(data->class_dev)) {
>  		err = PTR_ERR(data->class_dev);
> @@ -1470,6 +1595,14 @@ static struct w83627hf_data *w83627hf_up
>  			   (data->type = w83627hf || data->type = w83697hf))
>  				break;
>  		}
> +		for (i = 1; i <= 3; i++) {
> +			u8 tmp = w83627hf_read_value(data,
> +				W836X7HF_REG_PWM_FREQ(data->type, i));
> +			data->pwm_freq[i - 1] = tmp;

You don't need a temporary variable here.

> +			if(i = 2 &&
> +			   (data->type != w83637hf || data->type != w83687thf))
> +				break;
> +		}
> 
>  		data->temp = w83627hf_read_value(data, W83781D_REG_TEMP(1));
>  		data->temp_max 
Please address my comments, test and resubmit.

-- 
Jean Delvare

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

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

* Re: [lm-sensors] hwmon/w83627hf pwm_freq support
  2007-05-20 13:52 [lm-sensors] hwmon/w83627hf pwm_freq support Carlos Olalla Martínez
  2007-05-21 11:57 ` Jean Delvare
@ 2007-05-26 12:55 ` Carlos Olalla Martínez
  2007-05-28  8:14 ` Jean Delvare
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Carlos Olalla Martínez @ 2007-05-26 12:55 UTC (permalink / raw)
  To: lm-sensors


[-- Attachment #1.1: Type: text/plain, Size: 143 bytes --]



>Please address my comments, test and resubmit.

Resubmission, sending it as an attachment.

Signed-off-by:
Carlos Olalla <com.ea@tinet.org>

[-- Attachment #1.2: Type: text/html, Size: 169 bytes --]

[-- Attachment #2: patch_2.6.22-rc1 --]
[-- Type: application/octet-stream, Size: 6484 bytes --]

diff -uprN -X linux-2.6.22-rc1/Documentation/dontdiff linux-2.6.22-rc1.orig/drivers/hwmon/w83627hf.c linux-2.6.22-rc1/drivers/hwmon/w83627hf.c
--- linux-2.6.22-rc1.orig/drivers/hwmon/w83627hf.c	2007-05-13 03:45:56.000000000 +0200
+++ linux-2.6.22-rc1/drivers/hwmon/w83627hf.c	2007-05-26 14:45:05.000000000 +0200
@@ -220,6 +220,18 @@ static const u8 regpwm[] = { W83627THF_R
 #define W836X7HF_REG_PWM(type, nr) (((type) == w83627hf) ? \
                                      regpwm_627hf[(nr) - 1] : regpwm[(nr) - 1])
 
+#define W83627HF_REG_PWM_FREQ		0x5C	/* Only for the 627HF */
+
+#define W83637HF_REG_PWM_FREQ1		0x00	/* 697HF/687THF too */
+#define W83637HF_REG_PWM_FREQ2		0x02	/* 697HF/687THF too */
+#define W83637HF_REG_PWM_FREQ3		0x10	/* 687THF too */
+
+static const u8 W83637HF_REG_PWM_FREQ[] = { W83637HF_REG_PWM_FREQ1, 
+					W83637HF_REG_PWM_FREQ2, 
+					W83637HF_REG_PWM_FREQ1 };
+
+#define W83627HF_BASE_PWM_FREQ	46870
+
 #define W83781D_REG_I2C_ADDR 0x48
 #define W83781D_REG_I2C_SUBADDR 0x4A
 
@@ -267,6 +279,47 @@ static int TEMP_FROM_REG(u8 reg)
 
 #define PWM_TO_REG(val) (SENSORS_LIMIT((val),0,255))
 
+static inline unsigned long pwm_freq_from_reg_627hf(u8 reg)
+{
+	unsigned long freq;
+	freq = W83627HF_BASE_PWM_FREQ >> reg;
+	return freq;
+}
+static inline u8 pwm_freq_to_reg_627hf(unsigned long val)
+{
+	u8 i;
+	/* Only 5 dividers (1 2 4 8 16)... Search for the nearest available frequency */
+	for (i = 0; i < 5; i++) {
+		if (val > (((W83627HF_BASE_PWM_FREQ >> i) + (W83627HF_BASE_PWM_FREQ >> (i+1))) / 2)) 
+			break;
+	}
+	return i;
+}
+
+static inline unsigned long pwm_freq_from_reg(u8 reg)
+{
+	/* Clock bit 8 -> 180 kHz or 24 MHz */
+	unsigned long clock = (reg & 0x80) ? 180000UL : 24000000UL;
+
+	reg &= 0x7f;
+	/* This should not happen but anyway... */
+	if (reg == 0)
+		reg++;
+	return (clock / (reg << 8));
+}
+static inline u8 pwm_freq_to_reg(unsigned long val)
+{
+	/* Minimum divider value is 0x01 and maximum is 0x7F */
+	if (val >= 93750)	/* The highest we can do */
+		return 0x01;
+	if (val >= 720)	/* Use 24 MHz clock */
+		return (24000000UL / (val << 8));
+	if (val < 6)		/* The lowest we can do */
+		return 0xFF;
+	else			/* Use 180 kHz clock */
+		return (0x80 | (180000UL / (val << 8)));
+}
+
 #define BEEP_MASK_FROM_REG(val)		 (val)
 #define BEEP_MASK_TO_REG(val)		((val) & 0xffffff)
 #define BEEP_ENABLE_TO_REG(val)		((val)?1:0)
@@ -316,6 +369,7 @@ struct w83627hf_data {
 	u32 beep_mask;		/* Register encoding, combined */
 	u8 beep_enable;		/* Boolean */
 	u8 pwm[3];		/* Register value */
+	u8 pwm_freq[3];		/* Register value */ 
 	u16 sens[3];		/* 782D/783S only.
 				   1 = pentium diode; 2 = 3904 diode;
 				   3000-5000 = thermistor beta.
@@ -852,6 +906,59 @@ sysfs_pwm(2);
 sysfs_pwm(3);
 
 static ssize_t
+show_pwm_freq_reg(struct device *dev, char *buf, int nr)
+{
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	if (data->type == w83627hf) 
+		return sprintf(buf, "%ld\n", pwm_freq_from_reg_627hf(data->pwm_freq[nr - 1]));
+	else
+		return sprintf(buf, "%ld\n", pwm_freq_from_reg(data->pwm_freq[nr - 1]));
+}
+
+static ssize_t
+store_pwm_freq_reg(struct device *dev, const char *buf, size_t count, int nr)
+{
+	struct w83627hf_data *data = dev_get_drvdata(dev);
+	static const u8 mask[]={0xF8, 0x8F};
+	u32 val;
+
+	val = simple_strtoul(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
+
+	if (data->type == w83627hf) {
+		data->pwm_freq[nr - 1] = pwm_freq_to_reg_627hf(val);
+		w83627hf_write_value(data, W83627HF_REG_PWM_FREQ, 
+					(data->pwm_freq[nr - 1] << ((nr - 1)*4)) | 
+					(w83627hf_read_value(data, W83627HF_REG_PWM_FREQ) & mask[nr - 1]));
+	} else {
+		data->pwm_freq[nr - 1] = pwm_freq_to_reg(val);
+		w83627hf_write_value(data, W83637HF_REG_PWM_FREQ[nr - 1],
+					data->pwm_freq[nr - 1]);
+	}
+
+	mutex_unlock(&data->update_lock);
+	return count;
+}
+
+#define sysfs_pwm_freq(offset) \
+static ssize_t show_regs_pwm_freq_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
+{ \
+	return show_pwm_freq_reg(dev, buf, offset); \
+} \
+static ssize_t \
+store_regs_pwm_freq_##offset (struct device *dev, struct device_attribute *attr, const char *buf, size_t count) \
+{ \
+	return store_pwm_freq_reg(dev, buf, count, offset); \
+} \
+static DEVICE_ATTR(pwm##offset##_freq, S_IRUGO | S_IWUSR, \
+		  show_regs_pwm_freq_##offset, store_regs_pwm_freq_##offset);
+
+sysfs_pwm_freq(1);
+sysfs_pwm_freq(2);
+sysfs_pwm_freq(3);
+
+static ssize_t
 show_sensor_reg(struct device *dev, char *buf, int nr)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
@@ -1075,6 +1182,9 @@ static struct attribute *w83627hf_attrib
 
 	&dev_attr_pwm3.attr,
 
+	&dev_attr_pwm1_freq.attr,
+	&dev_attr_pwm2_freq.attr,
+	&dev_attr_pwm3_freq.attr,
 	NULL
 };
 
@@ -1137,7 +1247,9 @@ static int __devinit w83627hf_probe(stru
 		 || (err = device_create_file(dev, &dev_attr_in5_max))
 		 || (err = device_create_file(dev, &dev_attr_in6_input))
 		 || (err = device_create_file(dev, &dev_attr_in6_min))
-		 || (err = device_create_file(dev, &dev_attr_in6_max)))
+		 || (err = device_create_file(dev, &dev_attr_in6_max))
+		 || (err = device_create_file(dev, &dev_attr_pwm1_freq))
+		 || (err = device_create_file(dev, &dev_attr_pwm2_freq)))
 			goto ERROR4;
 
 	if (data->type != w83697hf)
@@ -1167,6 +1279,12 @@ static int __devinit w83627hf_probe(stru
 		if ((err = device_create_file(dev, &dev_attr_pwm3)))
 			goto ERROR4;
 
+	if (data->type == w83637hf || data->type == w83687thf)
+		if ((err = device_create_file(dev, &dev_attr_pwm1_freq))
+		 || (err = device_create_file(dev, &dev_attr_pwm2_freq))
+		 || (err = device_create_file(dev, &dev_attr_pwm3_freq)))
+			goto ERROR4;
+
 	data->class_dev = hwmon_device_register(dev);
 	if (IS_ERR(data->class_dev)) {
 		err = PTR_ERR(data->class_dev);
@@ -1470,6 +1588,22 @@ static struct w83627hf_data *w83627hf_up
 			   (data->type == w83627hf || data->type == w83697hf))
 				break;
 		}
+		if (data->type != w83627thf) {
+			if (data->type == w83627hf) {
+					u8 tmp = w83627hf_read_value(data, 
+							W83627HF_REG_PWM_FREQ);
+					data->pwm_freq[0] = tmp & 0x07;
+					data->pwm_freq[1] = (tmp >> 4) & 0x07;
+				}
+			else {
+				for (i = 1; i <= 3; i++) {
+					data->pwm_freq[i - 1] = w83627hf_read_value(data,
+								 W83637HF_REG_PWM_FREQ[i - 1]);
+					if(i == 2 && (data->type == w83697hf))
+						break;
+				}
+			}
+		}
 
 		data->temp = w83627hf_read_value(data, W83781D_REG_TEMP(1));
 		data->temp_max =

[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

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

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

* Re: [lm-sensors] hwmon/w83627hf pwm_freq support
  2007-05-20 13:52 [lm-sensors] hwmon/w83627hf pwm_freq support Carlos Olalla Martínez
  2007-05-21 11:57 ` Jean Delvare
  2007-05-26 12:55 ` Carlos Olalla Martínez
@ 2007-05-28  8:14 ` Jean Delvare
  2007-05-28 12:37 ` Carlos Olalla Martínez
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Jean Delvare @ 2007-05-28  8:14 UTC (permalink / raw)
  To: lm-sensors

Hi Carlos,

On Sat, 26 May 2007 14:55:19 +0200 (CEST), Carlos Olalla Martínez wrote:

> diff -uprN -X linux-2.6.22-rc1/Documentation/dontdiff linux-2.6.22-rc1.orig/drivers/hwmon/w83627hf.c linux-2.6.22-rc1/drivers/hwmon/w83627hf.c
> --- linux-2.6.22-rc1.orig/drivers/hwmon/w83627hf.c	2007-05-13 03:45:56.000000000 +0200
> +++ linux-2.6.22-rc1/drivers/hwmon/w83627hf.c	2007-05-26 14:45:05.000000000 +0200
> @@ -220,6 +220,18 @@ static const u8 regpwm[] = { W83627THF_R
>  #define W836X7HF_REG_PWM(type, nr) (((type) = w83627hf) ? \
>                                       regpwm_627hf[(nr) - 1] : regpwm[(nr) - 1])
>  
> +#define W83627HF_REG_PWM_FREQ		0x5C	/* Only for the 627HF */
> +
> +#define W83637HF_REG_PWM_FREQ1		0x00	/* 697HF/687THF too */
> +#define W83637HF_REG_PWM_FREQ2		0x02	/* 697HF/687THF too */
> +#define W83637HF_REG_PWM_FREQ3		0x10	/* 687THF too */
> +
> +static const u8 W83637HF_REG_PWM_FREQ[] = { W83637HF_REG_PWM_FREQ1, 
> +					W83637HF_REG_PWM_FREQ2, 
> +					W83637HF_REG_PWM_FREQ1 };

This should obviously be W83637HF_REG_PWM_FREQ3.

> +
> +#define W83627HF_BASE_PWM_FREQ	46870
> +
>  #define W83781D_REG_I2C_ADDR 0x48
>  #define W83781D_REG_I2C_SUBADDR 0x4A
>  
> @@ -267,6 +279,47 @@ static int TEMP_FROM_REG(u8 reg)
>  
>  #define PWM_TO_REG(val) (SENSORS_LIMIT((val),0,255))
>  
> +static inline unsigned long pwm_freq_from_reg_627hf(u8 reg)
> +{
> +	unsigned long freq;
> +	freq = W83627HF_BASE_PWM_FREQ >> reg;
> +	return freq;
> +}
> +static inline u8 pwm_freq_to_reg_627hf(unsigned long val)
> +{
> +	u8 i;
> +	/* Only 5 dividers (1 2 4 8 16)... Search for the nearest available frequency */
> +	for (i = 0; i < 5; i++) {
> +		if (val > (((W83627HF_BASE_PWM_FREQ >> i) + (W83627HF_BASE_PWM_FREQ >> (i+1))) / 2)) 
> +			break;
> +	}
> +	return i;
> +}

This could return with i = 5, which isn't correct.

Also, line length is limited to 80, please fold the lines that are
longer than that. This applies to the rest of your patch too.

> +
> +static inline unsigned long pwm_freq_from_reg(u8 reg)
> +{
> +	/* Clock bit 8 -> 180 kHz or 24 MHz */
> +	unsigned long clock = (reg & 0x80) ? 180000UL : 24000000UL;
> +
> +	reg &= 0x7f;
> +	/* This should not happen but anyway... */
> +	if (reg = 0)
> +		reg++;
> +	return (clock / (reg << 8));
> +}
> +static inline u8 pwm_freq_to_reg(unsigned long val)
> +{
> +	/* Minimum divider value is 0x01 and maximum is 0x7F */
> +	if (val >= 93750)	/* The highest we can do */
> +		return 0x01;
> +	if (val >= 720)	/* Use 24 MHz clock */
> +		return (24000000UL / (val << 8));
> +	if (val < 6)		/* The lowest we can do */
> +		return 0xFF;
> +	else			/* Use 180 kHz clock */
> +		return (0x80 | (180000UL / (val << 8)));
> +}
> +
>  #define BEEP_MASK_FROM_REG(val)		 (val)
>  #define BEEP_MASK_TO_REG(val)		((val) & 0xffffff)
>  #define BEEP_ENABLE_TO_REG(val)		((val)?1:0)
> @@ -316,6 +369,7 @@ struct w83627hf_data {
>  	u32 beep_mask;		/* Register encoding, combined */
>  	u8 beep_enable;		/* Boolean */
>  	u8 pwm[3];		/* Register value */
> +	u8 pwm_freq[3];		/* Register value */ 
>  	u16 sens[3];		/* 782D/783S only.
>  				   1 = pentium diode; 2 = 3904 diode;
>  				   3000-5000 = thermistor beta.
> @@ -852,6 +906,59 @@ sysfs_pwm(2);
>  sysfs_pwm(3);
>  
>  static ssize_t
> +show_pwm_freq_reg(struct device *dev, char *buf, int nr)
> +{
> +	struct w83627hf_data *data = w83627hf_update_device(dev);
> +	if (data->type = w83627hf) 
> +		return sprintf(buf, "%ld\n", pwm_freq_from_reg_627hf(data->pwm_freq[nr - 1]));
> +	else
> +		return sprintf(buf, "%ld\n", pwm_freq_from_reg(data->pwm_freq[nr - 1]));
> +}
> +
> +static ssize_t
> +store_pwm_freq_reg(struct device *dev, const char *buf, size_t count, int nr)
> +{
> +	struct w83627hf_data *data = dev_get_drvdata(dev);
> +	static const u8 mask[]={0xF8, 0x8F};
> +	u32 val;
> +
> +	val = simple_strtoul(buf, NULL, 10);
> +
> +	mutex_lock(&data->update_lock);
> +
> +	if (data->type = w83627hf) {
> +		data->pwm_freq[nr - 1] = pwm_freq_to_reg_627hf(val);
> +		w83627hf_write_value(data, W83627HF_REG_PWM_FREQ, 
> +					(data->pwm_freq[nr - 1] << ((nr - 1)*4)) | 
> +					(w83627hf_read_value(data, W83627HF_REG_PWM_FREQ) & mask[nr - 1]));
> +	} else {
> +		data->pwm_freq[nr - 1] = pwm_freq_to_reg(val);
> +		w83627hf_write_value(data, W83637HF_REG_PWM_FREQ[nr - 1],
> +					data->pwm_freq[nr - 1]);
> +	}
> +
> +	mutex_unlock(&data->update_lock);
> +	return count;
> +}
> +
> +#define sysfs_pwm_freq(offset) \
> +static ssize_t show_regs_pwm_freq_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
> +{ \
> +	return show_pwm_freq_reg(dev, buf, offset); \
> +} \
> +static ssize_t \
> +store_regs_pwm_freq_##offset (struct device *dev, struct device_attribute *attr, const char *buf, size_t count) \
> +{ \
> +	return store_pwm_freq_reg(dev, buf, count, offset); \
> +} \
> +static DEVICE_ATTR(pwm##offset##_freq, S_IRUGO | S_IWUSR, \
> +		  show_regs_pwm_freq_##offset, store_regs_pwm_freq_##offset);
> +
> +sysfs_pwm_freq(1);
> +sysfs_pwm_freq(2);
> +sysfs_pwm_freq(3);
> +
> +static ssize_t
>  show_sensor_reg(struct device *dev, char *buf, int nr)
>  {
>  	struct w83627hf_data *data = w83627hf_update_device(dev);
> @@ -1075,6 +1182,9 @@ static struct attribute *w83627hf_attrib
>  
>  	&dev_attr_pwm3.attr,
>  
> +	&dev_attr_pwm1_freq.attr,
> +	&dev_attr_pwm2_freq.attr,
> +	&dev_attr_pwm3_freq.attr,
>  	NULL
>  };
>  
> @@ -1137,7 +1247,9 @@ static int __devinit w83627hf_probe(stru
>  		 || (err = device_create_file(dev, &dev_attr_in5_max))
>  		 || (err = device_create_file(dev, &dev_attr_in6_input))
>  		 || (err = device_create_file(dev, &dev_attr_in6_min))
> -		 || (err = device_create_file(dev, &dev_attr_in6_max)))
> +		 || (err = device_create_file(dev, &dev_attr_in6_max))
> +		 || (err = device_create_file(dev, &dev_attr_pwm1_freq))
> +		 || (err = device_create_file(dev, &dev_attr_pwm2_freq)))
>  			goto ERROR4;
>  
>  	if (data->type != w83697hf)
> @@ -1167,6 +1279,12 @@ static int __devinit w83627hf_probe(stru
>  		if ((err = device_create_file(dev, &dev_attr_pwm3)))
>  			goto ERROR4;
>  
> +	if (data->type = w83637hf || data->type = w83687thf)
> +		if ((err = device_create_file(dev, &dev_attr_pwm1_freq))
> +		 || (err = device_create_file(dev, &dev_attr_pwm2_freq))
> +		 || (err = device_create_file(dev, &dev_attr_pwm3_freq)))
> +			goto ERROR4;
> +
>  	data->class_dev = hwmon_device_register(dev);
>  	if (IS_ERR(data->class_dev)) {
>  		err = PTR_ERR(data->class_dev);
> @@ -1470,6 +1588,22 @@ static struct w83627hf_data *w83627hf_up
>  			   (data->type = w83627hf || data->type = w83697hf))
>  				break;
>  		}
> +		if (data->type != w83627thf) {
> +			if (data->type = w83627hf) {
> +					u8 tmp = w83627hf_read_value(data, 
> +							W83627HF_REG_PWM_FREQ);
> +					data->pwm_freq[0] = tmp & 0x07;
> +					data->pwm_freq[1] = (tmp >> 4) & 0x07;
> +				}
> +			else {
> +				for (i = 1; i <= 3; i++) {
> +					data->pwm_freq[i - 1] = w83627hf_read_value(data,
> +								 W83637HF_REG_PWM_FREQ[i - 1]);
> +					if(i = 2 && (data->type = w83697hf))
> +						break;
> +				}
> +			}
> +		}

The type tests can be optimized:

		if (data->type = w83627hf) {
			(...)
		} else if (data->type != w83627thf) {
			(...)
		}


>  
>  		data->temp = w83627hf_read_value(data, W83781D_REG_TEMP(1));
>  		data->temp_max 
Other than that, your patch looks good to me, thanks. Please provide an
updated version addressing the few remaining issues and I'll apply it.

-- 
Jean Delvare

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

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

* Re: [lm-sensors] hwmon/w83627hf pwm_freq support
  2007-05-20 13:52 [lm-sensors] hwmon/w83627hf pwm_freq support Carlos Olalla Martínez
                   ` (2 preceding siblings ...)
  2007-05-28  8:14 ` Jean Delvare
@ 2007-05-28 12:37 ` Carlos Olalla Martínez
  2007-05-28 12:54 ` Dmitry Bely
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Carlos Olalla Martínez @ 2007-05-28 12:37 UTC (permalink / raw)
  To: lm-sensors

> > +	u8 i;
> > +	/* Only 5 dividers (1 2 4 8 16)... Search for the nearest available
frequency */
> > +	for (i = 0; i < 5; i++) {
> > +		if (val > (((W83627HF_BASE_PWM_FREQ >> i) + (W83627HF_BASE_PWM_FREQ
>> (i+1))) / 2))
> > +			break;
> > +	}
> > +	return i;
> > +}

> This could return with i = 5, which isn't correct.

Hi Jean,

I dont agree on your statement; I think

for (i = 0; i < 5; i++)

is the same as

for (i = 0; i <= 4; i++)

so maximum i returned is 4. Am I wrong?¿

Carlos



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

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

* Re: [lm-sensors] hwmon/w83627hf pwm_freq support
  2007-05-20 13:52 [lm-sensors] hwmon/w83627hf pwm_freq support Carlos Olalla Martínez
                   ` (3 preceding siblings ...)
  2007-05-28 12:37 ` Carlos Olalla Martínez
@ 2007-05-28 12:54 ` Dmitry Bely
  2007-05-28 13:16 ` Carlos Olalla Martínez
  2007-05-28 16:15 ` Jean Delvare
  6 siblings, 0 replies; 8+ messages in thread
From: Dmitry Bely @ 2007-05-28 12:54 UTC (permalink / raw)
  To: lm-sensors

On 5/28/07, Carlos Olalla Martínez <com.ea@tinet.org> wrote:
> > > +   u8 i;
> > > +   /* Only 5 dividers (1 2 4 8 16)... Search for the nearest available
> frequency */
> > > +   for (i = 0; i < 5; i++) {
> > > +           if (val > (((W83627HF_BASE_PWM_FREQ >> i) + (W83627HF_BASE_PWM_FREQ
> >> (i+1))) / 2))
> > > +                   break;
> > > +   }
> > > +   return i;
> > > +}
>
> > This could return with i = 5, which isn't correct.
>
> Hi Jean,
>
> I dont agree on your statement; I think
>
> for (i = 0; i < 5; i++)
>
> is the same as
>
> for (i = 0; i <= 4; i++)
>
> so maximum i returned is 4. Am I wrong?

Of course, wrong:

for (i = 0; i < 5; i++){
}
// i = 5 here

- Dmitry Bely

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

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

* Re: [lm-sensors] hwmon/w83627hf pwm_freq support
  2007-05-20 13:52 [lm-sensors] hwmon/w83627hf pwm_freq support Carlos Olalla Martínez
                   ` (4 preceding siblings ...)
  2007-05-28 12:54 ` Dmitry Bely
@ 2007-05-28 13:16 ` Carlos Olalla Martínez
  2007-05-28 16:15 ` Jean Delvare
  6 siblings, 0 replies; 8+ messages in thread
From: Carlos Olalla Martínez @ 2007-05-28 13:16 UTC (permalink / raw)
  To: lm-sensors

[-- Attachment #1: Type: text/plain, Size: 80 bytes --]

Ok got the point.

Resubmitting;
Signed-off-by: Carlos Olalla <com.ea@tinet.org>

[-- Attachment #2: patch_2.6.22-rc1 --]
[-- Type: application/octet-stream, Size: 6476 bytes --]

diff -uprN -X linux-2.6.22-rc1/Documentation/dontdiff linux-2.6.22-rc1.orig/drivers/hwmon/w83627hf.c linux-2.6.22-rc1/drivers/hwmon/w83627hf.c
--- linux-2.6.22-rc1.orig/drivers/hwmon/w83627hf.c	2007-05-13 03:45:56.000000000 +0200
+++ linux-2.6.22-rc1/drivers/hwmon/w83627hf.c	2007-05-28 15:09:22.000000000 +0200
@@ -220,6 +220,18 @@ static const u8 regpwm[] = { W83627THF_R
 #define W836X7HF_REG_PWM(type, nr) (((type) == w83627hf) ? \
                                      regpwm_627hf[(nr) - 1] : regpwm[(nr) - 1])
 
+#define W83627HF_REG_PWM_FREQ		0x5C	/* Only for the 627HF */
+
+#define W83637HF_REG_PWM_FREQ1		0x00	/* 697HF/687THF too */
+#define W83637HF_REG_PWM_FREQ2		0x02	/* 697HF/687THF too */
+#define W83637HF_REG_PWM_FREQ3		0x10	/* 687THF too */
+
+static const u8 W83637HF_REG_PWM_FREQ[] = { W83637HF_REG_PWM_FREQ1, 
+					W83637HF_REG_PWM_FREQ2, 
+					W83637HF_REG_PWM_FREQ3 };
+
+#define W83627HF_BASE_PWM_FREQ	46870
+
 #define W83781D_REG_I2C_ADDR 0x48
 #define W83781D_REG_I2C_SUBADDR 0x4A
 
@@ -267,6 +279,49 @@ static int TEMP_FROM_REG(u8 reg)
 
 #define PWM_TO_REG(val) (SENSORS_LIMIT((val),0,255))
 
+static inline unsigned long pwm_freq_from_reg_627hf(u8 reg)
+{
+	unsigned long freq;
+	freq = W83627HF_BASE_PWM_FREQ >> reg;
+	return freq;
+}
+static inline u8 pwm_freq_to_reg_627hf(unsigned long val)
+{
+	u8 i;
+	/* 	Only 5 dividers (1 2 4 8 16)
+		 Search for the nearest available frequency */
+	for (i = 0; i < 4; i++) {
+		if (val > (((W83627HF_BASE_PWM_FREQ >> i) 
+			+ (W83627HF_BASE_PWM_FREQ >> (i+1))) / 2)) 
+			break;
+	}
+	return i;
+}
+
+static inline unsigned long pwm_freq_from_reg(u8 reg)
+{
+	/* Clock bit 8 -> 180 kHz or 24 MHz */
+	unsigned long clock = (reg & 0x80) ? 180000UL : 24000000UL;
+
+	reg &= 0x7f;
+	/* This should not happen but anyway... */
+	if (reg == 0)
+		reg++;
+	return (clock / (reg << 8));
+}
+static inline u8 pwm_freq_to_reg(unsigned long val)
+{
+	/* Minimum divider value is 0x01 and maximum is 0x7F */
+	if (val >= 93750)	/* The highest we can do */
+		return 0x01;
+	if (val >= 720)	/* Use 24 MHz clock */
+		return (24000000UL / (val << 8));
+	if (val < 6)		/* The lowest we can do */
+		return 0xFF;
+	else			/* Use 180 kHz clock */
+		return (0x80 | (180000UL / (val << 8)));
+}
+
 #define BEEP_MASK_FROM_REG(val)		 (val)
 #define BEEP_MASK_TO_REG(val)		((val) & 0xffffff)
 #define BEEP_ENABLE_TO_REG(val)		((val)?1:0)
@@ -316,6 +371,7 @@ struct w83627hf_data {
 	u32 beep_mask;		/* Register encoding, combined */
 	u8 beep_enable;		/* Boolean */
 	u8 pwm[3];		/* Register value */
+	u8 pwm_freq[3];		/* Register value */ 
 	u16 sens[3];		/* 782D/783S only.
 				   1 = pentium diode; 2 = 3904 diode;
 				   3000-5000 = thermistor beta.
@@ -852,6 +908,62 @@ sysfs_pwm(2);
 sysfs_pwm(3);
 
 static ssize_t
+show_pwm_freq_reg(struct device *dev, char *buf, int nr)
+{
+	struct w83627hf_data *data = w83627hf_update_device(dev);
+	if (data->type == w83627hf) 
+		return sprintf(buf, "%ld\n", 
+			pwm_freq_from_reg_627hf(data->pwm_freq[nr - 1]));
+	else
+		return sprintf(buf, "%ld\n", 
+			pwm_freq_from_reg(data->pwm_freq[nr - 1]));
+}
+
+static ssize_t
+store_pwm_freq_reg(struct device *dev, const char *buf, size_t count, int nr)
+{
+	struct w83627hf_data *data = dev_get_drvdata(dev);
+	static const u8 mask[]={0xF8, 0x8F};
+	u32 val;
+
+	val = simple_strtoul(buf, NULL, 10);
+
+	mutex_lock(&data->update_lock);
+
+	if (data->type == w83627hf) {
+		data->pwm_freq[nr - 1] = pwm_freq_to_reg_627hf(val);
+		w83627hf_write_value(data, W83627HF_REG_PWM_FREQ, 
+				(data->pwm_freq[nr - 1] << ((nr - 1)*4)) | 
+				(w83627hf_read_value(data, 
+				W83627HF_REG_PWM_FREQ) & mask[nr - 1]));
+	} else {
+		data->pwm_freq[nr - 1] = pwm_freq_to_reg(val);
+		w83627hf_write_value(data, W83637HF_REG_PWM_FREQ[nr - 1],
+				data->pwm_freq[nr - 1]);
+	}
+
+	mutex_unlock(&data->update_lock);
+	return count;
+}
+
+#define sysfs_pwm_freq(offset) \
+static ssize_t show_regs_pwm_freq_##offset (struct device *dev, struct device_attribute *attr, char *buf) \
+{ \
+	return show_pwm_freq_reg(dev, buf, offset); \
+} \
+static ssize_t \
+store_regs_pwm_freq_##offset (struct device *dev, struct device_attribute *attr, const char *buf, size_t count) \
+{ \
+	return store_pwm_freq_reg(dev, buf, count, offset); \
+} \
+static DEVICE_ATTR(pwm##offset##_freq, S_IRUGO | S_IWUSR, \
+		  show_regs_pwm_freq_##offset, store_regs_pwm_freq_##offset);
+
+sysfs_pwm_freq(1);
+sysfs_pwm_freq(2);
+sysfs_pwm_freq(3);
+
+static ssize_t
 show_sensor_reg(struct device *dev, char *buf, int nr)
 {
 	struct w83627hf_data *data = w83627hf_update_device(dev);
@@ -1075,6 +1187,9 @@ static struct attribute *w83627hf_attrib
 
 	&dev_attr_pwm3.attr,
 
+	&dev_attr_pwm1_freq.attr,
+	&dev_attr_pwm2_freq.attr,
+	&dev_attr_pwm3_freq.attr,
 	NULL
 };
 
@@ -1137,7 +1252,9 @@ static int __devinit w83627hf_probe(stru
 		 || (err = device_create_file(dev, &dev_attr_in5_max))
 		 || (err = device_create_file(dev, &dev_attr_in6_input))
 		 || (err = device_create_file(dev, &dev_attr_in6_min))
-		 || (err = device_create_file(dev, &dev_attr_in6_max)))
+		 || (err = device_create_file(dev, &dev_attr_in6_max))
+		 || (err = device_create_file(dev, &dev_attr_pwm1_freq))
+		 || (err = device_create_file(dev, &dev_attr_pwm2_freq)))
 			goto ERROR4;
 
 	if (data->type != w83697hf)
@@ -1167,6 +1284,12 @@ static int __devinit w83627hf_probe(stru
 		if ((err = device_create_file(dev, &dev_attr_pwm3)))
 			goto ERROR4;
 
+	if (data->type == w83637hf || data->type == w83687thf)
+		if ((err = device_create_file(dev, &dev_attr_pwm1_freq))
+		 || (err = device_create_file(dev, &dev_attr_pwm2_freq))
+		 || (err = device_create_file(dev, &dev_attr_pwm3_freq)))
+			goto ERROR4;
+
 	data->class_dev = hwmon_device_register(dev);
 	if (IS_ERR(data->class_dev)) {
 		err = PTR_ERR(data->class_dev);
@@ -1470,6 +1593,19 @@ static struct w83627hf_data *w83627hf_up
 			   (data->type == w83627hf || data->type == w83697hf))
 				break;
 		}
+		if (data->type == w83627hf) {
+				u8 tmp = w83627hf_read_value(data, 
+						W83627HF_REG_PWM_FREQ);
+				data->pwm_freq[0] = tmp & 0x07;
+				data->pwm_freq[1] = (tmp >> 4) & 0x07;
+		} else if (data->type != w83627thf) {
+			for (i = 1; i <= 3; i++) {
+				data->pwm_freq[i - 1] = w83627hf_read_value(data,
+							 W83637HF_REG_PWM_FREQ[i - 1]);
+				if(i == 2 && (data->type == w83697hf))
+					break;
+			}
+		}
 
 		data->temp = w83627hf_read_value(data, W83781D_REG_TEMP(1));
 		data->temp_max =

[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

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

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

* Re: [lm-sensors] hwmon/w83627hf pwm_freq support
  2007-05-20 13:52 [lm-sensors] hwmon/w83627hf pwm_freq support Carlos Olalla Martínez
                   ` (5 preceding siblings ...)
  2007-05-28 13:16 ` Carlos Olalla Martínez
@ 2007-05-28 16:15 ` Jean Delvare
  6 siblings, 0 replies; 8+ messages in thread
From: Jean Delvare @ 2007-05-28 16:15 UTC (permalink / raw)
  To: lm-sensors

On Mon, 28 May 2007 15:16:39 +0200 (CEST), Carlos Olalla Martínez wrote:
> Ok got the point.
> 
> Resubmitting;
> Signed-off-by: Carlos Olalla <com.ea@tinet.org>

Applied, thanks!

-- 
Jean Delvare

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

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

end of thread, other threads:[~2007-05-28 16:15 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-05-20 13:52 [lm-sensors] hwmon/w83627hf pwm_freq support Carlos Olalla Martínez
2007-05-21 11:57 ` Jean Delvare
2007-05-26 12:55 ` Carlos Olalla Martínez
2007-05-28  8:14 ` Jean Delvare
2007-05-28 12:37 ` Carlos Olalla Martínez
2007-05-28 12:54 ` Dmitry Bely
2007-05-28 13:16 ` Carlos Olalla Martínez
2007-05-28 16:15 ` Jean Delvare

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.