On Mon, Sep 09, 2024 at 09:23:07AM +0800, Li Zetao wrote: > 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. > > Signed-off-by: Li Zetao <lizetao1@xxxxxxxxxx> Subject should include 'asus_rog_ryujin'. Other than that, Acked-by: Guenter Roeck <linux@xxxxxxxxxxxx> > --- > 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/20240904123607.3407364-14-lizetao1@xxxxxxxxxx/ > > drivers/hwmon/asus_rog_ryujin.c | 47 +++++---------------------------- > 1 file changed, 7 insertions(+), 40 deletions(-) > > 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, > }; > > -- > 2.34.1 > >
