Re: [PATCH -next 08/19] HID: hid-picolcd: Use devm_hid_hw_start_and_open in picolcd_probe()

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

 



On Sep 04 2024, Li Zetao wrote:
> Currently, the hid-picolcd 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_cleanup_hid_ll and
> err_cleanup_hid_hw lables.
> 
> Signed-off-by: Li Zetao <lizetao1@xxxxxxxxxx>
> ---
>  drivers/hid/hid-picolcd_core.c | 22 ++++------------------
>  1 file changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c
> index 297103be3381..973822d1b2db 100644
> --- a/drivers/hid/hid-picolcd_core.c
> +++ b/drivers/hid/hid-picolcd_core.c
> @@ -551,23 +551,15 @@ static int picolcd_probe(struct hid_device *hdev,
>  		goto err_cleanup_data;
>  	}
>  
> -	error = hid_hw_start(hdev, 0);
> +	error = devm_hid_hw_start_and_open(hdev, 0);
>  	if (error) {
> -		hid_err(hdev, "hardware start failed\n");
> +		hid_err(hdev, "hardware start and open failed\n");
>  		goto err_cleanup_data;
>  	}
>  
> -	error = hid_hw_open(hdev);
> -	if (error) {
> -		hid_err(hdev, "failed to open input interrupt pipe for key and IR events\n");
> -		goto err_cleanup_hid_hw;
> -	}
> -
>  	error = device_create_file(&hdev->dev, &dev_attr_operation_mode_delay);
> -	if (error) {
> -		hid_err(hdev, "failed to create sysfs attributes\n");
> -		goto err_cleanup_hid_ll;
> -	}
> +	if (error)
> +		goto err_cleanup_data;
>  
>  	error = device_create_file(&hdev->dev, &dev_attr_operation_mode);
>  	if (error) {
> @@ -589,10 +581,6 @@ static int picolcd_probe(struct hid_device *hdev,
>  	device_remove_file(&hdev->dev, &dev_attr_operation_mode);
>  err_cleanup_sysfs1:
>  	device_remove_file(&hdev->dev, &dev_attr_operation_mode_delay);
> -err_cleanup_hid_ll:
> -	hid_hw_close(hdev);
> -err_cleanup_hid_hw:
> -	hid_hw_stop(hdev);
>  err_cleanup_data:
>  	kfree(data);
>  	return error;
> @@ -611,8 +599,6 @@ static void picolcd_remove(struct hid_device *hdev)
>  	picolcd_exit_devfs(data);
>  	device_remove_file(&hdev->dev, &dev_attr_operation_mode);
>  	device_remove_file(&hdev->dev, &dev_attr_operation_mode_delay);
> -	hid_hw_close(hdev);
> -	hid_hw_stop(hdev);

Again, this doesn't seem correct. the spin_lock from below expects the
hw to be stopped, and this patch changes that by postponing the stop
after the remove.

CHeers,
Benjamin

>  
>  	/* Shortcut potential pending reply that will never arrive */
>  	spin_lock_irqsave(&data->lock, flags);
> -- 
> 2.34.1
> 




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux