Add definition and driver for optional EMC2301 fan controller#125
Merged
svenrademakers merged 2 commits intoturing-machines:masterfrom Oct 16, 2023
Merged
Add definition and driver for optional EMC2301 fan controller#125svenrademakers merged 2 commits intoturing-machines:masterfrom
svenrademakers merged 2 commits intoturing-machines:masterfrom
Conversation
Collaborator
As i understood, this was done to reduce costs. Soldering the EMC2301 on there should work, although Turing Machine will not give any official statement on this. |
svenrademakers
approved these changes
Oct 9, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The original design of the TP2 board includes a 4-pin fan header and EMC2301 fan controller. (These are J16 and U109 on the PCB, respectively.) They aren't populated at the factory, but some users may nonetheless want to order and solder these parts themselves, so it makes sense to have out-of-the-box support for it in the firmware, at least.
For boards with the EMC2301 soldered, a
/sys/class/thermal/cooling_device0and/sys/class/hwmon/hwmon1entry appear in sysfs to manage the fan speed. For boards without, the driver (silently) fails to bind, and the BMC operates as usual. So, this makes the EMC2301 pretty much "plug-and-play" -- or, "solder-and-play" at least. :)This PR may be controversial not on software/technical grounds but on logistical ones: I do not know why these parts weren't populated at the factory, but if there was a solid business reason for it, accepting this PR might be incongruous with that reason.
My speculation is that there was uncertainty about whether a pure SMBus/I²C device like the EMC2301 could coexist on the bus with the RTL8370CG, since the latter uses a weird variant of I²C, or perhaps whether the RTL8370CG driver might "hog" the lines, rendering the EMC2301 inaccessible. I can at least confirm that this isn't the case; I quickly put together a Rust tool that can read/write RTL8370CG registers through the ordinary I²C interface, and I can interleave RTL8370CG operations and EMC2301 operations on the same bus with no problem.
But, that's just speculation. Again: I don't know the reason the components aren't populated. I'd recommend not accepting this PR before finding out.