Re: [PATCH v2] hwmon: Add driver for Gigabyte AORUS Waterforce AIO coolers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 12/5/23 06:22, Aleksa Savic wrote:
On 2023-12-03 18:36:35 GMT+01:00, Guenter Roeck wrote:
On Sun, Dec 03, 2023 at 01:06:48PM +0100, Aleksa Savic wrote:
This driver exposes hardware sensors of the Gigabyte AORUS Waterforce
all-in-one CPU liquid coolers, which communicate through a proprietary
USB HID protocol. Report offsets were initially discovered in [1] and
confirmed by me on a Waterforce X240 by observing the sent reports from
the official software.

Available sensors are pump and fan speed in RPM, as well as coolant
temperature. Also available through debugfs is the firmware version.

Attaching a fan is optional and allows it to be controlled from the
device. If it's not connected, the fan-related sensors will report
zeroes.

The addressable RGB LEDs and LCD screen are not supported in this
driver and should be controlled through userspace tools.

[1]: https://github.com/liquidctl/liquidctl/issues/167

Signed-off-by: Aleksa Savic <savicaleksa83@xxxxxxxxx>
---
...
+static int waterforce_get_status(struct waterforce_data *priv)
+{
+	int ret = 0;
+
+	if (!mutex_lock_interruptible(&priv->status_report_request_mutex)) {
+		if (!time_after(jiffies, priv->updated + msecs_to_jiffies(STATUS_VALIDITY))) {
+			/* Data is up to date */
+			goto unlock_and_return;
+		}

What is the point of this check ? The calling code already checks it.
Checking it twice, once inside and once outside the lock, seems
excessive.


If there are multiple readers here, only the first one should request the status,
so when others enter the mutex they can exit early if the data is there.


Please change the code to only check once from within the mutex.

Guenter





[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux