Closed xaizek closed 7 months ago
@xaizek Can you add sample in readme to manage fan speed before merge?
@xaizek Hello, Can you set the full path of echo 128 > pwm6
...
I have symlink at /sys/class/hwmon/hwmon5
pointing to /sys/devices/platform/nct6687.2592/hwmon/hwmon5
and I don't think it's the same path for other boards. Just that number 5
is likely to be different (don't know what 2592
is).
I can add directions to look somewhere in /sys/class/hwmon/hwmon*
.
I have symlink at
/sys/class/hwmon/hwmon5
pointing to/sys/devices/platform/nct6687.2592/hwmon/hwmon5
and I don't think it's the same path for other boards. Just that number5
is likely to be different (don't know what2592
is).I can add directions to look somewhere in
/sys/class/hwmon/hwmon*
.
Mine is: /sys/devices/platform/nct6687.2592/hwmon/hwmon3
this setting is not pwm[*]_mode, this is pwm[1-7]_bypass
@SergeyMy Could you explain?
https://github.com/Fred78290/nct6687d/issues/82
example: 1. linux-6.7-rc6/Documentation/hwmon/nct6775.rst
pwm[1-7]**_mode**
- controls if output is PWM or DC level
* 0 DC output
* 1 PWM output
2. in nct6683-nct6687 this register is definitely bypass, it completely disables and stops all PWM adjustment units
#define NCT6683_REG_FAN_CFG_MODE 0xa00 //0-7bit=pwm0-pwm7. 0-normal, 1-bypass_mode.
choice of type of regulation.... I haven’t found it yet, but it exists :)
pwm[1-7]_enable
- this file controls mode of fan/temperature control:
* 0 Fan control disabled (fans set to maximum speed)
* 1 Manual mode, write to pwm[0-5] any value 0-255
* 2 "Thermal Cruise" mode
* 3 "Fan Speed Cruise" mode
* 4 "Smart Fan III" mode (NCT6775F only)
* 5 "Smart Fan IV" mode
nct7904.rst
:
pwm[1-4]_enable R/W, 1/2 for manual or SmartFan mode
Setting SmartFan mode is supported only if it has been
previously configured by BIOS (or configuration EEPROM)
Very similar to what's done here. How manual mode is implemented or called internally (if it's actually called that way in Nuvoton docs) isn't relevant for the API which is wrong in it's own way (_enabled
is actually _mode
, and _mode
should be _units
or something like that).
choice of type of regulation.... I haven’t found it yet, but it exists :)
Sounds like you haven't seen that there is https://github.com/Dasharo/coreboot/blob/dasharo/src/superio/nuvoton/nct6687d/nct6687d_hwm.c
I'm talking about the same thing, it's a mess. but we have a register: controls if output is PWM or DC level what shall we call it? :)
PS and yes, this register not only provides manual input, but also disables most of the regulator
but we have a register: controls if output is PWM or DC level what shall we call it?
pwm*_mode
, as per kernel documentation. Compatibility and consistency defines the outcome here.
Compatibility and consistency defines the outcome here.
pwm[1-*]_mode
direct current or pulse-width modulation.
:v:
turn everything back. make changes to pwm[1-*]_enable according to the table:
pwm[1-7]_enable
0 Fan control disabled (fans set to maximum speed) 1 Manual mode, write to pwm[0-5] any value 0-255 2 "Thermal Cruise" mode
...
@Fred78290 feedback commit
pwm[1-]_enable - Fan speed control method. (0...5) (1-man, 2 or other -"Thermal Cruise" ) pwm[1-]_mode - direct current or pulse-width modulation. (0...1) DC or PWM level
(don't know what
2592
is)
=0xA20 = EC base I/O port Found NCT6687D or compatible chip at 0x4e:0xa20
@Fred78290 I'm ready to send
SENSOR_TEMPLATE(pwm_mode, "pwm%d_mode", S_IRUGO, show_pwm_mode, NULL, 0);
but there is a conflict
SENSOR_TEMPLATE_2(pwm_mode, "pwm%d_enable", S_IRUGO, show_pwm_mode, store_pwm_mode, 0, 0);
@Fred78290 I'm ready to send
SENSOR_TEMPLATE(pwm_mode, "pwm%d_mode", S_IRUGO, show_pwm_mode, NULL, 0);
but there is a conflict
SENSOR_TEMPLATE_2(pwm_mode, "pwm%d_enable", S_IRUGO, show_pwm_mode, store_pwm_mode, 0, 0);
?
@Fred78290 I'm ready to send
SENSOR_TEMPLATE(pwm_mode, "pwm%d_mode", S_IRUGO, show_pwm_mode, NULL, 0);
but there is a conflictSENSOR_TEMPLATE_2(pwm_mode, "pwm%d_enable", S_IRUGO, show_pwm_mode, store_pwm_mode, 0, 0);
?
??? you need to rename the functions show_pwm_mode, store_pwm_mode in -> something else otherwise the code will turn into something not very good It's a bit strange to bind the pwm_enable object to the pwm_mode function :)
Rename *_mode
to *_enable
. I wasn't sure whether to call them per API name or per functionality, but having several functions with unmatched names will be confusing.
@SergeyMy @xaizek
is pwm_X_enable & pwm_x_mode not doing the same?
My approach is "enable" term is boolean true/false, "mode" term is analogic 0..255.
So I'm a little bit confused.
@xaizek your pull request - you and rename it
So I'm a little bit confused.
@xaizek understands what we're talking about, everything will be decided one way or another he himself gave a link to... https://www.kernel.org/doc/html/latest/hwmon/sysfs-interface.html#pwm
@SergeyMy, this PR is already merged. Just change 3 extra lines in a new PR.
pwm[1-8]_enable
to switch between manual/firmware-setup mode.Closes #78.