Hi,
在 2024/9/4 22:14, Guenter Roeck 写道:
On 9/4/24 05:36, Li Zetao wrote:
Currently, the nzxt-smart2 module needs to maintain hid resources
by itself. Consider using devm_hid_hw_start_and_open helper to ensure
For all patches:
s/Consider using/Use/
ok
that hid resources are consistent with the device life cycle, and release
hid resources before device is released. At the same time, it can avoid
the goto-release encoding, drop the out_hw_close and out_hw_stop
lables, and directly return the error code when an error occurs.
Signed-off-by: Li Zetao <lizetao1@xxxxxxxxxx>
---
drivers/hwmon/nzxt-smart2.c | 22 +++-------------------
1 file changed, 3 insertions(+), 19 deletions(-)
diff --git a/drivers/hwmon/nzxt-smart2.c b/drivers/hwmon/nzxt-smart2.c
index df6fa72a6b59..b5721a42c0d3 100644
--- a/drivers/hwmon/nzxt-smart2.c
+++ b/drivers/hwmon/nzxt-smart2.c
@@ -750,14 +750,10 @@ static int nzxt_smart2_hid_probe(struct
hid_device *hdev,
if (ret)
return ret;
- ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
+ ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
if (ret)
return ret;
- ret = hid_hw_open(hdev);
- if (ret)
- goto out_hw_stop;
-
hid_device_io_start(hdev);
init_device(drvdata, UPDATE_INTERVAL_DEFAULT_MS);
@@ -765,19 +761,10 @@ static int nzxt_smart2_hid_probe(struct
hid_device *hdev,
drvdata->hwmon =
hwmon_device_register_with_info(&hdev->dev, "nzxtsmart2",
drvdata,
&nzxt_smart2_chip_info, NULL);
- if (IS_ERR(drvdata->hwmon)) {
- ret = PTR_ERR(drvdata->hwmon);
- goto out_hw_close;
- }
+ if (IS_ERR(drvdata->hwmon))
+ return PTR_ERR(drvdata->hwmon);
return 0;
return PTR_ERR_OR_ZERO(drvdata->hwmon);
Also, this can be optimized further.
struct device *hwmon; // and drop from struct drvdata
...
hwmon = devm_hwmon_device_register_with_info(&hdev->dev,
"nzxtsmart2", drvdata,
&nzxt_smart2_chip_info, NULL);
return PTR_ERR_OR_ZERO(hwmon);
and drop the remove function entirely.
Benjamin mentioned that there are unsafe scenarios in
hid_hw_close_and_stop, and it is necessary to ensure that the driver can
not use manual kzalloc/kfree. So, to be extra safe, I would delete
.remove in v2.
Thanks,
Guenter
-
-out_hw_close:
- hid_hw_close(hdev);
-
-out_hw_stop:
- hid_hw_stop(hdev);
- return ret;
}
static void nzxt_smart2_hid_remove(struct hid_device *hdev)
@@ -785,9 +772,6 @@ static void nzxt_smart2_hid_remove(struct
hid_device *hdev)
struct drvdata *drvdata = hid_get_drvdata(hdev);
hwmon_device_unregister(drvdata->hwmon);
-
- hid_hw_close(hdev);
- hid_hw_stop(hdev);
}
static const struct hid_device_id nzxt_smart2_hid_id_table[] = {
