Re: [PATCH -next 11/19] HID: wiimote: Use devm_hid_hw_start_and_open in wiimote_hid_probe()

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

 



On Sep 04 2024, Li Zetao wrote:
> Currently, the wiimote 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 err_close and err_stop lables.
> 
> Signed-off-by: Li Zetao <lizetao1@xxxxxxxxxx>
> ---
>  drivers/hid/hid-wiimote-core.c | 20 +++-----------------
>  1 file changed, 3 insertions(+), 17 deletions(-)
> 
> diff --git a/drivers/hid/hid-wiimote-core.c b/drivers/hid/hid-wiimote-core.c
> index 26167cfb696f..28cd9ccbb617 100644
> --- a/drivers/hid/hid-wiimote-core.c
> +++ b/drivers/hid/hid-wiimote-core.c
> @@ -1780,8 +1780,6 @@ static void wiimote_destroy(struct wiimote_data *wdata)
>  	wiimote_ext_unload(wdata);
>  	wiimote_modules_unload(wdata);
>  	cancel_work_sync(&wdata->queue.worker);
> -	hid_hw_close(wdata->hdev);
> -	hid_hw_stop(wdata->hdev);
>  
>  	kfree(wdata);

This is wrong because wdata is not devres managed.

So there is probably a race condition where it gets freed while the
underlying bus wasn't stopped.

Cheers,
Benjamin

>  }
> @@ -1806,22 +1804,14 @@ static int wiimote_hid_probe(struct hid_device *hdev,
>  		goto err;
>  	}
>  
> -	ret = hid_hw_start(hdev, HID_CONNECT_HIDRAW);
> -	if (ret) {
> -		hid_err(hdev, "HW start failed\n");
> +	ret = devm_hid_hw_start_and_open(hdev, HID_CONNECT_HIDRAW);
> +	if (ret)
>  		goto err;
> -	}
> -
> -	ret = hid_hw_open(hdev);
> -	if (ret) {
> -		hid_err(hdev, "cannot start hardware I/O\n");
> -		goto err_stop;
> -	}
>  
>  	ret = device_create_file(&hdev->dev, &dev_attr_extension);
>  	if (ret) {
>  		hid_err(hdev, "cannot create sysfs attribute\n");
> -		goto err_close;
> +		goto err;
>  	}
>  
>  	ret = device_create_file(&hdev->dev, &dev_attr_devtype);
> @@ -1847,10 +1837,6 @@ static int wiimote_hid_probe(struct hid_device *hdev,
>  
>  err_ext:
>  	device_remove_file(&wdata->hdev->dev, &dev_attr_extension);
> -err_close:
> -	hid_hw_close(hdev);
> -err_stop:
> -	hid_hw_stop(hdev);
>  err:
>  	input_free_device(wdata->ir);
>  	input_free_device(wdata->accel);
> -- 
> 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