Re: [PATCH -next v2 09/15] hwmon: Use devm_hid_hw_start_and_open in rog_ryujin_probe()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [LM Sensors]     [Linux Sound]     [ALSA Users]     [ALSA Devel]     [Linux Audio Users]     [Linux Media]     [Kernel]     [Gimp]     [Yosemite News]     [Linux Media]

  Powered by Linux