On Wed, 4 Sep 2024 20:36:03 +0800 Li Zetao <lizetao1@xxxxxxxxxx> wrote: > Currently, the corsair-psu module needs to maintain hid resources > by itself. Consider using 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. > > Signed-off-by: Li Zetao <lizetao1@xxxxxxxxxx> > --- > drivers/hwmon/corsair-psu.c | 24 +++++------------------- > 1 file changed, 5 insertions(+), 19 deletions(-) > > diff --git a/drivers/hwmon/corsair-psu.c b/drivers/hwmon/corsair-psu.c > index f8f22b8a67cd..b574ec9cd00f 100644 > --- a/drivers/hwmon/corsair-psu.c > +++ b/drivers/hwmon/corsair-psu.c > @@ -787,14 +787,10 @@ static int corsairpsu_probe(struct hid_device *hdev, const struct > hid_device_id 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 fail_and_stop; > - > priv->hdev = hdev; > hid_set_drvdata(hdev, priv); > mutex_init(&priv->lock); > @@ -805,13 +801,13 @@ static int corsairpsu_probe(struct hid_device *hdev, const struct > hid_device_id ret = corsairpsu_init(priv); > if (ret < 0) { > dev_err(&hdev->dev, "unable to initialize device (%d)\n", ret); > - goto fail_and_stop; > + return ret; > } > > ret = corsairpsu_fwinfo(priv); > if (ret < 0) { > dev_err(&hdev->dev, "unable to query firmware (%d)\n", ret); > - goto fail_and_stop; > + return ret; > } > > corsairpsu_get_criticals(priv); > @@ -820,20 +816,12 @@ static int corsairpsu_probe(struct hid_device *hdev, const struct > hid_device_id priv->hwmon_dev = hwmon_device_register_with_info(&hdev->dev, "corsairpsu", priv, > &corsairpsu_chip_info, NULL); > > - if (IS_ERR(priv->hwmon_dev)) { > - ret = PTR_ERR(priv->hwmon_dev); > - goto fail_and_close; > - } > + if (IS_ERR(priv->hwmon_dev)) > + return PTR_ERR(priv->hwmon_dev); > > corsairpsu_debugfs_init(priv); > > return 0; > - > -fail_and_close: > - hid_hw_close(hdev); > -fail_and_stop: > - hid_hw_stop(hdev); > - return ret; > } > > static void corsairpsu_remove(struct hid_device *hdev) > @@ -842,8 +830,6 @@ static void corsairpsu_remove(struct hid_device *hdev) > > debugfs_remove_recursive(priv->debugfs); > hwmon_device_unregister(priv->hwmon_dev); > - hid_hw_close(hdev); > - hid_hw_stop(hdev); > } > > static int corsairpsu_raw_event(struct hid_device *hdev, struct hid_report *report, u8 *data, So far looks fine to me. Reviewed-by: Wilken Gottwalt <wilken.gottwalt@xxxxxxxxxx> greetings, Wilken