On 06/26/2017 06:08 AM, Joel Stanley wrote:
On Sat, Jun 24, 2017 at 11:23 PM, Guenter Roeck <linux@xxxxxxxxxxxx> wrote:
On 06/23/2017 08:39 PM, Patrick Venture wrote:
Reduce the fan_tach period such that the fan controller uses a shorter
period to measure the rpm.
This explains what you are doing, but not why. What is the problem you are
trying to solve, and why doesn't it create other problems ? Presumably there
was a reason for the larger period used earlier. If not, if it was just a
conservative setting, here is the place to say it.
I'd update the information myself, but I don't think the underlying
specification
is public, or at least I did not find it.
The datasheet for the Aspeed SoC is not public. Patrick and I can help
answer questions, and I've added Ryan from Aspeed who can answer
questions in more detail.
As I understand it this setting is a trade off between giving the tach
unit enough time to measure complete fan rotations under all
conditions (ie, when the fans are going slow) and getting a timely
measurement. With the driver as-is we have a large delay between
readings, which makes writing a control loop impossible.
I'm not convinced that this driver had enough testing before it was
merged. I agree that Patrick should provide reasoning for his changes,
The driver or the latest set of patches ?
Everyone is encouraged to review patches. If there are concerns with a driver,
those should be raised during the review process. I am not really the fastest
reacting maintainer nowadays, so I would think there should have been enough
time for such feedback. In case concerns were raised and I missed it, my apologies.
Thanks,
Guenter
but I also encourage his efforts as he has spent time looking at the
hardware in detail.
Cheers,
Joel
Guenter
Testing: Tested on an ast2400 sitting on a quanta-q71l.
Signed-off-by: Patrick Venture <venture@xxxxxxxxxx>
---
v3: Added missing change log
v2: Updated commit message language
---
drivers/hwmon/aspeed-pwm-tacho.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/hwmon/aspeed-pwm-tacho.c
b/drivers/hwmon/aspeed-pwm-tacho.c
index 86e2ea8287a7..b2ab5612d8a4 100644
--- a/drivers/hwmon/aspeed-pwm-tacho.c
+++ b/drivers/hwmon/aspeed-pwm-tacho.c
@@ -160,7 +160,7 @@
* 11: reserved.
*/
#define M_TACH_MODE 0x02 /* 10b */
-#define M_TACH_UNIT 0x1000
+#define M_TACH_UNIT 0x00c0
#define INIT_FAN_CTRL 0xFF
struct aspeed_pwm_tacho_data {
--
To unsubscribe from this list: send the line "unsubscribe linux-hwmon" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
