| Message ID | 20240904123607.3407364-9-lizetao1@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: convert to devm_hid_hw_start_and_open() | expand |
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@huawei.com> > --- > 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 >
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); /* Shortcut potential pending reply that will never arrive */ spin_lock_irqsave(&data->lock, flags);
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@huawei.com> --- drivers/hid/hid-picolcd_core.c | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-)