[-next,v3,09/15] hwmon: (asus_rog_ryujin) Use devm_hid_hw_start_and_open in rog_ryujin_probe()

Commit Message

lizetao Sept. 10, 2024, 3:45 p.m. UTC

Currently, the rog_ryujin module needs to maintain hid resources
by itself. Use devm_hid_hw_start_and_open helper to ensure 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 fail_and_close and fail_and_stop
lables, and directly return the error code when an error occurs.

Further optimization, use devm_hwmon_device_register_with_info to replace
hwmon_device_register_with_info, the remote operation can be completely
deleted, and the rog_ryujin_data structure no longer needs to hold
hwmon device.

Acked-by: Guenter Roeck <[e-mail]>
Reviewed-by: Aleksa Savic <[e-mail]>
Signed-off-by: Li Zetao <[e-mail]>
---
v2 -> v3: Added asus_rog_ryujin to prefix subject.
v2: https://lore.kernel.org/all/[e-mail]/
v1 -> v2:
  1) Further optimize using devm_hwmon_device_register_with_info
and remove the .remove operation
  2) Adjust commit information
v1: https://lore.kernel.org/all/[e-mail]/

drivers/hwmon/asus_rog_ryujin.c | 47 +++++----------------------------
 1 file changed, 7 insertions(+), 40 deletions(-)

Patch

diff --git a/drivers/hwmon/asus_rog_ryujin.c b/drivers/hwmon/asus_rog_ryujin.c
index f8b20346a995..ef0d9b72a824 100644
--- a/drivers/hwmon/asus_rog_ryujin.c
+++ b/drivers/hwmon/asus_rog_ryujin.c
@@ -80,7 +80,6 @@  static const char *const rog_ryujin_speed_label[] = {
 
 struct rog_ryujin_data {
 	struct hid_device *hdev;
-	struct device *hwmon_dev;
 	/* For locking access to buffer */
 	struct mutex buffer_lock;
 	/* For queueing multiple readers */
@@ -497,6 +496,7 @@  static int rog_ryujin_raw_event(struct hid_device *hdev, struct hid_report *repo
 static int rog_ryujin_probe(struct hid_device *hdev, const struct hid_device_id *id)
 {
 	struct rog_ryujin_data *priv;
+	struct device *hwmon_dev;
 	int ret;
 
 	priv = devm_kzalloc(&hdev->dev, sizeof(*priv), GFP_KERNEL);
@@ -520,23 +520,13 @@  static int rog_ryujin_probe(struct hid_device *hdev, const struct hid_device_id
 	}
 
 	/* 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);
+	ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
+	if (ret)
 		return ret;
-	}
-
-	ret = hid_hw_open(hdev);
-	if (ret) {
-		hid_err(hdev, "hid hw open failed with %d\n", ret);
-		goto fail_and_stop;
-	}
 
 	priv->buffer = devm_kzalloc(&hdev->dev, MAX_REPORT_LENGTH, GFP_KERNEL);
-	if (!priv->buffer) {
-		ret = -ENOMEM;
-		goto fail_and_close;
-	}
+	if (!priv->buffer)
+		return -ENOMEM;
 
 	mutex_init(&priv->status_report_request_mutex);
 	mutex_init(&priv->buffer_lock);
@@ -548,31 +538,9 @@  static int rog_ryujin_probe(struct hid_device *hdev, const struct hid_device_id
 	init_completion(&priv->cooler_duty_set);
 	init_completion(&priv->controller_duty_set);
 
-	priv->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, "rog_ryujin",
+	hwmon_dev = devm_hwmon_device_register_with_info(&hdev->dev, "rog_ryujin",
 							  priv, &rog_ryujin_chip_info, NULL);
-	if (IS_ERR(priv->hwmon_dev)) {
-		ret = PTR_ERR(priv->hwmon_dev);
-		hid_err(hdev, "hwmon registration failed with %d\n", ret);
-		goto fail_and_close;
-	}
-
-	return 0;
-
-fail_and_close:
-	hid_hw_close(hdev);
-fail_and_stop:
-	hid_hw_stop(hdev);
-	return ret;
-}
-
-static void rog_ryujin_remove(struct hid_device *hdev)
-{
-	struct rog_ryujin_data *priv = hid_get_drvdata(hdev);
-
-	hwmon_device_unregister(priv->hwmon_dev);
-
-	hid_hw_close(hdev);
-	hid_hw_stop(hdev);
+	return PTR_ERR_OR_ZERO(hwmon_dev);
 }
 
 static const struct hid_device_id rog_ryujin_table[] = {
@@ -586,7 +554,6 @@  static struct hid_driver rog_ryujin_driver = {
 	.name = "rog_ryujin",
 	.id_table = rog_ryujin_table,
 	.probe = rog_ryujin_probe,
-	.remove = rog_ryujin_remove,
 	.raw_event = rog_ryujin_raw_event,
 };