lkml.org 
[lkml]   [2023]   [Dec]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH v2] hwmon: Add driver for Gigabyte AORUS Waterforce AIO coolers
    From
    On 2023-12-05 15:35:19 GMT+01:00, Guenter Roeck wrote:
    > 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@gmail.com>
    >>>> ---
    > ...
    >>>> +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
    >

    Done in v3.

    Thanks,
    Aleksa

    \
     
     \ /
      Last update: 2023-12-07 13:23    [W:9.418 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site