corsair-hydro-platinum: Add hwmon driver for Corsair H150i Elite RGB and other Hydro AIOs #78
Conversation
|
Marked as draft just in case this somehow bricks those untested devices. |
ProjectSynchro
changed the title
ProjectSynchro
force-pushed
the
h150i_support
branch
from
a1a77d0 to
ff70c8d
Compare
|
Should pass CI now 😄 |
|
I think I have solved everything that was up (may still fail builds for some reason) Since CI tests with kernels with CRC8 disabled, I added a fallback implementation that should allow the driver to still function and build for kernels without it. I also added documentation and a file in debugfs that returns the firmware version, can add more / remove some if wanted. Going to open this for review, this shouldn't brick untested devices, worst case they will need a reset. |
|
Will rebase on the master branch when #80 is merged, that fixes the CI issues for the NZXT drivers which should clear up CI here. |
Adds a new hwmon driver `corsair-hydro-platinum` to support Corsair AIO coolers using the "Hydro Platinum" protocol (e.g., H100i/H150i Elite RGB). This device uses a distinct protocol from the "Commander Core" based devices, communicating via 64-byte HID reports. Technical Implementation Details: 1. Write Operations (Control Transfer): Standard `hid_hw_output_report` fails with -38 (ENOSYS) as the device lacks an Interrupt OUT endpoint. Commands are instead sent via `hid_hw_raw_request` using `HID_REQ_SET_REPORT` (Control Transfer) over Endpoint 0. 2. Packet Structure: To avoid -32 (EPIPE) stalls during Control Transfers, the firmware requires a 65-byte padded buffer structure: [0x00] (Report ID padding) + [0x3F] (Command Prefix) + Payload. 3. Asynchronous Reporting: Status reports are received asynchronously via standard HID Input Reports (raw_event). `hid_device_io_start()` is explicitly called in probe to ensure these reports are delivered to the driver when using `HID_CONNECT_HIDRAW`. 4. Protocol Split (Fan 3 Quirk): The main cooling command (Feature 0x00) only supports 2 fans + Pump. For 360mm models (H150i/Elite), a second command (Feature 0x03) must be sent to control the 3rd fan. Command ordering is critical: Feature 0x00 MUST be sent before Feature 0x03 to avoid device stalls. Features: - Monitoring: - Liquid Temperature. - Pump Speed and Duty Cycle. - Fan Speeds and Duty Cycles (up to 3 fans). - Control (PWM): - Pump Mode Control (Quiet/Balanced/Extreme) via pwm1. - Fan Speed Control (0-100% Duty Cycle) via pwm[2-4]. - Initialization logic defaults fans to 50% to prevent startup noise. - Attempted Robustness: - CRC-8 verification on all received reports. - Synchronous transaction logic (Write + Wait for Report) to ensure data validity. - Exposes human-readable model name (e.g., "Corsair iCUE H150i Elite RGB") via standard `label` sysfs attribute. Signed-off-by: Jack Greiner <jack@emoss.org>
Also make sure we pull completion.h explicitly just in case. Signed-off-by: Jack Greiner <jack@emoss.org>
The driver previously used a single shared buffer for both transmitting commands and receiving responses. This may have caused race conditions when the driver was accessed concurrently, or when userspace tools (like OpenRGB or liquidctl) were communicating with the device at the same time as the kernel driver was. This is easily replicated by using the "Effects" plugin for OpenRGB It seems like I didn't quite implement the CRC validation correctly, it will now properly detect and ignore responses not intended for the driver. Additionally rate-limited logging will report CRC check failures instead. This allows the kernel driver to coexist and remain somewhat resiliant when userspace tools are also using the device. Signed-off-by: Jack Greiner <jack@emoss.org>
Previously this was wishy washy and only really done for FEATURE_COOLING_FAN3 commands (I was having particular issues there). Now it is more consistant and covers all fans + pump commands. Signed-off-by: Jack Greiner <jack@emoss.org>
So we can support older kernel versions we need to use dev_warn_ratelimited and dev_err_ratelimited as it turns out hid_err_ratelimited and hid_warn_ratelimited are newer helper macros. Signed-off-by: Jack Greiner <jack@emoss.org>
Signed-off-by: Jack Greiner <jack@emoss.org>
…RC8 is disabled Signed-off-by: Jack Greiner <jack@emoss.org>
Signed-off-by: Jack Greiner <jack@emoss.org>
…version" Signed-off-by: Jack Greiner <jack@emoss.org>
This should be inline with what the other drivers in this repo are doing for documentation. Signed-off-by: Jack Greiner <jack@emoss.org>
Signed-off-by: Jack Greiner <jack@emoss.org>
…CONFIG_CRC8 is disabled" This reverts commit 3f9dd62. Signed-off-by: Jack Greiner <jack@emoss.org>
ProjectSynchro
force-pushed
the
h150i_support
branch
from
b98c054 to
b47509c
Compare
|
Rebased from master, should pass checks now. |
ProjectSynchro
force-pushed
the
h150i_support
branch
3 times, most recently
from
856718c to
cca01f5
Compare
The `corsair-hydro-platinum` driver relies on the kernel's `crc8` library. In recent mainline kernels, `CONFIG_CRC8` was converted to a hidden symbol (prompt removed), which causes `make allnoconfig` to silently ignore `CONFIG_CRC8=y` in all.config This resulted in `modpost` failures due to undefined `crc8` symbols. See: torvalds/linux@aa09b32 Hopefully fix this by: 1. Explicitly enabling `CONFIG_CRC8=y` in all.config 2. Patching the kernel's Kconfig in build.yml to restore the "CRC8" prompt. This makes the symbol visible and selectable again during configuration, bypassing the limitation of `allnoconfig` with hidden symbols. This ensures the driver builds correctly on both stable (6.8) and mainline configurations. Signed-off-by: Jack Greiner <jack@emoss.org>
ProjectSynchro
force-pushed
the
h150i_support
branch
from
cca01f5 to
1b28bd5
Compare
|
Finally :) Seems like CONFIG_CRC8 has been removed recently for the mainline kernel (as of torvalds/linux@aa09b32) So with a bit of patching we can re-enable this option so the module builds correctly. |
|
Thanks! I'll try to take a look at it next week. (Please ping me if you don't hear back from me). |
jonasmalacofilho
left a comment
jonasmalacofilho
left a comment
There was a problem hiding this comment.
Note: I've tried to be as rigorous as I could, even with minor issues, to prepare this PR for you to mainline.
| * 0 - 84: Quiet Mode | ||
| * 85 - 169: Balanced Mode | ||
| * 170 - 255: Extreme Mode |
There was a problem hiding this comment.
Interesting workaround. I'm not sure if Guenter will approve, but IMO it's worth a try.
| $ sudo insmod drivers/hwmon/nzxt-kraken2.ko # NZXT Kraken X42/X52/X62/X72 | ||
| $ sudo insmod drivers/hwmon/nzxt-kraken3.ko # NZXT Kraken X53/X63/X73, Z53/Z63/Z73, Kraken 2023 (standard, Elite) | ||
| $ sudo insmod drivers/hwmon/nzxt-smart2.ko # NZXT Smart Device V2/RGB & Fan Controller | ||
| $ sudo insmod drivers/hwmon/corsair-hydro-platinum.ko # Corsair Hydro H100i/H115i Platinum/SE, Pro XT, Elite RGB |
There was a problem hiding this comment.
Maybe adjust the previous lines to keep at least 2 paces before the #, to make clearer what is command and what is comment.
| * - Corsair Hydro H100i Platinum / SE (Untested) | ||
| * - Corsair Hydro H115i Platinum (Untested) | ||
| * - Corsair Hydro H60i / H100i / H115i / H150i Pro XT (Untested) | ||
| * - Corsair iCUE H100i / H115i / H150i Elite RGB (Tested) |
There was a problem hiding this comment.
Tested/untested notes prob. belong to commit/PR message, not here, especially when mainlining the driver.
| * The device communicates via USB HID. Unlike standard HID devices, it requires | ||
| * commands to be sent via Control Transfers (Set Report, Endpoint 0). |
There was a problem hiding this comment.
Maybe s/standard/typical/, as Set Report is part of the HID spec, so it's standard by definition. Also, I'm personally not sure it's that uncommon for HIDs to use Set Report as much as this device.
| * IN endpoint. | ||
| * | ||
| * Initialization: | ||
| * The device requires an initialization command (Set Cooling) to begin |
There was a problem hiding this comment.
Given the previous mention of Set Report, which exists in the HID spec, it might be better to omit our ad-hoc name for this other command here, to avoid confusion.
| if (!info) | ||
| return -ENODEV; |
| if (ret) | ||
| hid_warn(hdev, "initialization command failed: %d\n", ret); |
There was a problem hiding this comment.
Do we really want to keep going even if the initialization fails?
| if (ret) | ||
| hid_warn(hdev, "initialization command failed: %d\n", ret); | ||
|
|
||
| /* Wait for response to init command. hydro_platinum_write_cooling handles the delay. */ |
There was a problem hiding this comment.
Shouldn't this comment appear a couple of lines earlier?
| .id_table = hydro_platinum_table, | ||
| .probe = hydro_platinum_probe, | ||
| .remove = hydro_platinum_remove, | ||
| .raw_event = hydro_platinum_raw_event, |
There was a problem hiding this comment.
You probably need a .reset_resume handler when CONFIG_PM is set. You can take a look at the other drivers in this repo for examples.
| hid_hw_close(hdev); | ||
| fail_and_stop: | ||
| hid_hw_stop(hdev); | ||
| return ret; |
There was a problem hiding this comment.
Here would be another opportunity to call mutex_destroy.
2 participants
See the commit message for specifics on the implementation (I fed my really bad notes into an LLM to clean them up so they are more readable)
This started off as a hacky project so I apologise for this being a single dump.
This is pretty much based on the implementation in liquidctl, I made sure that the driver still allows for userspace tools to interact with the device (e.g. OpenRGB control) and the CRC checking mechanism is also mirrored here which should verify responses (tried to mirror what liquidctl does)
Ideally I would have this driver not set anything in order to "wake" the device but that seems to be required for the device to react to commands. I decided to set this as a "safe" and relatively quiet 50% duty cycle on fans and the "balanced" preset for the pump. (Feel free to let me know if there's a better way to handle this)
I also tried my best to make the coolers appear with pretty names by setting a
model_name, not all software seems to respect or show model information.Let me know if the implementation can be improved any, it should work with all devices listed in the foreword since it mirrors the liquidctl implementation where possible.
Anyone with:
Nothing should break but some testing would be nice.. I don't know if bricking is possible, so test at your own risk (I can disable the untested devices if wanted)
If the device decides to stall, power cycling the AIO seems to reset it's state.