Re: [PATCH -next 19/19] hwmon: (nzxt-smart2) Use devm_hid_hw_start_and_open in nzxt_smart2_hid_probe()

[Guenter Roeck <linux@xxxxxxxxxxxx>]

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/

> 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.

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[] = {