Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-06-22 Thread Michał Kępień 
> > >> Tested on a Dell XPS13 9333. Changing the value of pwm1_enable appears
> > >> to have no effect here.
> > > 
> > > Can you check what i8k_enable_fan_auto_mode() returns? Error code or not?
> > 
> > No error. However, independently on the value I set, the BIOS always
> > overrides the speed of the fan.
> 
> Ok, looks like this is model or platform specific call...
> 
> If you want, you can try to play with dellfan source code [1]. There is
> e.g. in comment specified two methods for disabling that BIOS thermal
> automode [2]. Looks like second method is that which I implemented in
> patch...
> 
> [1] - https://github.com/clopez/dellfan
> [2] - https://github.com/clopez/dellfan/blob/master/dellfan.c#L64

Pali, I played around with your patch a bit on my Vostro V131 (BIOS A04)
and the results are similar to Gabriele's:

  * I can write values 1 and 2 to pwm{1,2}_enable, which causes
i8k_enable_fan_auto_mode() to return 0, but the desired speed set
using pwm{1,2} is quickly overridden by the BIOS, no matter which
thermal control mode is set,

  * trying to set the BIOS thermal control mode to the same value it was
already set to causes i8k_enable_fan_auto_mode() to return -22:

# echo 1 > /sys/class/hwmon/hwmon1/pwm1_enable
# echo 1 > /sys/class/hwmon/hwmon1/pwm1_enable
bash: echo: write error: Input/output error
# echo 2 > /sys/class/hwmon/hwmon1/pwm2_enable
# echo 2 > /sys/class/hwmon/hwmon1/pwm2_enable
bash: echo: write error: Input/output error
# echo 1 > /sys/class/hwmon/hwmon1/pwm1_enable
# echo 2 > /sys/class/hwmon/hwmon1/pwm2_enable
#

  * I tried changing I8K_SMM_MANUAL_FAN and I8K_SMM_AUTO_FAN to 0x30a3
and 0x31a3, respectively (according to [2]), but it hasn't changed
anything, i.e. BIOS is taking over shortly after any value is
written to pwm{1,2}.

-- 
Best regards,
Michał Kępień

Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-06-22 Thread Michał Kępień 
> > >> Tested on a Dell XPS13 9333. Changing the value of pwm1_enable appears
> > >> to have no effect here.
> > > 
> > > Can you check what i8k_enable_fan_auto_mode() returns? Error code or not?
> > 
> > No error. However, independently on the value I set, the BIOS always
> > overrides the speed of the fan.
> 
> Ok, looks like this is model or platform specific call...
> 
> If you want, you can try to play with dellfan source code [1]. There is
> e.g. in comment specified two methods for disabling that BIOS thermal
> automode [2]. Looks like second method is that which I implemented in
> patch...
> 
> [1] - https://github.com/clopez/dellfan
> [2] - https://github.com/clopez/dellfan/blob/master/dellfan.c#L64

Pali, I played around with your patch a bit on my Vostro V131 (BIOS A04)
and the results are similar to Gabriele's:

  * I can write values 1 and 2 to pwm{1,2}_enable, which causes
i8k_enable_fan_auto_mode() to return 0, but the desired speed set
using pwm{1,2} is quickly overridden by the BIOS, no matter which
thermal control mode is set,

  * trying to set the BIOS thermal control mode to the same value it was
already set to causes i8k_enable_fan_auto_mode() to return -22:

# echo 1 > /sys/class/hwmon/hwmon1/pwm1_enable
# echo 1 > /sys/class/hwmon/hwmon1/pwm1_enable
bash: echo: write error: Input/output error
# echo 2 > /sys/class/hwmon/hwmon1/pwm2_enable
# echo 2 > /sys/class/hwmon/hwmon1/pwm2_enable
bash: echo: write error: Input/output error
# echo 1 > /sys/class/hwmon/hwmon1/pwm1_enable
# echo 2 > /sys/class/hwmon/hwmon1/pwm2_enable
#

  * I tried changing I8K_SMM_MANUAL_FAN and I8K_SMM_AUTO_FAN to 0x30a3
and 0x31a3, respectively (according to [2]), but it hasn't changed
anything, i.e. BIOS is taking over shortly after any value is
written to pwm{1,2}.

-- 
Best regards,
Michał Kępień

Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-06-02 Thread Pali Rohár 
On Monday 30 May 2016 17:26:36 Gabriele Mazzotta wrote:
> On 30/05/2016 11:32, Pali Rohár wrote:
> > On Friday 27 May 2016 14:11:11 Gabriele Mazzotta wrote:
> >> On 22/05/2016 13:50, Pali Rohár wrote:
> >>> This patch exports standard hwmon pwmX_enable sysfs attribute for enabling
> >>> or disabling automatic fan control by BIOS. Standard value "1" is for
> >>> disabling automatic BIOS fan control and value "2" for enabling.
> >>>
> >>> Currently there is no way to check if BIOS auto mode is enabled (at least
> >>> it is not know how to do it), so hwmon sysfs attribute is write-only.
> >>>
> >>> By default BIOS auto mode is enabled by laptop firmware.
> >>>
> >>> When BIOS auto mode is enabled, custom fan speed value (set via hwmon pwmX
> >>> sysfs attribute) is overwritten by SMM in few seconds and therefore any
> >>> custom settings are without effect. So this is reason why implementing
> >>> option for disabling BIOS auto mode is needed.
> >>>
> >>> So finally this patch allows kernel to set and control fan speed on
> >>> laptops, but it can be dangerous (like setting speed of other fans).
> >>>
> >>> This new feature is highly experimental, uses Dell SMM calls, so does not
> >>> have to be supported by all laptops BIOSes. It was tested on Dell Latitude
> >>> E6440 with BIOS A5.
> >>
> >> Tested on a Dell XPS13 9333. Changing the value of pwm1_enable appears
> >> to have no effect here.
> > 
> > Can you check what i8k_enable_fan_auto_mode() returns? Error code or not?
> 
> No error. However, independently on the value I set, the BIOS always
> overrides the speed of the fan.

Ok, looks like this is model or platform specific call...

If you want, you can try to play with dellfan source code [1]. There is
e.g. in comment specified two methods for disabling that BIOS thermal
automode [2]. Looks like second method is that which I implemented in
patch...

[1] - https://github.com/clopez/dellfan
[2] - https://github.com/clopez/dellfan/blob/master/dellfan.c#L64

-- 
Pali Rohár
pali.ro...@gmail.com

Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-06-02 Thread Pali Rohár 
On Monday 30 May 2016 17:26:36 Gabriele Mazzotta wrote:
> On 30/05/2016 11:32, Pali Rohár wrote:
> > On Friday 27 May 2016 14:11:11 Gabriele Mazzotta wrote:
> >> On 22/05/2016 13:50, Pali Rohár wrote:
> >>> This patch exports standard hwmon pwmX_enable sysfs attribute for enabling
> >>> or disabling automatic fan control by BIOS. Standard value "1" is for
> >>> disabling automatic BIOS fan control and value "2" for enabling.
> >>>
> >>> Currently there is no way to check if BIOS auto mode is enabled (at least
> >>> it is not know how to do it), so hwmon sysfs attribute is write-only.
> >>>
> >>> By default BIOS auto mode is enabled by laptop firmware.
> >>>
> >>> When BIOS auto mode is enabled, custom fan speed value (set via hwmon pwmX
> >>> sysfs attribute) is overwritten by SMM in few seconds and therefore any
> >>> custom settings are without effect. So this is reason why implementing
> >>> option for disabling BIOS auto mode is needed.
> >>>
> >>> So finally this patch allows kernel to set and control fan speed on
> >>> laptops, but it can be dangerous (like setting speed of other fans).
> >>>
> >>> This new feature is highly experimental, uses Dell SMM calls, so does not
> >>> have to be supported by all laptops BIOSes. It was tested on Dell Latitude
> >>> E6440 with BIOS A5.
> >>
> >> Tested on a Dell XPS13 9333. Changing the value of pwm1_enable appears
> >> to have no effect here.
> > 
> > Can you check what i8k_enable_fan_auto_mode() returns? Error code or not?
> 
> No error. However, independently on the value I set, the BIOS always
> overrides the speed of the fan.

Ok, looks like this is model or platform specific call...

If you want, you can try to play with dellfan source code [1]. There is
e.g. in comment specified two methods for disabling that BIOS thermal
automode [2]. Looks like second method is that which I implemented in
patch...

[1] - https://github.com/clopez/dellfan
[2] - https://github.com/clopez/dellfan/blob/master/dellfan.c#L64

-- 
Pali Rohár
pali.ro...@gmail.com

Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-05-30 Thread Gabriele Mazzotta 
On 30/05/2016 11:32, Pali Rohár wrote:
> On Friday 27 May 2016 14:11:11 Gabriele Mazzotta wrote:
>> On 22/05/2016 13:50, Pali Rohár wrote:
>>> This patch exports standard hwmon pwmX_enable sysfs attribute for enabling
>>> or disabling automatic fan control by BIOS. Standard value "1" is for
>>> disabling automatic BIOS fan control and value "2" for enabling.
>>>
>>> Currently there is no way to check if BIOS auto mode is enabled (at least
>>> it is not know how to do it), so hwmon sysfs attribute is write-only.
>>>
>>> By default BIOS auto mode is enabled by laptop firmware.
>>>
>>> When BIOS auto mode is enabled, custom fan speed value (set via hwmon pwmX
>>> sysfs attribute) is overwritten by SMM in few seconds and therefore any
>>> custom settings are without effect. So this is reason why implementing
>>> option for disabling BIOS auto mode is needed.
>>>
>>> So finally this patch allows kernel to set and control fan speed on
>>> laptops, but it can be dangerous (like setting speed of other fans).
>>>
>>> This new feature is highly experimental, uses Dell SMM calls, so does not
>>> have to be supported by all laptops BIOSes. It was tested on Dell Latitude
>>> E6440 with BIOS A5.
>>
>> Tested on a Dell XPS13 9333. Changing the value of pwm1_enable appears
>> to have no effect here.
> 
> Can you check what i8k_enable_fan_auto_mode() returns? Error code or not?

No error. However, independently on the value I set, the BIOS always
overrides the speed of the fan.

Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-05-30 Thread Gabriele Mazzotta 
On 30/05/2016 11:32, Pali Rohár wrote:
> On Friday 27 May 2016 14:11:11 Gabriele Mazzotta wrote:
>> On 22/05/2016 13:50, Pali Rohár wrote:
>>> This patch exports standard hwmon pwmX_enable sysfs attribute for enabling
>>> or disabling automatic fan control by BIOS. Standard value "1" is for
>>> disabling automatic BIOS fan control and value "2" for enabling.
>>>
>>> Currently there is no way to check if BIOS auto mode is enabled (at least
>>> it is not know how to do it), so hwmon sysfs attribute is write-only.
>>>
>>> By default BIOS auto mode is enabled by laptop firmware.
>>>
>>> When BIOS auto mode is enabled, custom fan speed value (set via hwmon pwmX
>>> sysfs attribute) is overwritten by SMM in few seconds and therefore any
>>> custom settings are without effect. So this is reason why implementing
>>> option for disabling BIOS auto mode is needed.
>>>
>>> So finally this patch allows kernel to set and control fan speed on
>>> laptops, but it can be dangerous (like setting speed of other fans).
>>>
>>> This new feature is highly experimental, uses Dell SMM calls, so does not
>>> have to be supported by all laptops BIOSes. It was tested on Dell Latitude
>>> E6440 with BIOS A5.
>>
>> Tested on a Dell XPS13 9333. Changing the value of pwm1_enable appears
>> to have no effect here.
> 
> Can you check what i8k_enable_fan_auto_mode() returns? Error code or not?

No error. However, independently on the value I set, the BIOS always
overrides the speed of the fan.

Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-05-30 Thread Pali Rohár 
On Friday 27 May 2016 14:11:11 Gabriele Mazzotta wrote:
> On 22/05/2016 13:50, Pali Rohár wrote:
> > This patch exports standard hwmon pwmX_enable sysfs attribute for enabling
> > or disabling automatic fan control by BIOS. Standard value "1" is for
> > disabling automatic BIOS fan control and value "2" for enabling.
> > 
> > Currently there is no way to check if BIOS auto mode is enabled (at least
> > it is not know how to do it), so hwmon sysfs attribute is write-only.
> > 
> > By default BIOS auto mode is enabled by laptop firmware.
> > 
> > When BIOS auto mode is enabled, custom fan speed value (set via hwmon pwmX
> > sysfs attribute) is overwritten by SMM in few seconds and therefore any
> > custom settings are without effect. So this is reason why implementing
> > option for disabling BIOS auto mode is needed.
> > 
> > So finally this patch allows kernel to set and control fan speed on
> > laptops, but it can be dangerous (like setting speed of other fans).
> > 
> > This new feature is highly experimental, uses Dell SMM calls, so does not
> > have to be supported by all laptops BIOSes. It was tested on Dell Latitude
> > E6440 with BIOS A5.
> 
> Tested on a Dell XPS13 9333. Changing the value of pwm1_enable appears
> to have no effect here.

Can you check what i8k_enable_fan_auto_mode() returns? Error code or not?

-- 
Pali Rohár
pali.ro...@gmail.com

Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-05-30 Thread Pali Rohár 
On Friday 27 May 2016 14:11:11 Gabriele Mazzotta wrote:
> On 22/05/2016 13:50, Pali Rohár wrote:
> > This patch exports standard hwmon pwmX_enable sysfs attribute for enabling
> > or disabling automatic fan control by BIOS. Standard value "1" is for
> > disabling automatic BIOS fan control and value "2" for enabling.
> > 
> > Currently there is no way to check if BIOS auto mode is enabled (at least
> > it is not know how to do it), so hwmon sysfs attribute is write-only.
> > 
> > By default BIOS auto mode is enabled by laptop firmware.
> > 
> > When BIOS auto mode is enabled, custom fan speed value (set via hwmon pwmX
> > sysfs attribute) is overwritten by SMM in few seconds and therefore any
> > custom settings are without effect. So this is reason why implementing
> > option for disabling BIOS auto mode is needed.
> > 
> > So finally this patch allows kernel to set and control fan speed on
> > laptops, but it can be dangerous (like setting speed of other fans).
> > 
> > This new feature is highly experimental, uses Dell SMM calls, so does not
> > have to be supported by all laptops BIOSes. It was tested on Dell Latitude
> > E6440 with BIOS A5.
> 
> Tested on a Dell XPS13 9333. Changing the value of pwm1_enable appears
> to have no effect here.

Can you check what i8k_enable_fan_auto_mode() returns? Error code or not?

-- 
Pali Rohár
pali.ro...@gmail.com

Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-05-27 Thread Gabriele Mazzotta 
On 22/05/2016 13:50, Pali Rohár wrote:
> This patch exports standard hwmon pwmX_enable sysfs attribute for enabling
> or disabling automatic fan control by BIOS. Standard value "1" is for
> disabling automatic BIOS fan control and value "2" for enabling.
> 
> Currently there is no way to check if BIOS auto mode is enabled (at least
> it is not know how to do it), so hwmon sysfs attribute is write-only.
> 
> By default BIOS auto mode is enabled by laptop firmware.
> 
> When BIOS auto mode is enabled, custom fan speed value (set via hwmon pwmX
> sysfs attribute) is overwritten by SMM in few seconds and therefore any
> custom settings are without effect. So this is reason why implementing
> option for disabling BIOS auto mode is needed.
> 
> So finally this patch allows kernel to set and control fan speed on
> laptops, but it can be dangerous (like setting speed of other fans).
> 
> This new feature is highly experimental, uses Dell SMM calls, so does not
> have to be supported by all laptops BIOSes. It was tested on Dell Latitude
> E6440 with BIOS A5.

Tested on a Dell XPS13 9333. Changing the value of pwm1_enable appears
to have no effect here.

> Signed-off-by: Pali Rohár 
> ---
>  drivers/hwmon/dell-smm-hwmon.c |   64 
> ++--
>  1 file changed, 55 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 8a66a42..7b5144a 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -45,6 +45,8 @@
>  #define I8K_SMM_GET_SPEED0x02a3
>  #define I8K_SMM_GET_FAN_TYPE 0x03a3
>  #define I8K_SMM_GET_NOM_SPEED0x04a3
> +#define I8K_SMM_MANUAL_FAN   0x34a3
> +#define I8K_SMM_AUTO_FAN 0x35a3
>  #define I8K_SMM_GET_TEMP 0x10a3
>  #define I8K_SMM_GET_TEMP_TYPE0x11a3
>  #define I8K_SMM_GET_DELL_SIG10xfea3
> @@ -284,6 +286,17 @@ static int i8k_get_fan_nominal_speed(int fan, int speed)
>  }
>  
>  /*
> + * Enable or disable automatic BIOS fan control support
> + */
> +static int i8k_enable_fan_auto_mode(bool enable)
> +{
> + struct smm_regs regs = { };
> +
> + regs.eax = enable ? I8K_SMM_AUTO_FAN : I8K_SMM_MANUAL_FAN;
> + return i8k_smm();
> +}
> +
> +/*
>   * Set the fan speed (off, low, high). Returns the new fan status.
>   */
>  static int i8k_set_fan(int fan, int speed)
> @@ -702,6 +715,30 @@ static ssize_t i8k_hwmon_set_pwm(struct device *dev,
>   return err < 0 ? -EIO : count;
>  }
>  
> +static ssize_t i8k_hwmon_set_pwm_enable(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int err;
> + bool enable;
> + unsigned long val;
> +
> + err = kstrtoul(buf, 10, );
> + if (err)
> + return err;
> +
> + if (val == 0)
> + return -EINVAL;
> +
> + enable = (val != 1);
> +
> + mutex_lock(_mutex);
> + err = i8k_enable_fan_auto_mode(enable);
> + mutex_unlock(_mutex);
> +
> + return err ? -EIO : count;
> +}
> +
>  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, i8k_hwmon_show_temp, NULL, 
> 0);
>  static SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO, i8k_hwmon_show_temp_label, 
> NULL,
> 0);
> @@ -719,18 +756,24 @@ static SENSOR_DEVICE_ATTR(fan1_label, S_IRUGO, 
> i8k_hwmon_show_fan_label, NULL,
> 0);
>  static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
> i8k_hwmon_set_pwm, 0);
> +static SENSOR_DEVICE_ATTR(pwm1_enable, S_IWUSR, NULL, 
> i8k_hwmon_set_pwm_enable,
> +   0);
>  static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
> 1);
>  static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, i8k_hwmon_show_fan_label, 
> NULL,
> 1);
>  static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
> i8k_hwmon_set_pwm, 1);
> +static SENSOR_DEVICE_ATTR(pwm2_enable, S_IWUSR, NULL, 
> i8k_hwmon_set_pwm_enable,
> +   1);
>  static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
> 2);
>  static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, i8k_hwmon_show_fan_label, 
> NULL,
> 2);
>  static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
> i8k_hwmon_set_pwm, 2);
> +static SENSOR_DEVICE_ATTR(pwm3_enable, S_IWUSR, NULL, 
> i8k_hwmon_set_pwm_enable,
> +   2);
>  
>  static struct attribute *i8k_attrs[] = {
>   _dev_attr_temp1_input.dev_attr.attr, /* 0 */
> @@ -744,12 +787,15 @@ static struct attribute *i8k_attrs[] = {
>   _dev_attr_fan1_input.dev_attr.attr,  /* 8 */
>   _dev_attr_fan1_label.dev_attr.attr,  /* 9 */
>   _dev_attr_pwm1.dev_attr.attr,/* 10 */
> -

Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-05-27 Thread Gabriele Mazzotta 
On 22/05/2016 13:50, Pali Rohár wrote:
> This patch exports standard hwmon pwmX_enable sysfs attribute for enabling
> or disabling automatic fan control by BIOS. Standard value "1" is for
> disabling automatic BIOS fan control and value "2" for enabling.
> 
> Currently there is no way to check if BIOS auto mode is enabled (at least
> it is not know how to do it), so hwmon sysfs attribute is write-only.
> 
> By default BIOS auto mode is enabled by laptop firmware.
> 
> When BIOS auto mode is enabled, custom fan speed value (set via hwmon pwmX
> sysfs attribute) is overwritten by SMM in few seconds and therefore any
> custom settings are without effect. So this is reason why implementing
> option for disabling BIOS auto mode is needed.
> 
> So finally this patch allows kernel to set and control fan speed on
> laptops, but it can be dangerous (like setting speed of other fans).
> 
> This new feature is highly experimental, uses Dell SMM calls, so does not
> have to be supported by all laptops BIOSes. It was tested on Dell Latitude
> E6440 with BIOS A5.

Tested on a Dell XPS13 9333. Changing the value of pwm1_enable appears
to have no effect here.

> Signed-off-by: Pali Rohár 
> ---
>  drivers/hwmon/dell-smm-hwmon.c |   64 
> ++--
>  1 file changed, 55 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
> index 8a66a42..7b5144a 100644
> --- a/drivers/hwmon/dell-smm-hwmon.c
> +++ b/drivers/hwmon/dell-smm-hwmon.c
> @@ -45,6 +45,8 @@
>  #define I8K_SMM_GET_SPEED0x02a3
>  #define I8K_SMM_GET_FAN_TYPE 0x03a3
>  #define I8K_SMM_GET_NOM_SPEED0x04a3
> +#define I8K_SMM_MANUAL_FAN   0x34a3
> +#define I8K_SMM_AUTO_FAN 0x35a3
>  #define I8K_SMM_GET_TEMP 0x10a3
>  #define I8K_SMM_GET_TEMP_TYPE0x11a3
>  #define I8K_SMM_GET_DELL_SIG10xfea3
> @@ -284,6 +286,17 @@ static int i8k_get_fan_nominal_speed(int fan, int speed)
>  }
>  
>  /*
> + * Enable or disable automatic BIOS fan control support
> + */
> +static int i8k_enable_fan_auto_mode(bool enable)
> +{
> + struct smm_regs regs = { };
> +
> + regs.eax = enable ? I8K_SMM_AUTO_FAN : I8K_SMM_MANUAL_FAN;
> + return i8k_smm();
> +}
> +
> +/*
>   * Set the fan speed (off, low, high). Returns the new fan status.
>   */
>  static int i8k_set_fan(int fan, int speed)
> @@ -702,6 +715,30 @@ static ssize_t i8k_hwmon_set_pwm(struct device *dev,
>   return err < 0 ? -EIO : count;
>  }
>  
> +static ssize_t i8k_hwmon_set_pwm_enable(struct device *dev,
> + struct device_attribute *attr,
> + const char *buf, size_t count)
> +{
> + int err;
> + bool enable;
> + unsigned long val;
> +
> + err = kstrtoul(buf, 10, );
> + if (err)
> + return err;
> +
> + if (val == 0)
> + return -EINVAL;
> +
> + enable = (val != 1);
> +
> + mutex_lock(_mutex);
> + err = i8k_enable_fan_auto_mode(enable);
> + mutex_unlock(_mutex);
> +
> + return err ? -EIO : count;
> +}
> +
>  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, i8k_hwmon_show_temp, NULL, 
> 0);
>  static SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO, i8k_hwmon_show_temp_label, 
> NULL,
> 0);
> @@ -719,18 +756,24 @@ static SENSOR_DEVICE_ATTR(fan1_label, S_IRUGO, 
> i8k_hwmon_show_fan_label, NULL,
> 0);
>  static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
> i8k_hwmon_set_pwm, 0);
> +static SENSOR_DEVICE_ATTR(pwm1_enable, S_IWUSR, NULL, 
> i8k_hwmon_set_pwm_enable,
> +   0);
>  static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
> 1);
>  static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, i8k_hwmon_show_fan_label, 
> NULL,
> 1);
>  static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
> i8k_hwmon_set_pwm, 1);
> +static SENSOR_DEVICE_ATTR(pwm2_enable, S_IWUSR, NULL, 
> i8k_hwmon_set_pwm_enable,
> +   1);
>  static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
> 2);
>  static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, i8k_hwmon_show_fan_label, 
> NULL,
> 2);
>  static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
> i8k_hwmon_set_pwm, 2);
> +static SENSOR_DEVICE_ATTR(pwm3_enable, S_IWUSR, NULL, 
> i8k_hwmon_set_pwm_enable,
> +   2);
>  
>  static struct attribute *i8k_attrs[] = {
>   _dev_attr_temp1_input.dev_attr.attr, /* 0 */
> @@ -744,12 +787,15 @@ static struct attribute *i8k_attrs[] = {
>   _dev_attr_fan1_input.dev_attr.attr,  /* 8 */
>   _dev_attr_fan1_label.dev_attr.attr,  /* 9 */
>   _dev_attr_pwm1.dev_attr.attr,/* 10 */
> - _dev_attr_fan2_input.dev_attr.attr,  /* 11

Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-05-22 Thread Guenter Roeck 
On Sun, May 22, 2016 at 05:17:26PM +0200, Pali Rohár wrote:
> On Sunday 22 May 2016 17:04:16 Guenter Roeck wrote:
> > On 05/22/2016 04:50 AM, Pali Rohár wrote:
> > > This patch exports standard hwmon pwmX_enable sysfs attribute for
> > > enabling or disabling automatic fan control by BIOS. Standard
> > > value "1" is for disabling automatic BIOS fan control and value
> > > "2" for enabling.
> > > 
> > > Currently there is no way to check if BIOS auto mode is enabled (at
> > > least it is not know how to do it), so hwmon sysfs attribute is
> > > write-only.
> > 
> > You could cache the mode and report it after it was set, and define
> > '0' as read value for 'unknown'.
> 
> I was thinking about it, but there is problem, that userspace can also 
> update/change BIOS auto mode. Process with CAP_SYS_RAWIO can use 
> sys_iopl(3) or sys_ioperm() which allows it access to I/O ports. There 
> are already different i8k* userspace tools which do that. And once 
> userspace change BIOS auto mode, then cached value in kernel does not 
> have to be correct and can confuse other applications which use standard 
> hwmon interface (not direct I/O ports).
> 
> And value '0' is already defined in hwmon API as:
> "no fan speed control (i.e. fan at full speed)"
> 
Ok.

> > > +static ssize_t i8k_hwmon_set_pwm_enable(struct device *dev,
> > > + struct device_attribute *attr,
> > > + const char *buf, size_t count)
> > > +{
> > > + int err;
> > > + bool enable;
> > > + unsigned long val;
> > > +
> > > + err = kstrtoul(buf, 10, );
> > > + if (err)
> > > + return err;
> > > +
> > > + if (val == 0)
> > > + return -EINVAL;
> > > +
> > > + enable = (val != 1);
> > > +
> > 
> > Please accepted only explicit values (1 and 2).
> 
> In Documentation/hwmon/sysfs-interface is written:
> 
> pwm[1-*]_enable
>   2+: automatic fan speed control enabled
> 
> And this reason why my patch accept also other numeric values.
> 
> If other values does not make sense, maybe update documentation file?
> 

The idea behind 2+ is that there can be multiple automatic fan speed modes.
Which modes are actually supported, and what those modes mean, is then
commonly described in the driver documentation.

It does not mean (or is not supposed to mean) that random values shall
be accepted.

Of course that assumes that a driver documentation actually exists,
which AFAICS is not the case for dell-smm-hwmon. Maybe it would make
sense to add one ?

Thanks,
Guenter

Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-05-22 Thread Guenter Roeck 
On Sun, May 22, 2016 at 05:17:26PM +0200, Pali Rohár wrote:
> On Sunday 22 May 2016 17:04:16 Guenter Roeck wrote:
> > On 05/22/2016 04:50 AM, Pali Rohár wrote:
> > > This patch exports standard hwmon pwmX_enable sysfs attribute for
> > > enabling or disabling automatic fan control by BIOS. Standard
> > > value "1" is for disabling automatic BIOS fan control and value
> > > "2" for enabling.
> > > 
> > > Currently there is no way to check if BIOS auto mode is enabled (at
> > > least it is not know how to do it), so hwmon sysfs attribute is
> > > write-only.
> > 
> > You could cache the mode and report it after it was set, and define
> > '0' as read value for 'unknown'.
> 
> I was thinking about it, but there is problem, that userspace can also 
> update/change BIOS auto mode. Process with CAP_SYS_RAWIO can use 
> sys_iopl(3) or sys_ioperm() which allows it access to I/O ports. There 
> are already different i8k* userspace tools which do that. And once 
> userspace change BIOS auto mode, then cached value in kernel does not 
> have to be correct and can confuse other applications which use standard 
> hwmon interface (not direct I/O ports).
> 
> And value '0' is already defined in hwmon API as:
> "no fan speed control (i.e. fan at full speed)"
> 
Ok.

> > > +static ssize_t i8k_hwmon_set_pwm_enable(struct device *dev,
> > > + struct device_attribute *attr,
> > > + const char *buf, size_t count)
> > > +{
> > > + int err;
> > > + bool enable;
> > > + unsigned long val;
> > > +
> > > + err = kstrtoul(buf, 10, );
> > > + if (err)
> > > + return err;
> > > +
> > > + if (val == 0)
> > > + return -EINVAL;
> > > +
> > > + enable = (val != 1);
> > > +
> > 
> > Please accepted only explicit values (1 and 2).
> 
> In Documentation/hwmon/sysfs-interface is written:
> 
> pwm[1-*]_enable
>   2+: automatic fan speed control enabled
> 
> And this reason why my patch accept also other numeric values.
> 
> If other values does not make sense, maybe update documentation file?
> 

The idea behind 2+ is that there can be multiple automatic fan speed modes.
Which modes are actually supported, and what those modes mean, is then
commonly described in the driver documentation.

It does not mean (or is not supposed to mean) that random values shall
be accepted.

Of course that assumes that a driver documentation actually exists,
which AFAICS is not the case for dell-smm-hwmon. Maybe it would make
sense to add one ?

Thanks,
Guenter

Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-05-22 Thread Pali Rohár 
On Sunday 22 May 2016 17:04:16 Guenter Roeck wrote:
> On 05/22/2016 04:50 AM, Pali Rohár wrote:
> > This patch exports standard hwmon pwmX_enable sysfs attribute for
> > enabling or disabling automatic fan control by BIOS. Standard
> > value "1" is for disabling automatic BIOS fan control and value
> > "2" for enabling.
> > 
> > Currently there is no way to check if BIOS auto mode is enabled (at
> > least it is not know how to do it), so hwmon sysfs attribute is
> > write-only.
> 
> You could cache the mode and report it after it was set, and define
> '0' as read value for 'unknown'.

I was thinking about it, but there is problem, that userspace can also 
update/change BIOS auto mode. Process with CAP_SYS_RAWIO can use 
sys_iopl(3) or sys_ioperm() which allows it access to I/O ports. There 
are already different i8k* userspace tools which do that. And once 
userspace change BIOS auto mode, then cached value in kernel does not 
have to be correct and can confuse other applications which use standard 
hwmon interface (not direct I/O ports).

And value '0' is already defined in hwmon API as:
"no fan speed control (i.e. fan at full speed)"

> > By default BIOS auto mode is enabled by laptop firmware.
> > 
> > When BIOS auto mode is enabled, custom fan speed value (set via
> > hwmon pwmX sysfs attribute) is overwritten by SMM in few seconds
> > and therefore any custom settings are without effect. So this is
> > reason why implementing option for disabling BIOS auto mode is
> > needed.
> > 
> > So finally this patch allows kernel to set and control fan speed on
> > laptops, but it can be dangerous (like setting speed of other
> > fans).
> > 
> > This new feature is highly experimental, uses Dell SMM calls, so
> > does not have to be supported by all laptops BIOSes. It was tested
> > on Dell Latitude E6440 with BIOS A5.
> > 
> > Signed-off-by: Pali Rohár 
> > ---
> > 
> >   drivers/hwmon/dell-smm-hwmon.c |   64
> >   ++-- 1 file changed, 55
> >   insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/hwmon/dell-smm-hwmon.c
> > b/drivers/hwmon/dell-smm-hwmon.c index 8a66a42..7b5144a 100644
> > --- a/drivers/hwmon/dell-smm-hwmon.c
> > +++ b/drivers/hwmon/dell-smm-hwmon.c
> > @@ -45,6 +45,8 @@
> > 
> >   #define I8K_SMM_GET_SPEED 0x02a3
> >   #define I8K_SMM_GET_FAN_TYPE  0x03a3
> >   #define I8K_SMM_GET_NOM_SPEED 0x04a3
> > 
> > +#define I8K_SMM_MANUAL_FAN 0x34a3
> > +#define I8K_SMM_AUTO_FAN   0x35a3
> > 
> >   #define I8K_SMM_GET_TEMP  0x10a3
> >   #define I8K_SMM_GET_TEMP_TYPE 0x11a3
> >   #define I8K_SMM_GET_DELL_SIG1 0xfea3
> > 
> > @@ -284,6 +286,17 @@ static int i8k_get_fan_nominal_speed(int fan,
> > int speed)
> > 
> >   }
> >   
> >   /*
> > 
> > + * Enable or disable automatic BIOS fan control support
> > + */
> > +static int i8k_enable_fan_auto_mode(bool enable)
> > +{
> > +   struct smm_regs regs = { };
> > +
> > +   regs.eax = enable ? I8K_SMM_AUTO_FAN : I8K_SMM_MANUAL_FAN;
> > +   return i8k_smm();
> > +}
> > +
> > +/*
> > 
> >* Set the fan speed (off, low, high). Returns the new fan
> >status. */
> >   
> >   static int i8k_set_fan(int fan, int speed)
> > 
> > @@ -702,6 +715,30 @@ static ssize_t i8k_hwmon_set_pwm(struct device
> > *dev,
> > 
> > return err < 0 ? -EIO : count;
> >   
> >   }
> > 
> > +static ssize_t i8k_hwmon_set_pwm_enable(struct device *dev,
> > +   struct device_attribute *attr,
> > +   const char *buf, size_t count)
> > +{
> > +   int err;
> > +   bool enable;
> > +   unsigned long val;
> > +
> > +   err = kstrtoul(buf, 10, );
> > +   if (err)
> > +   return err;
> > +
> > +   if (val == 0)
> > +   return -EINVAL;
> > +
> > +   enable = (val != 1);
> > +
> 
> Please accepted only explicit values (1 and 2).

In Documentation/hwmon/sysfs-interface is written:

pwm[1-*]_enable
2+: automatic fan speed control enabled

And this reason why my patch accept also other numeric values.

If other values does not make sense, maybe update documentation file?

> Thanks,
> Guenter
> 
> > +   mutex_lock(_mutex);
> > +   err = i8k_enable_fan_auto_mode(enable);
> > +   mutex_unlock(_mutex);
> > +
> > +   return err ? -EIO : count;
> > +}
> > +
> > 
> >   static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
> >   i8k_hwmon_show_temp, NULL, 0); static
> >   SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO,
> >   i8k_hwmon_show_temp_label, NULL,
> >   
> >   0);
> > 
> > @@ -719,18 +756,24 @@ static SENSOR_DEVICE_ATTR(fan1_label,
> > S_IRUGO, i8k_hwmon_show_fan_label, NULL,
> > 
> >   0);
> >   
> >   static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR,
> >   i8k_hwmon_show_pwm,
> >   
> >   i8k_hwmon_set_pwm, 0);
> > 
> > +static SENSOR_DEVICE_ATTR(pwm1_enable, S_IWUSR, NULL,
> > i8k_hwmon_set_pwm_enable, +   0);
> > 
> >   static

Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-05-22 Thread Pali Rohár 
On Sunday 22 May 2016 17:04:16 Guenter Roeck wrote:
> On 05/22/2016 04:50 AM, Pali Rohár wrote:
> > This patch exports standard hwmon pwmX_enable sysfs attribute for
> > enabling or disabling automatic fan control by BIOS. Standard
> > value "1" is for disabling automatic BIOS fan control and value
> > "2" for enabling.
> > 
> > Currently there is no way to check if BIOS auto mode is enabled (at
> > least it is not know how to do it), so hwmon sysfs attribute is
> > write-only.
> 
> You could cache the mode and report it after it was set, and define
> '0' as read value for 'unknown'.

I was thinking about it, but there is problem, that userspace can also 
update/change BIOS auto mode. Process with CAP_SYS_RAWIO can use 
sys_iopl(3) or sys_ioperm() which allows it access to I/O ports. There 
are already different i8k* userspace tools which do that. And once 
userspace change BIOS auto mode, then cached value in kernel does not 
have to be correct and can confuse other applications which use standard 
hwmon interface (not direct I/O ports).

And value '0' is already defined in hwmon API as:
"no fan speed control (i.e. fan at full speed)"

> > By default BIOS auto mode is enabled by laptop firmware.
> > 
> > When BIOS auto mode is enabled, custom fan speed value (set via
> > hwmon pwmX sysfs attribute) is overwritten by SMM in few seconds
> > and therefore any custom settings are without effect. So this is
> > reason why implementing option for disabling BIOS auto mode is
> > needed.
> > 
> > So finally this patch allows kernel to set and control fan speed on
> > laptops, but it can be dangerous (like setting speed of other
> > fans).
> > 
> > This new feature is highly experimental, uses Dell SMM calls, so
> > does not have to be supported by all laptops BIOSes. It was tested
> > on Dell Latitude E6440 with BIOS A5.
> > 
> > Signed-off-by: Pali Rohár 
> > ---
> > 
> >   drivers/hwmon/dell-smm-hwmon.c |   64
> >   ++-- 1 file changed, 55
> >   insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/hwmon/dell-smm-hwmon.c
> > b/drivers/hwmon/dell-smm-hwmon.c index 8a66a42..7b5144a 100644
> > --- a/drivers/hwmon/dell-smm-hwmon.c
> > +++ b/drivers/hwmon/dell-smm-hwmon.c
> > @@ -45,6 +45,8 @@
> > 
> >   #define I8K_SMM_GET_SPEED 0x02a3
> >   #define I8K_SMM_GET_FAN_TYPE  0x03a3
> >   #define I8K_SMM_GET_NOM_SPEED 0x04a3
> > 
> > +#define I8K_SMM_MANUAL_FAN 0x34a3
> > +#define I8K_SMM_AUTO_FAN   0x35a3
> > 
> >   #define I8K_SMM_GET_TEMP  0x10a3
> >   #define I8K_SMM_GET_TEMP_TYPE 0x11a3
> >   #define I8K_SMM_GET_DELL_SIG1 0xfea3
> > 
> > @@ -284,6 +286,17 @@ static int i8k_get_fan_nominal_speed(int fan,
> > int speed)
> > 
> >   }
> >   
> >   /*
> > 
> > + * Enable or disable automatic BIOS fan control support
> > + */
> > +static int i8k_enable_fan_auto_mode(bool enable)
> > +{
> > +   struct smm_regs regs = { };
> > +
> > +   regs.eax = enable ? I8K_SMM_AUTO_FAN : I8K_SMM_MANUAL_FAN;
> > +   return i8k_smm();
> > +}
> > +
> > +/*
> > 
> >* Set the fan speed (off, low, high). Returns the new fan
> >status. */
> >   
> >   static int i8k_set_fan(int fan, int speed)
> > 
> > @@ -702,6 +715,30 @@ static ssize_t i8k_hwmon_set_pwm(struct device
> > *dev,
> > 
> > return err < 0 ? -EIO : count;
> >   
> >   }
> > 
> > +static ssize_t i8k_hwmon_set_pwm_enable(struct device *dev,
> > +   struct device_attribute *attr,
> > +   const char *buf, size_t count)
> > +{
> > +   int err;
> > +   bool enable;
> > +   unsigned long val;
> > +
> > +   err = kstrtoul(buf, 10, );
> > +   if (err)
> > +   return err;
> > +
> > +   if (val == 0)
> > +   return -EINVAL;
> > +
> > +   enable = (val != 1);
> > +
> 
> Please accepted only explicit values (1 and 2).

In Documentation/hwmon/sysfs-interface is written:

pwm[1-*]_enable
2+: automatic fan speed control enabled

And this reason why my patch accept also other numeric values.

If other values does not make sense, maybe update documentation file?

> Thanks,
> Guenter
> 
> > +   mutex_lock(_mutex);
> > +   err = i8k_enable_fan_auto_mode(enable);
> > +   mutex_unlock(_mutex);
> > +
> > +   return err ? -EIO : count;
> > +}
> > +
> > 
> >   static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO,
> >   i8k_hwmon_show_temp, NULL, 0); static
> >   SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO,
> >   i8k_hwmon_show_temp_label, NULL,
> >   
> >   0);
> > 
> > @@ -719,18 +756,24 @@ static SENSOR_DEVICE_ATTR(fan1_label,
> > S_IRUGO, i8k_hwmon_show_fan_label, NULL,
> > 
> >   0);
> >   
> >   static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR,
> >   i8k_hwmon_show_pwm,
> >   
> >   i8k_hwmon_set_pwm, 0);
> > 
> > +static SENSOR_DEVICE_ATTR(pwm1_enable, S_IWUSR, NULL,
> > i8k_hwmon_set_pwm_enable, +   0);
> > 
> >   static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO,
> >

Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-05-22 Thread Guenter Roeck 

On 05/22/2016 04:50 AM, Pali Rohár wrote:

This patch exports standard hwmon pwmX_enable sysfs attribute for enabling
or disabling automatic fan control by BIOS. Standard value "1" is for
disabling automatic BIOS fan control and value "2" for enabling.

Currently there is no way to check if BIOS auto mode is enabled (at least
it is not know how to do it), so hwmon sysfs attribute is write-only.


You could cache the mode and report it after it was set, and define '0'
as read value for 'unknown'.


By default BIOS auto mode is enabled by laptop firmware.

When BIOS auto mode is enabled, custom fan speed value (set via hwmon pwmX
sysfs attribute) is overwritten by SMM in few seconds and therefore any
custom settings are without effect. So this is reason why implementing
option for disabling BIOS auto mode is needed.

So finally this patch allows kernel to set and control fan speed on
laptops, but it can be dangerous (like setting speed of other fans).

This new feature is highly experimental, uses Dell SMM calls, so does not
have to be supported by all laptops BIOSes. It was tested on Dell Latitude
E6440 with BIOS A5.

Signed-off-by: Pali Rohár 
---
  drivers/hwmon/dell-smm-hwmon.c |   64 ++--
  1 file changed, 55 insertions(+), 9 deletions(-)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 8a66a42..7b5144a 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -45,6 +45,8 @@
  #define I8K_SMM_GET_SPEED 0x02a3
  #define I8K_SMM_GET_FAN_TYPE  0x03a3
  #define I8K_SMM_GET_NOM_SPEED 0x04a3
+#define I8K_SMM_MANUAL_FAN 0x34a3
+#define I8K_SMM_AUTO_FAN   0x35a3
  #define I8K_SMM_GET_TEMP  0x10a3
  #define I8K_SMM_GET_TEMP_TYPE 0x11a3
  #define I8K_SMM_GET_DELL_SIG1 0xfea3
@@ -284,6 +286,17 @@ static int i8k_get_fan_nominal_speed(int fan, int speed)
  }

  /*
+ * Enable or disable automatic BIOS fan control support
+ */
+static int i8k_enable_fan_auto_mode(bool enable)
+{
+   struct smm_regs regs = { };
+
+   regs.eax = enable ? I8K_SMM_AUTO_FAN : I8K_SMM_MANUAL_FAN;
+   return i8k_smm();
+}
+
+/*
   * Set the fan speed (off, low, high). Returns the new fan status.
   */
  static int i8k_set_fan(int fan, int speed)
@@ -702,6 +715,30 @@ static ssize_t i8k_hwmon_set_pwm(struct device *dev,
return err < 0 ? -EIO : count;
  }

+static ssize_t i8k_hwmon_set_pwm_enable(struct device *dev,
+   struct device_attribute *attr,
+   const char *buf, size_t count)
+{
+   int err;
+   bool enable;
+   unsigned long val;
+
+   err = kstrtoul(buf, 10, );
+   if (err)
+   return err;
+
+   if (val == 0)
+   return -EINVAL;
+
+   enable = (val != 1);
+

Please accepted only explicit values (1 and 2).

Thanks,
Guenter


+   mutex_lock(_mutex);
+   err = i8k_enable_fan_auto_mode(enable);
+   mutex_unlock(_mutex);
+
+   return err ? -EIO : count;
+}
+
  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, i8k_hwmon_show_temp, NULL, 0);
  static SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO, i8k_hwmon_show_temp_label, 
NULL,
  0);
@@ -719,18 +756,24 @@ static SENSOR_DEVICE_ATTR(fan1_label, S_IRUGO, 
i8k_hwmon_show_fan_label, NULL,
  0);
  static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
  i8k_hwmon_set_pwm, 0);
+static SENSOR_DEVICE_ATTR(pwm1_enable, S_IWUSR, NULL, i8k_hwmon_set_pwm_enable,
+ 0);
  static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
  1);
  static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL,
  1);
  static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
  i8k_hwmon_set_pwm, 1);
+static SENSOR_DEVICE_ATTR(pwm2_enable, S_IWUSR, NULL, i8k_hwmon_set_pwm_enable,
+ 1);
  static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
  2);
  static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL,
  2);
  static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
  i8k_hwmon_set_pwm, 2);
+static SENSOR_DEVICE_ATTR(pwm3_enable, S_IWUSR, NULL, i8k_hwmon_set_pwm_enable,
+ 2);

  static struct attribute *i8k_attrs[] = {
_dev_attr_temp1_input.dev_attr.attr, /* 0 */
@@ -744,12 +787,15 @@ static struct attribute *i8k_attrs[] = {
_dev_attr_fan1_input.dev_attr.attr,  /* 8 */
_dev_attr_fan1_label.dev_attr.attr,  /* 9 */
_dev_attr_pwm1.dev_attr.attr,/* 10 */
-   _dev_attr_fan2_input.dev_attr.attr,  /* 11 */
-   _dev_attr_fan2_label.dev_attr.attr,  /* 12 */
-

Re: [Experimental PATCH] dell-smm-hwmon: Add support for disabling automatic BIOS fan control

2016-05-22 Thread Guenter Roeck 

On 05/22/2016 04:50 AM, Pali Rohár wrote:

This patch exports standard hwmon pwmX_enable sysfs attribute for enabling
or disabling automatic fan control by BIOS. Standard value "1" is for
disabling automatic BIOS fan control and value "2" for enabling.

Currently there is no way to check if BIOS auto mode is enabled (at least
it is not know how to do it), so hwmon sysfs attribute is write-only.


You could cache the mode and report it after it was set, and define '0'
as read value for 'unknown'.


By default BIOS auto mode is enabled by laptop firmware.

When BIOS auto mode is enabled, custom fan speed value (set via hwmon pwmX
sysfs attribute) is overwritten by SMM in few seconds and therefore any
custom settings are without effect. So this is reason why implementing
option for disabling BIOS auto mode is needed.

So finally this patch allows kernel to set and control fan speed on
laptops, but it can be dangerous (like setting speed of other fans).

This new feature is highly experimental, uses Dell SMM calls, so does not
have to be supported by all laptops BIOSes. It was tested on Dell Latitude
E6440 with BIOS A5.

Signed-off-by: Pali Rohár 
---
  drivers/hwmon/dell-smm-hwmon.c |   64 ++--
  1 file changed, 55 insertions(+), 9 deletions(-)

diff --git a/drivers/hwmon/dell-smm-hwmon.c b/drivers/hwmon/dell-smm-hwmon.c
index 8a66a42..7b5144a 100644
--- a/drivers/hwmon/dell-smm-hwmon.c
+++ b/drivers/hwmon/dell-smm-hwmon.c
@@ -45,6 +45,8 @@
  #define I8K_SMM_GET_SPEED 0x02a3
  #define I8K_SMM_GET_FAN_TYPE  0x03a3
  #define I8K_SMM_GET_NOM_SPEED 0x04a3
+#define I8K_SMM_MANUAL_FAN 0x34a3
+#define I8K_SMM_AUTO_FAN   0x35a3
  #define I8K_SMM_GET_TEMP  0x10a3
  #define I8K_SMM_GET_TEMP_TYPE 0x11a3
  #define I8K_SMM_GET_DELL_SIG1 0xfea3
@@ -284,6 +286,17 @@ static int i8k_get_fan_nominal_speed(int fan, int speed)
  }

  /*
+ * Enable or disable automatic BIOS fan control support
+ */
+static int i8k_enable_fan_auto_mode(bool enable)
+{
+   struct smm_regs regs = { };
+
+   regs.eax = enable ? I8K_SMM_AUTO_FAN : I8K_SMM_MANUAL_FAN;
+   return i8k_smm();
+}
+
+/*
   * Set the fan speed (off, low, high). Returns the new fan status.
   */
  static int i8k_set_fan(int fan, int speed)
@@ -702,6 +715,30 @@ static ssize_t i8k_hwmon_set_pwm(struct device *dev,
return err < 0 ? -EIO : count;
  }

+static ssize_t i8k_hwmon_set_pwm_enable(struct device *dev,
+   struct device_attribute *attr,
+   const char *buf, size_t count)
+{
+   int err;
+   bool enable;
+   unsigned long val;
+
+   err = kstrtoul(buf, 10, );
+   if (err)
+   return err;
+
+   if (val == 0)
+   return -EINVAL;
+
+   enable = (val != 1);
+

Please accepted only explicit values (1 and 2).

Thanks,
Guenter


+   mutex_lock(_mutex);
+   err = i8k_enable_fan_auto_mode(enable);
+   mutex_unlock(_mutex);
+
+   return err ? -EIO : count;
+}
+
  static SENSOR_DEVICE_ATTR(temp1_input, S_IRUGO, i8k_hwmon_show_temp, NULL, 0);
  static SENSOR_DEVICE_ATTR(temp1_label, S_IRUGO, i8k_hwmon_show_temp_label, 
NULL,
  0);
@@ -719,18 +756,24 @@ static SENSOR_DEVICE_ATTR(fan1_label, S_IRUGO, 
i8k_hwmon_show_fan_label, NULL,
  0);
  static SENSOR_DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
  i8k_hwmon_set_pwm, 0);
+static SENSOR_DEVICE_ATTR(pwm1_enable, S_IWUSR, NULL, i8k_hwmon_set_pwm_enable,
+ 0);
  static SENSOR_DEVICE_ATTR(fan2_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
  1);
  static SENSOR_DEVICE_ATTR(fan2_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL,
  1);
  static SENSOR_DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
  i8k_hwmon_set_pwm, 1);
+static SENSOR_DEVICE_ATTR(pwm2_enable, S_IWUSR, NULL, i8k_hwmon_set_pwm_enable,
+ 1);
  static SENSOR_DEVICE_ATTR(fan3_input, S_IRUGO, i8k_hwmon_show_fan, NULL,
  2);
  static SENSOR_DEVICE_ATTR(fan3_label, S_IRUGO, i8k_hwmon_show_fan_label, NULL,
  2);
  static SENSOR_DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, i8k_hwmon_show_pwm,
  i8k_hwmon_set_pwm, 2);
+static SENSOR_DEVICE_ATTR(pwm3_enable, S_IWUSR, NULL, i8k_hwmon_set_pwm_enable,
+ 2);

  static struct attribute *i8k_attrs[] = {
_dev_attr_temp1_input.dev_attr.attr, /* 0 */
@@ -744,12 +787,15 @@ static struct attribute *i8k_attrs[] = {
_dev_attr_fan1_input.dev_attr.attr,  /* 8 */
_dev_attr_fan1_label.dev_attr.attr,  /* 9 */
_dev_attr_pwm1.dev_attr.attr,/* 10 */
-   _dev_attr_fan2_input.dev_attr.attr,  /* 11 */
-   _dev_attr_fan2_label.dev_attr.attr,  /* 12 */
-   _dev_attr_pwm2.dev_attr.attr,