| Message ID | 20240909012313.500341-10-lizetao1@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | HID: convert to devm_hid_hw_start_and_open() | expand |
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@huawei.com> Subject should include 'asus_rog_ryujin'. Other than that, Acked-by: Guenter Roeck <linux@roeck-us.net> > --- > 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@huawei.com/ > > 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 > >
On 2024-09-09 03:23:07 GMT+02:00, 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@huawei.com> Reviewed-by: Aleksa Savic <savicaleksa83@gmail.com>
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, };
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@huawei.com> --- 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@huawei.com/ drivers/hwmon/asus_rog_ryujin.c | 47 +++++---------------------------- 1 file changed, 7 insertions(+), 40 deletions(-)