hwmon: (asus_rog_ryujin) Ryujin III pump control source

v2: patch 2: compute the elapsed time with an unsigned subtraction so a jiffies wrap on 32-bit cannot cause a long sleep (found by Sashiko). Patch 1 unchanged.
v1: https://lore.kernel.org/linux-hwmon/[email protected]/

The RYUJIN III pump either runs at the duty set over USB or is handed to its internal controller, which follows the PWM of the motherboard header its 4-pin cable is plugged into. Byte 2 of the set cooler duty command selects which. The driver sends 1 there with every duty write, so a pwm2 write takes the pump away from its internal controller, and after power-on a pwm1 write of the reported 40% leaves the pump at full speed. Patch 1 fixes both. Patch 2 adds pwm1_enable to switch between the two and to report which one is in control.

Patch 1 fixes a user-visible bug and applies to v7.3-rc4 as well as to hwmon-next. On v7.3-rc4 the driver does not bind the base RYUJIN III 360, whose ID is only in hwmon-next, so there patch 1 changes behavior for the EXTREME, EVA and WHITE EDITION, none of which I could test. Edward Blair's analysis of the WHITE EDITION firmware shows the same pump control source. Patch 2 is a new feature for hwmon-next.

Tested on a RYUJIN III 360, firmware AURJ2-S750-0108, with out-of-tree builds of drivers/hwmon/asus_rog_ryujin.c on 7.0.0-34-generic:
hwmon-next 9edd5ab2656 as is, with patch 1, and with both patches. The pump cable is on AIO_PUMP (nct6799 pwm7, set by hand for the tests); it was on CPU_FAN at the time of the first comments in the linked issue. Pump rpm at header 50% / 80%, then the reported source:

                                  before       patch 1      patches 1-2
a  on header, pwm2 = 40%          1710/1710 1  2100/3060 0  2100/3060 0
b  on header, pwm1 = 40%          1725/1740 1  1740/1740 1  1725/1740 1
c  pwm1 = 60%, then 40%           2460, 1740   2460, 1740   2460, 1740
d  then pwm2 = 40%                1740/1740 1  1740/1740 1  1740/1740 1
e1 reload with the pump on USB    1740/1740 1  1740/1740 1  1740/1740 1
   then pwm2, pwm1 = 40%          1740, 1740   1755, 1740   1740, 1740
e2 reload with the pump on header 2160/3090 0  2160/3090 0  2160/3090 0
   then pwm2 = 40%                1740/1740 1  2130/3090 0  2130/3090 0
   then pwm1 = 60%                2475 1       2460 1       2460 1
f  pwm1_enable = 2                                          2130/3090 0
   pwm1_enable = 1                                          1740/1740 1
   pwm1_enable = 0, 3                                       -EINVAL

"On header" is the state after EC 1A 00 28 1E, which leaves the pump on its internal controller. The internal fan followed pwm2 in every column (40%: 1545 to 1590 rpm).

Written back to back, 2 runs each, final state:

  pwm1 = 60%, pwm2 = 40% (patch 1)    source 1, pump 60%, fan 40%
  pwm1 = 40%, pwm1 = 60% (patch 1)    source 1, pump 60%
  pwm1_enable = 2, pwm1 = 60%         source 1, pump 60%
  pwm1_enable = 1, 2, 1               source 1
  pwm1 = 60%, pwm1_enable = 2         source 0

The failure in the linked issue needs the power-on state, which a USB reset does not bring back, so it was measured from power-on (PSU switched off), once per column, with the header at 100%. Source, byte 9 of the status report (the duty the pump runs at) and pump rpm:

                           before         patch 1        patches 1-2
  no write yet             0  100  3570   0  100  3570   0  100  3570
  pwm2 = 40%               1  100  3570   0  100  3570   0  100  3570
  pwm1 = 40% (reported)    1  100  3570   1   40  1710   1   40  1710
  pwm1 = 60%               1   60  2430
  pwm1 = 40%               1   40  1725

pwm1_enable = 2 0 100 3585

The patches 1-2 column above was measured with v1 of patch 2. v2 only changes how the wait before a source change is computed, and that wait does not apply to the first source change after the driver loads.

The two writes that patch 1 sends for a pwm1 write of the reported duty reached the requested duty in all 19 trials, 9 with raw commands and 10 through the driver.

The commit in the Fixes: tag, cd258aca0dc4, first appeared in v7.3-rc1.

Zac Lanzon (2):
hwmon: (asus_rog_ryujin) Fix pump control source on the Ryujin III
hwmon: (asus_rog_ryujin) Add pwm1_enable for the Ryujin III pump

 Documentation/hwmon/asus_rog_ryujin.rst |  10 ++
 drivers/hwmon/asus_rog_ryujin.c         | 164 +++++++++++++++++++++++-
 2 files changed, 171 insertions(+), 3 deletions(-)

base-commit: 9edd5ab2656f040d726b5388f49ceac10bb2944f

2.53.0

The RYUJIN III pump has a 4-pin cable for a motherboard fan header in addition to USB. Byte 2 of the set cooler duty command selects the pump control source: 0 hands the pump to its internal controller, which follows the PWM of the header its 4-pin cable is plugged into, and 1 makes it follow the pump duty in byte 3. Byte 3 of the cooler duty report returns the current source.

The driver fills byte 2 with duty_channel, which is 1 for the RYUJIN III, and a pwm2 write re-sends the reported pump duty with the new fan duty. So writing only the internal fan duty takes the pump away from its internal controller.

Measured on a RYUJIN III 360 (firmware AURJ2-S750-0108) with the pump cable on AIO_PUMP and the header PWM set through nct6775, after loading the driver with the pump on its internal controller (pump rpm):

header 50% header 80% source

  no write yet                        2160         3090         0
  pwm2 = 40%, before this patch       1740         1740         1
  pwm2 = 40%, with this patch         2130         3090         0

Send the reported source with pwm2 writes. The fan duty applies with either source (40%: 1545 to 1590 rpm, 30%: 870 rpm). A pwm2 write sent right after a pwm1 write that took the pump over, before the report could show the change, left the pump on USB and applied its fan duty (2 of 2 trials, and 7 of 7 with raw commands up to 0.9 s after).

pwm1 writes keep selecting source 1. The cooler reports the pump duty of the last write, whichever source it selected, and 40% after power-on. Byte 9 of the status report follows the duty the pump runs at. Measured from power-on (PSU switched off) once per driver, with the header at 100%:

before this patch with this patch
source duty rpm source duty rpm

  no write yet                       0    100  3570      0    100  3570
  pwm2 = 40%                         1    100  3570      0    100  3570
  pwm1 = 40%, the reported duty      1    100  3570      1     40  1710
  pwm1 = 60%                         1     60  2430
  pwm1 = 40%                         1     40  1725

So a pwm1 write of the reported duty did not reach the pump, while other duties applied in one write, also when taking the pump from its internal controller (60%: 2460 rpm). When a pwm1 write requests the reported duty, write a duty 1% away first (below at 100%), then the requested one. Sent back to back, the two writes left the pump at the requested duty in all 19 trials, 9 with raw commands and 10 through this driver.

Tested on the RYUJIN III 360 only. The EXTREME, EVA and WHITE EDITION use the same rog_ryujin_iii_info and send the same command, and Edward Blair's analysis of the WHITE EDITION firmware shows the same pump control source. The RYUJIN II, whose duty_channel is 0, is unchanged.

---
 Documentation/hwmon/asus_rog_ryujin.rst |  6 +++
 drivers/hwmon/asus_rog_ryujin.c         | 54 ++++++++++++++++++++++++-
 2 files changed, 59 insertions(+), 1 deletion(-)

index 3ebab230e8c..fe072e94d87 100644
@@ -34,6 +34,12 @@ Usage notes
 As these are USB HIDs, the driver can be loaded automatically by the kernel and
 supports hot swapping.
 
+The pump of the RYUJIN III also has a 4-pin cable for a motherboard fan header.
+The cooler either hands the pump to its internal controller, which follows the
+PWM of the header its 4-pin cable is plugged into, or runs it at the pump duty
+set over USB. Writing pwm1 selects the duty set over USB. Writing pwm2 sets
+only the internal fan duty and leaves the pump control as it is.
+
 Sysfs entries
 -------------
 

2.53.0

Applied.

Thanks,
Guenter

Hi Guenter,

Gentle ping on v2 of this series. Patch 1 fixes the pump control source on the Ryujin III and is Cc'd to stable. Patch 2 adds pwm1_enable.

I can rebase onto hwmon-next and resend if that helps.

Thanks,
Zac

The patches are queued, and I'll push them into my published hwmon-next branch after the commit window closes. The first one should actually already be visible in the hwmon branch since it is a bug fix.

I did send a note, but I am having trouble with gmail; it often refuses to accept my e-mails, claiming that I am a Spammer. Since I am working for Google, I guess they must know ;-).

Guenter

Once pwm1 has been written, nothing in the driver hands the RYUJIN III pump back to its internal controller, and user space cannot tell which source controls the pump.

Expose the pump control source as pwm1_enable, read from the cooler duty report: 1 when the pump follows pwm1, 2 when it is handed to its internal controller, which follows the PWM of the header its 4-pin cable is plugged into. Writing 2 sends the reported duties with source 0, and writing 1 sends them with source 1, with the same neighbouring duty write as pwm1. Other values return -EINVAL. Writing pwm1 still selects source 1. The attribute is not created for the RYUJIN II.

With changes in both directions, source changes can follow each other closely. The cooler ignored a source change sent 500 ms or less after the previous one (7 of 7 trials) and applied one sent 1 s or more after it (5 of 5). Its duty report showed a change within 1.1 s of the command in every trial. So a write that changes the source now waits until 1 s has passed since the previous change, then polls the duty report every 100 ms until it shows the new source, and fails with -ETIMEDOUT after 2 s. pump_source is only taken from the report. Other writes are unaffected.

Measured on a RYUJIN III 360 (firmware AURJ2-S750-0108) with the pump cable on AIO_PUMP and the header PWM set through nct6775 (pump rpm):

header 50% header 80% pwm1_enable

  driver loaded, internal controller    2160        3090          2
  pwm1 = 40%                            1725        1740          1
  pwm1_enable = 2                       2130        3090          2
  pwm1_enable = 1                       1740        1740          1
  pwm1_enable = 2, then pwm1 = 60%      2475                      1
  driver reloaded, pump on USB          1740        1740          1

Written back to back, each source change took 0.95 to 1.07 s, and the sequences ended as requested in 2 of 2 runs each: pwm1_enable = 2 then pwm1 = 60% with the pump at 60% on USB, pwm1_enable = 1, 2, 1 on USB, and pwm1 = 60% then pwm1_enable = 2 on the internal controller.

From power-on (measured with v1 of this patch; v2 changes only how the wait is computed, and the first source change after loading never waits), with the header at 100%, pwm1_enable read 2, read 1 after pwm1 = 40% (pump at 1710 rpm), and writing 2 handed the pump back to its internal controller (3585 rpm).

---
v2: patch 2: compute the elapsed time with an unsigned subtraction so a jiffies wrap on 32-bit cannot cause a long sleep (found by Sashiko). Patch 1 unchanged.
v1: https://lore.kernel.org/linux-hwmon/[email protected]/

 Documentation/hwmon/asus_rog_ryujin.rst |   8 +-
 drivers/hwmon/asus_rog_ryujin.c         | 112 +++++++++++++++++++++++-
 2 files changed, 115 insertions(+), 5 deletions(-)

index fe072e94d87..1fdf6eb9c84 100644
@@ -37,8 +37,10 @@ supports hot swapping.
 The pump of the RYUJIN III also has a 4-pin cable for a motherboard fan header.
 The cooler either hands the pump to its internal controller, which follows the
 PWM of the header its 4-pin cable is plugged into, or runs it at the pump duty
-set over USB. Writing pwm1 selects the duty set over USB. Writing pwm2 sets
-only the internal fan duty and leaves the pump control as it is.
+set over USB, as reported by pwm1_enable. Writing pwm1 selects the duty set
+over USB, and writing 2 to pwm1_enable hands the pump back to its internal
+controller. Writing pwm2 sets only the internal fan duty and leaves the pump
+control as it is.
 
 Sysfs entries
 -------------

@@ -52,6 +54,8 @@ fan5_input  External (controller) fan 3 speed (in rpm, RYUJIN II only)
 fan6_input  External (controller) fan 4 speed (in rpm, RYUJIN II only)
 temp1_input Coolant temperature (in millidegrees Celsius)
 pwm1        Pump duty
+pwm1_enable Pump control: 1 = pwm1, 2 = internal controller following
+            the fan header PWM (RYUJIN III only)
 pwm2        Internal fan duty
 pwm3        External (controller) fan duty (RYUJIN II only)
 =========== ==========================================================

2.53.0

Applied.

Thanks,
Guenter