hwmon: (aspeed-pwm-tacho) increase fan tach period
Subsystems (2)
PATCH 1/1 hwmon: (aspeed-pwm-tacho) increase fan tach period
The previous value reduced the time required to determine the fan value, however, it's also used as the final timeout mechanism. The prevous value would work for any fan speed greater than around 3k RPM. This increased value, lets the fan speeds return quickly but will wait longer to handle speeds below 3k RPM.
Testing: this value was determined through experimentation on the ast2400 on the Quanta-q71l. This configurations runs 8 fans attached to the controller.
--- drivers/hwmon/aspeed-pwm-tacho.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/hwmon/aspeed-pwm-tacho.c +1 -1
@@ -161,7 +161,7 @@* 11: reserved. */ #define M_TACH_MODE 0x02/* 10b */ -#define M_TACH_UNIT 0x00c0 +#define M_TACH_UNIT 0x0210#define INIT_FAN_CTRL 0xFF/* How long we sleep in us while waiting for an RPM result. */
Signature · 1 line
2.14.1.581.gf28d330327-goog
Joel Stanley
maintainerHi Patrick,
Quoted 6 lines · Patrick Venture
The previous value reduced the time required to determine the fan value, however, it's also used as the final timeout mechanism. The prevous value would work for any fan speed greater than around 3k RPM. This increased value, lets the fan speeds return quickly but will wait longer to handle speeds below 3k RPM.
Given you've had to tweak this value a few times, should we instead make it a device tree property?
We could still use the result of your findings to provide a sensible default, but machines that have specific fan configurations could use the property to tweak the driver.
Cheers,
Joel
Quoted 26 lines
Testing: this value was determined through experimentation on the ast2400 on the Quanta-q71l. This configurations runs 8 fans attached to the controller.
Signed-off-by Patrick Venture <[email protected]>--- drivers/hwmon/aspeed-pwm-tacho.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/hwmon/aspeed-pwm-tacho.c +1
-2@@ -161,7 +161,7 @@* 11: reserved. */ #define M_TACH_MODE 0x02/* 10b */ -#define M_TACH_UNIT 0x00c0 +#define M_TACH_UNIT 0x0210#define INIT_FAN_CTRL 0xFF/* How long we sleep in us while waiting for an RPM result. */ --2.14.1.581.gf28d330327-goog
A message that is not in the archive (sent off-list or not yet received): CAO=notx_s9wYV2W7MXdXesmF_4icqur8UuTH5=vi2ctOWB8aGw@mail.gmail.com
Guenter Roeck
maintainerQuoted 21 lines · Patrick Venture
Quoted 11 lines · Joel Stanley
Hi Patrick,
Quoted 6 lines · Patrick Venture
The previous value reduced the time required to determine the fan value, however, it's also used as the final timeout mechanism. The prevous value would work for any fan speed greater than around 3k RPM. This increased value, lets the fan speeds return quickly but will wait longer to handle speeds below 3k RPM.
Quoted 7 lines
Given you've had to tweak this value a few times, should we instead make it a device tree property?
We could still use the result of your findings to provide a sensible default, but machines that have specific fan configurations could use the property to tweak the driver.
Quoted 8 lines
I had a todo item in my last commit for this field to do just that -- make it a variable. I'm perfectly willing to head down this path. I found that my previous value was too aggressive for those slow speeds for the timeout. This path addresses that. I think though this new value, does make for a good default value as you've commented. I have some cycles coming up to move this value into something that can be set in a device tree.
You'll have tto find a means to explain that this is not a configuration parameter but a system property.
Either case, I am hesitant to update this again. This seems to be an endless exercise. How do I know that this won't go on forever ?
Guenter
Quoted 33 lines
Quoted 33 lines
Cheers,
Joel
Review
Quoted 11 lines
Testing: this value was determined through experimentation on the ast2400 on the Quanta-q71l. This configurations runs 8 fans attached to the controller.
Signed-off-by Patrick Venture <[email protected]>--- drivers/hwmon/aspeed-pwm-tacho.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)(quoted diff) +0
-0
b/drivers/hwmon/aspeed-pwm-tacho.cQuoted 15 lines
index
69b97d45e3cb..f914e5f41048100644drivers/hwmon/aspeed-pwm-tacho.c +1
-2@@ -161,7 +161,7 @@* 11: reserved. */ #define M_TACH_MODE 0x02/* 10b */ -#define M_TACH_UNIT 0x00c0 +#define M_TACH_UNIT 0x0210#define INIT_FAN_CTRL 0xFF/* How long we sleep in us while waiting for an RPM result. */ --2.14.1.581.gf28d330327-goog
Quoted 7 lines · Patrick Venture
The previous value reduced the time required to determine the fan value, however, it's also used as the final timeout mechanism. The prevous value would work for any fan speed greater than around 3k RPM. This increased value, lets the fan speeds return quickly but will wait longer to handle speeds below 3k RPM.
Quoted 5 lines
Testing: this value was determined through experimentation on the ast2400 on the Quanta-q71l. This configurations runs 8 fans attached to the controller.
Signed-off-by Patrick Venture <[email protected]>
With reservations:
This is the last time for me to accept a patch changing the value of M_TACH_UNIT in this driver. Any subsequent change will require a more comprehensive solution.
Guenter
Quoted 24 lines
--- drivers/hwmon/aspeed-pwm-tacho.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)drivers/hwmon/aspeed-pwm-tacho.c +1
-1@@ -161,7 +161,7 @@* 11: reserved. */ #define M_TACH_MODE 0x02/* 10b */ -#define M_TACH_UNIT 0x00c0 +#define M_TACH_UNIT 0x0210#define INIT_FAN_CTRL 0xFF/* How long we sleep in us while waiting for an RPM result. */ Signature · 6 lines
2.14.1.581.gf28d330327-goog -- To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html