[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <5404194d-9ee6-4def-a040-f5858d8c2d66@wanadoo.fr>
Date: Sun, 3 Dec 2023 16:26:02 +0100
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Aleksa Savic <savicaleksa83@...il.com>, linux-hwmon@...r.kernel.org
Cc: Jean Delvare <jdelvare@...e.com>,
Guenter Roeck <linux@...ck-us.net>,
Jonathan Corbet <corbet@....net>,
"open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] hwmon: Add driver for Gigabyte AORUS Waterforce AIO
coolers
Le 03/12/2023 à 16:00, Aleksa Savic a écrit :
>>> +static int waterforce_probe(struct hid_device *hdev, const struct hid_device_id *id)
>>> +{
>>> + struct waterforce_data *priv;
>>> + int ret;
>>> +
>>> + priv = devm_kzalloc(&hdev->dev, sizeof(*priv), GFP_KERNEL);
>>> + if (!priv)
>>> + return -ENOMEM;
>>> +
>>> + priv->hdev = hdev;
>>> + hid_set_drvdata(hdev, priv);
>>> +
>>> + /*
>>> + * Initialize priv->updated to STATUS_VALIDITY seconds in the past, making
>>> + * the initial empty data invalid for waterforce_read() without the need for
>>> + * a special case there.
>>> + */
>>> + priv->updated = jiffies - msecs_to_jiffies(STATUS_VALIDITY);
>>> +
>>> + ret = hid_parse(hdev);
>>> + if (ret) {
>>> + hid_err(hdev, "hid parse failed with %d\n", ret);
>>> + return ret;
>>> + }
>>> +
>>> + /*
>>> + * Enable hidraw so existing user-space tools can continue to work.
>>> + */
>>> + ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
>>> + if (ret) {
>>> + hid_err(hdev, "hid hw start failed with %d\n", ret);
>>> + goto fail_and_stop;
>>
>> Should this be 'return ret;' (the _start has failed, so why stop?)
>
> Hm, yes.
>
>>
>>> + }
>>> +
>>> + ret = hid_hw_open(hdev);
>>> + if (ret) {
>>> + hid_err(hdev, "hid hw open failed with %d\n", ret);
>>> + goto fail_and_close;
>>
>> Should this be 'fail_and_stop' (the _open has failed, so why close?)
>
> Also yes... I based this part on the nzxt-kraken2 driver in the tree,
> perhaps that should be investigated as well. The aquacomputer_d5next driver
> seems to be doing it correctly.
Done, patch sent.
CH
Powered by blists - more mailing lists