Hi Jean,
On Monday 19 May 2008 23:07, Jean Delvare wrote:
> Hi Dominik,
>
> On Fri, 16 May 2008 18:36:42 +0200, Dominik Geyer wrote:
> > Adds support for pwm_enable sysfs-interface for the w83627hf-driver.
> >
> > Signed-off-by: Dominik Geyer <dominik.geyer at gmx.de>
> >
> > Please CC me, because I'm not subscribed to the list.
> > ---
> >
> > Hi Jean,
> >
> > I've reworked my initial patch so that pwmX_enable sysfs-interface is
> > supported now. I'm not really sure about the "Thermal Cruise Mode" and
> > the "Fan Speed Cruise Mode" setting (thus in this patch only value 1 and 2
> > are allowed). I looked up the datasheets and it seems that all chips except
> > W83627HF have support for fan-config.
> >
> > I tested the "Manual PWM Control Mode" on a W83697HF.
>
> Thanks for working on this, I wanted to do it long ago but could never
> find the time.
>
> > --- linux-2.6.25.3/drivers/hwmon/w83627hf.c.orig 2008-05-12 11:40:03.000000000 +0000
> > +++ linux-2.6.25.3/drivers/hwmon/w83627hf.c 2008-05-16 16:01:34.000000000 +0000
> > @@ -209,6 +209,9 @@
> > #define W83627HF_REG_PWM1 0x5A
> > #define W83627HF_REG_PWM2 0x5B
> >
> > +#define W83627THF_REG_FAN_CONFIG 0x04 /* 697HF/637HF/687THF too */
> > +static const u8 W83627THF_PWM_ENABLE_SHIFT[] = { 2, 4 };
> > +
>
> The W83627THF, W83637HF and W83687THF have a 3rd PWM channel those mode
> can be changed as well. The configuration bits are in Fan Configuration
> Register II (0x12 in bank 0). While I understand that you have no
> interest in this for your W83697HF chip, I still would like you to add
> support for it in your patch, so that we can really say that the
> w83627hf support PWM mode switching. It shouldn't be too difficult.
Oh, I missed that one while looking into the datasheets. I assumed a potencial PWM_3_MODE
in [7:6] of REG 0x04, but these registers were reserved.
Fixed, added support for 3rd PWM channel.
> > #define W83627THF_REG_PWM1 0x01 /* 697HF/637HF/687THF too */
> > #define W83627THF_REG_PWM2 0x03 /* 697HF/637HF/687THF too */
> > #define W83627THF_REG_PWM3 0x11 /* 637HF/687THF too */
> > @@ -366,6 +369,8 @@
> > u32 alarms; /* Register encoding, combined */
> > u32 beep_mask; /* Register encoding, combined */
> > u8 pwm[3]; /* Register value */
> > + u8 pwm_enable[2]; /* 1->manual 2->thermal cruise (also called SmartFan I)
>
> Line too long.
Fixed (and the other ones, too).
> > +static ssize_t
> > +store_pwm_enable(struct device *dev, struct device_attribute *devattr,
> > + const char *buf, size_t count)
> > +{
> > + int nr = to_sensor_dev_attr(devattr)->index;
> > + struct w83627hf_data *data = dev_get_drvdata(dev);
> > + u32 val = simple_strtoul(buf, NULL, 10);
>
> Should be unsigned long.
Fixed.
> > + u16 reg;
>
> Could be u8.
Changed.
> > +
> > + if (!val || (val > 2)) /* only modes 1 and 2 are supported */
> > + return -EINVAL;
>
> You should support value 3 as well. The BIOS might have setup the chip
> in this mode, and if you let the user change it to manual, it's only
> fair that you let them change it back to the original value afterwards.
Sounds reasonable. I didn't know anything of mode-3, so I only allowed mode 1 and 2 as
a precaution. Changed.
> Please submit an updated patch. Unfortunately I won't be able to test
> it, as I no longer have a test board with a supported chip.
Please find my new patch (inline & attached).
There are no checks for the presence of a PWM3 in functions show_pwm_enable() and
store_pwm_enable(), I hope this is correct.
I tested switching to manual pwm-mode and back to automatic pwm-mode on a
W83697HF.
Regards,
Dominik
--
diff -Naur linux-2.6.25.3.orig/drivers/hwmon/w83627hf.c linux-2.6.25.3/drivers/hwmon/w83627hf.c
--- linux-2.6.25.3.orig/drivers/hwmon/w83627hf.c 2008-05-10 04:48:50.000000000 +0000
+++ linux-2.6.25.3/drivers/hwmon/w83627hf.c 2008-05-20 16:27:37.000000000 +0000
@@ -209,6 +209,13 @@
#define W83627HF_REG_PWM1 0x5A
#define W83627HF_REG_PWM2 0x5B
+static const u8 W83627THF_REG_PWM_ENABLE[] = {
+ 0x04, /* FAN 1 mode */
+ 0x04, /* FAN 2 mode */
+ 0x12, /* FAN AUX mode */
+};
+static const u8 W83627THF_PWM_ENABLE_SHIFT[] = { 2, 4, 1 };
+
#define W83627THF_REG_PWM1 0x01 /* 697HF/637HF/687THF too */
#define W83627THF_REG_PWM2 0x03 /* 697HF/637HF/687THF too */
#define W83627THF_REG_PWM3 0x11 /* 637HF/687THF too */
@@ -366,6 +373,9 @@
u32 alarms; /* Register encoding, combined */
u32 beep_mask; /* Register encoding, combined */
u8 pwm[3]; /* Register value */
+ u8 pwm_enable[3]; /* 1 = manual
+ 2 = thermal cruise (also called SmartFan I)
+ 3 = fan speed cruise */
u8 pwm_freq[3]; /* Register value */
u16 sens[3]; /* 1 = pentium diode; 2 = 3904 diode;
4 = thermistor */
@@ -957,6 +967,42 @@
static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO|S_IWUSR, show_pwm, store_pwm, 2);
static ssize_t
+show_pwm_enable(struct device *dev, struct device_attribute *devattr, char *buf)
+{
+ int nr = to_sensor_dev_attr(devattr)->index;
+ struct w83627hf_data *data = w83627hf_update_device(dev);
+ return sprintf(buf, "%d\n", data->pwm_enable[nr]);
+}
+
+static ssize_t
+store_pwm_enable(struct device *dev, struct device_attribute *devattr,
+ const char *buf, size_t count)
+{
+ int nr = to_sensor_dev_attr(devattr)->index;
+ struct w83627hf_data *data = dev_get_drvdata(dev);
+ unsigned long val = simple_strtoul(buf, NULL, 10);
+ u8 reg;
+
+ if (!val || (val > 3)) /* modes 1, 2 and 3 are supported */
+ return -EINVAL;
+ mutex_lock(&data->update_lock);
+ data->pwm_enable[nr] = val;
+ reg = w83627hf_read_value(data, W83627THF_REG_PWM_ENABLE[nr]);
+ reg &= ~(0x03 << W83627THF_PWM_ENABLE_SHIFT[nr]);
+ reg |= (val - 1) << W83627THF_PWM_ENABLE_SHIFT[nr];
+ w83627hf_write_value(data, W83627THF_REG_PWM_ENABLE[nr], reg);
+ mutex_unlock(&data->update_lock);
+ return count;
+}
+
+static SENSOR_DEVICE_ATTR(pwm1_enable, S_IRUGO|S_IWUSR, show_pwm_enable,
+ store_pwm_enable, 0);
+static SENSOR_DEVICE_ATTR(pwm2_enable, S_IRUGO|S_IWUSR, show_pwm_enable,
+ store_pwm_enable, 1);
+static SENSOR_DEVICE_ATTR(pwm3_enable, S_IRUGO|S_IWUSR, show_pwm_enable,
+ store_pwm_enable, 2);
+
+static ssize_t
show_pwm_freq(struct device *dev, struct device_attribute *devattr, char *buf)
{
int nr = to_sensor_dev_attr(devattr)->index;
@@ -1223,6 +1269,11 @@
&sensor_dev_attr_pwm1_freq.dev_attr.attr,
&sensor_dev_attr_pwm2_freq.dev_attr.attr,
&sensor_dev_attr_pwm3_freq.dev_attr.attr,
+
+ &sensor_dev_attr_pwm1_enable.dev_attr.attr,
+ &sensor_dev_attr_pwm2_enable.dev_attr.attr,
+ &sensor_dev_attr_pwm3_enable.dev_attr.attr,
+
NULL
};
@@ -1366,6 +1417,19 @@
&sensor_dev_attr_pwm3_freq.dev_attr)))
goto ERROR4;
+ if (data->type != w83627hf)
+ if ((err = device_create_file(dev,
+ &sensor_dev_attr_pwm1_enable.dev_attr))
+ || (err = device_create_file(dev,
+ &sensor_dev_attr_pwm2_enable.dev_attr)))
+ goto ERROR4;
+
+ if (data->type == w83627thf || data->type == w83637hf
+ || data->type == w83687thf)
+ if ((err = device_create_file(dev,
+ &sensor_dev_attr_pwm3_enable.dev_attr)))
+ goto ERROR4;
+
data->hwmon_dev = hwmon_device_register(dev);
if (IS_ERR(data->hwmon_dev)) {
err = PTR_ERR(data->hwmon_dev);
@@ -1655,6 +1719,7 @@
{
struct w83627hf_data *data = dev_get_drvdata(dev);
int i, num_temps = (data->type == w83697hf) ? 2 : 3;
+ int num_pwms = (data->type == w83697hf) ? 2 : 3;
mutex_lock(&data->update_lock);
@@ -1707,6 +1772,15 @@
break;
}
}
+ if (data->type != w83627hf) {
+ for (i = 0; i < num_pwms; i++) {
+ u8 tmp = w83627hf_read_value(data,
+ W83627THF_REG_PWM_ENABLE[i]);
+ data->pwm_enable[i] =
+ ((tmp >> W83627THF_PWM_ENABLE_SHIFT[i])
+ & 0x03) + 1;
+ }
+ }
for (i = 0; i < num_temps; i++) {
data->temp[i] = w83627hf_read_value(
data, w83627hf_reg_temp[i]);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: w83627hf-pwm-enable.patch
Type: text/x-diff
Size: 4226 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20080520/7df62125/attachment.bin
