* [PATCH 0/2] HID: do not call hid_set_drvdata(hdev, NULL) in drivers @ 2019-08-12 16:27 Benjamin Tissoires 2019-08-12 16:27 ` [PATCH 1/2] " Benjamin Tissoires 2019-08-12 16:27 ` [PATCH 2/2] HID: wacom: do not call hid_set_drvdata(hdev, NULL) Benjamin Tissoires 0 siblings, 2 replies; 8+ messages in thread From: Benjamin Tissoires @ 2019-08-12 16:27 UTC (permalink / raw) To: Bruno Prémont, Jonathan Cameron, Srinivas Pandruvada, Ping Cheng, Jason Gerecke, Jiri Kosina Cc: linux-input, linux-kernel, Benjamin Tissoires Hi, the wacom one is split dfrom the other drivers as the change is slightly bigger. Cheers, Benjamin Benjamin Tissoires (2): HID: do not call hid_set_drvdata(hdev, NULL) in drivers HID: wacom: do not call hid_set_drvdata(hdev, NULL) drivers/hid/hid-cougar.c | 6 ++---- drivers/hid/hid-gfrm.c | 7 ------- drivers/hid/hid-lenovo.c | 2 -- drivers/hid/hid-picolcd_core.c | 7 +------ drivers/hid/hid-sensor-hub.c | 1 - drivers/hid/wacom_sys.c | 18 +++++------------- 6 files changed, 8 insertions(+), 33 deletions(-) -- 2.19.2 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 1/2] HID: do not call hid_set_drvdata(hdev, NULL) in drivers 2019-08-12 16:27 [PATCH 0/2] HID: do not call hid_set_drvdata(hdev, NULL) in drivers Benjamin Tissoires @ 2019-08-12 16:27 ` Benjamin Tissoires 2019-08-12 16:39 ` Srinivas Pandruvada 2019-08-13 5:53 ` Bruno Prémont 2019-08-12 16:27 ` [PATCH 2/2] HID: wacom: do not call hid_set_drvdata(hdev, NULL) Benjamin Tissoires 1 sibling, 2 replies; 8+ messages in thread From: Benjamin Tissoires @ 2019-08-12 16:27 UTC (permalink / raw) To: Bruno Prémont, Jonathan Cameron, Srinivas Pandruvada, Ping Cheng, Jason Gerecke, Jiri Kosina Cc: linux-input, linux-kernel, Benjamin Tissoires This is a common pattern in the HID drivers to reset the drvdata. Some do it properly, some do it only in case of failure. But, this is actually already handled by driver core, so there is no need to do it manually. Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/hid/hid-cougar.c | 6 ++---- drivers/hid/hid-gfrm.c | 7 ------- drivers/hid/hid-lenovo.c | 2 -- drivers/hid/hid-picolcd_core.c | 7 +------ drivers/hid/hid-sensor-hub.c | 1 - 5 files changed, 3 insertions(+), 20 deletions(-) diff --git a/drivers/hid/hid-cougar.c b/drivers/hid/hid-cougar.c index e0bb7b34f3a4..4ff3bc1d25e2 100644 --- a/drivers/hid/hid-cougar.c +++ b/drivers/hid/hid-cougar.c @@ -207,7 +207,7 @@ static int cougar_probe(struct hid_device *hdev, error = hid_parse(hdev); if (error) { hid_err(hdev, "parse failed\n"); - goto fail; + return error; } if (hdev->collection->usage == COUGAR_VENDOR_USAGE) { @@ -219,7 +219,7 @@ static int cougar_probe(struct hid_device *hdev, error = hid_hw_start(hdev, connect_mask); if (error) { hid_err(hdev, "hw start failed\n"); - goto fail; + return error; } error = cougar_bind_shared_data(hdev, cougar); @@ -249,8 +249,6 @@ static int cougar_probe(struct hid_device *hdev, fail_stop_and_cleanup: hid_hw_stop(hdev); -fail: - hid_set_drvdata(hdev, NULL); return error; } diff --git a/drivers/hid/hid-gfrm.c b/drivers/hid/hid-gfrm.c index 86c317320bf2..699186ff2349 100644 --- a/drivers/hid/hid-gfrm.c +++ b/drivers/hid/hid-gfrm.c @@ -123,12 +123,6 @@ static int gfrm_probe(struct hid_device *hdev, const struct hid_device_id *id) return ret; } -static void gfrm_remove(struct hid_device *hdev) -{ - hid_hw_stop(hdev); - hid_set_drvdata(hdev, NULL); -} - static const struct hid_device_id gfrm_devices[] = { { HID_BLUETOOTH_DEVICE(0x58, 0x2000), .driver_data = GFRM100 }, @@ -142,7 +136,6 @@ static struct hid_driver gfrm_driver = { .name = "gfrm", .id_table = gfrm_devices, .probe = gfrm_probe, - .remove = gfrm_remove, .input_mapping = gfrm_input_mapping, .raw_event = gfrm_raw_event, .input_configured = gfrm_input_configured, diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c index 364bc7f11d9d..96fa2a2c2cd3 100644 --- a/drivers/hid/hid-lenovo.c +++ b/drivers/hid/hid-lenovo.c @@ -866,8 +866,6 @@ static void lenovo_remove_tpkbd(struct hid_device *hdev) led_classdev_unregister(&data_pointer->led_micmute); led_classdev_unregister(&data_pointer->led_mute); - - hid_set_drvdata(hdev, NULL); } static void lenovo_remove_cptkbd(struct hid_device *hdev) diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c index 5f7a39a5d4af..1b5c63241af0 100644 --- a/drivers/hid/hid-picolcd_core.c +++ b/drivers/hid/hid-picolcd_core.c @@ -534,8 +534,7 @@ static int picolcd_probe(struct hid_device *hdev, data = kzalloc(sizeof(struct picolcd_data), GFP_KERNEL); if (data == NULL) { hid_err(hdev, "can't allocate space for Minibox PicoLCD device data\n"); - error = -ENOMEM; - goto err_no_cleanup; + return -ENOMEM; } spin_lock_init(&data->lock); @@ -597,9 +596,6 @@ static int picolcd_probe(struct hid_device *hdev, hid_hw_stop(hdev); err_cleanup_data: kfree(data); -err_no_cleanup: - hid_set_drvdata(hdev, NULL); - return error; } @@ -635,7 +631,6 @@ static void picolcd_remove(struct hid_device *hdev) picolcd_exit_cir(data); picolcd_exit_keys(data); - hid_set_drvdata(hdev, NULL); mutex_destroy(&data->mutex); /* Finally, clean up the picolcd data itself */ kfree(data); diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c index be92a6f79687..94c7398b5c27 100644 --- a/drivers/hid/hid-sensor-hub.c +++ b/drivers/hid/hid-sensor-hub.c @@ -742,7 +742,6 @@ static void sensor_hub_remove(struct hid_device *hdev) } spin_unlock_irqrestore(&data->lock, flags); mfd_remove_devices(&hdev->dev); - hid_set_drvdata(hdev, NULL); mutex_destroy(&data->mutex); } -- 2.19.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] HID: do not call hid_set_drvdata(hdev, NULL) in drivers 2019-08-12 16:27 ` [PATCH 1/2] " Benjamin Tissoires @ 2019-08-12 16:39 ` Srinivas Pandruvada 2019-08-13 5:53 ` Bruno Prémont 1 sibling, 0 replies; 8+ messages in thread From: Srinivas Pandruvada @ 2019-08-12 16:39 UTC (permalink / raw) To: Benjamin Tissoires, Bruno Prémont, Jonathan Cameron, Ping Cheng, Jason Gerecke, Jiri Kosina Cc: linux-input, linux-kernel On Mon, 2019-08-12 at 18:27 +0200, Benjamin Tissoires wrote: > This is a common pattern in the HID drivers to reset the drvdata. > Some > do it properly, some do it only in case of failure. > > But, this is actually already handled by driver core, so there is no > need > to do it manually. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> for hid-sensor-hub.c Acked-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com> > --- > drivers/hid/hid-cougar.c | 6 ++---- > drivers/hid/hid-gfrm.c | 7 ------- > drivers/hid/hid-lenovo.c | 2 -- > drivers/hid/hid-picolcd_core.c | 7 +------ > drivers/hid/hid-sensor-hub.c | 1 - > 5 files changed, 3 insertions(+), 20 deletions(-) > > diff --git a/drivers/hid/hid-cougar.c b/drivers/hid/hid-cougar.c > index e0bb7b34f3a4..4ff3bc1d25e2 100644 > --- a/drivers/hid/hid-cougar.c > +++ b/drivers/hid/hid-cougar.c > @@ -207,7 +207,7 @@ static int cougar_probe(struct hid_device *hdev, > error = hid_parse(hdev); > if (error) { > hid_err(hdev, "parse failed\n"); > - goto fail; > + return error; > } > > if (hdev->collection->usage == COUGAR_VENDOR_USAGE) { > @@ -219,7 +219,7 @@ static int cougar_probe(struct hid_device *hdev, > error = hid_hw_start(hdev, connect_mask); > if (error) { > hid_err(hdev, "hw start failed\n"); > - goto fail; > + return error; > } > > error = cougar_bind_shared_data(hdev, cougar); > @@ -249,8 +249,6 @@ static int cougar_probe(struct hid_device *hdev, > > fail_stop_and_cleanup: > hid_hw_stop(hdev); > -fail: > - hid_set_drvdata(hdev, NULL); > return error; > } > > diff --git a/drivers/hid/hid-gfrm.c b/drivers/hid/hid-gfrm.c > index 86c317320bf2..699186ff2349 100644 > --- a/drivers/hid/hid-gfrm.c > +++ b/drivers/hid/hid-gfrm.c > @@ -123,12 +123,6 @@ static int gfrm_probe(struct hid_device *hdev, > const struct hid_device_id *id) > return ret; > } > > -static void gfrm_remove(struct hid_device *hdev) > -{ > - hid_hw_stop(hdev); > - hid_set_drvdata(hdev, NULL); > -} > - > static const struct hid_device_id gfrm_devices[] = { > { HID_BLUETOOTH_DEVICE(0x58, 0x2000), > .driver_data = GFRM100 }, > @@ -142,7 +136,6 @@ static struct hid_driver gfrm_driver = { > .name = "gfrm", > .id_table = gfrm_devices, > .probe = gfrm_probe, > - .remove = gfrm_remove, > .input_mapping = gfrm_input_mapping, > .raw_event = gfrm_raw_event, > .input_configured = gfrm_input_configured, > diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c > index 364bc7f11d9d..96fa2a2c2cd3 100644 > --- a/drivers/hid/hid-lenovo.c > +++ b/drivers/hid/hid-lenovo.c > @@ -866,8 +866,6 @@ static void lenovo_remove_tpkbd(struct hid_device > *hdev) > > led_classdev_unregister(&data_pointer->led_micmute); > led_classdev_unregister(&data_pointer->led_mute); > - > - hid_set_drvdata(hdev, NULL); > } > > static void lenovo_remove_cptkbd(struct hid_device *hdev) > diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid- > picolcd_core.c > index 5f7a39a5d4af..1b5c63241af0 100644 > --- a/drivers/hid/hid-picolcd_core.c > +++ b/drivers/hid/hid-picolcd_core.c > @@ -534,8 +534,7 @@ static int picolcd_probe(struct hid_device *hdev, > data = kzalloc(sizeof(struct picolcd_data), GFP_KERNEL); > if (data == NULL) { > hid_err(hdev, "can't allocate space for Minibox PicoLCD > device data\n"); > - error = -ENOMEM; > - goto err_no_cleanup; > + return -ENOMEM; > } > > spin_lock_init(&data->lock); > @@ -597,9 +596,6 @@ static int picolcd_probe(struct hid_device *hdev, > hid_hw_stop(hdev); > err_cleanup_data: > kfree(data); > -err_no_cleanup: > - hid_set_drvdata(hdev, NULL); > - > return error; > } > > @@ -635,7 +631,6 @@ static void picolcd_remove(struct hid_device > *hdev) > picolcd_exit_cir(data); > picolcd_exit_keys(data); > > - hid_set_drvdata(hdev, NULL); > mutex_destroy(&data->mutex); > /* Finally, clean up the picolcd data itself */ > kfree(data); > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor- > hub.c > index be92a6f79687..94c7398b5c27 100644 > --- a/drivers/hid/hid-sensor-hub.c > +++ b/drivers/hid/hid-sensor-hub.c > @@ -742,7 +742,6 @@ static void sensor_hub_remove(struct hid_device > *hdev) > } > spin_unlock_irqrestore(&data->lock, flags); > mfd_remove_devices(&hdev->dev); > - hid_set_drvdata(hdev, NULL); > mutex_destroy(&data->mutex); > } > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] HID: do not call hid_set_drvdata(hdev, NULL) in drivers 2019-08-12 16:27 ` [PATCH 1/2] " Benjamin Tissoires 2019-08-12 16:39 ` Srinivas Pandruvada @ 2019-08-13 5:53 ` Bruno Prémont 2019-08-22 15:50 ` Benjamin Tissoires 1 sibling, 1 reply; 8+ messages in thread From: Bruno Prémont @ 2019-08-13 5:53 UTC (permalink / raw) To: Benjamin Tissoires Cc: Jonathan Cameron, Srinivas Pandruvada, Ping Cheng, Jason Gerecke, Jiri Kosina, linux-input, linux-kernel On Mon, 12 Aug 2019 18:27:39 +0200 Benjamin Tissoires wrote: > This is a common pattern in the HID drivers to reset the drvdata. Some > do it properly, some do it only in case of failure. > > But, this is actually already handled by driver core, so there is no need > to do it manually. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> For hid-picolcd_core.c: Acked-by: Bruno Prémont <bonbons@linux-vserver.org> > --- > drivers/hid/hid-cougar.c | 6 ++---- > drivers/hid/hid-gfrm.c | 7 ------- > drivers/hid/hid-lenovo.c | 2 -- > drivers/hid/hid-picolcd_core.c | 7 +------ > drivers/hid/hid-sensor-hub.c | 1 - > 5 files changed, 3 insertions(+), 20 deletions(-) > > diff --git a/drivers/hid/hid-cougar.c b/drivers/hid/hid-cougar.c > index e0bb7b34f3a4..4ff3bc1d25e2 100644 > --- a/drivers/hid/hid-cougar.c > +++ b/drivers/hid/hid-cougar.c > @@ -207,7 +207,7 @@ static int cougar_probe(struct hid_device *hdev, > error = hid_parse(hdev); > if (error) { > hid_err(hdev, "parse failed\n"); > - goto fail; > + return error; > } > > if (hdev->collection->usage == COUGAR_VENDOR_USAGE) { > @@ -219,7 +219,7 @@ static int cougar_probe(struct hid_device *hdev, > error = hid_hw_start(hdev, connect_mask); > if (error) { > hid_err(hdev, "hw start failed\n"); > - goto fail; > + return error; > } > > error = cougar_bind_shared_data(hdev, cougar); > @@ -249,8 +249,6 @@ static int cougar_probe(struct hid_device *hdev, > > fail_stop_and_cleanup: > hid_hw_stop(hdev); > -fail: > - hid_set_drvdata(hdev, NULL); > return error; > } > > diff --git a/drivers/hid/hid-gfrm.c b/drivers/hid/hid-gfrm.c > index 86c317320bf2..699186ff2349 100644 > --- a/drivers/hid/hid-gfrm.c > +++ b/drivers/hid/hid-gfrm.c > @@ -123,12 +123,6 @@ static int gfrm_probe(struct hid_device *hdev, const struct hid_device_id *id) > return ret; > } > > -static void gfrm_remove(struct hid_device *hdev) > -{ > - hid_hw_stop(hdev); > - hid_set_drvdata(hdev, NULL); > -} > - > static const struct hid_device_id gfrm_devices[] = { > { HID_BLUETOOTH_DEVICE(0x58, 0x2000), > .driver_data = GFRM100 }, > @@ -142,7 +136,6 @@ static struct hid_driver gfrm_driver = { > .name = "gfrm", > .id_table = gfrm_devices, > .probe = gfrm_probe, > - .remove = gfrm_remove, > .input_mapping = gfrm_input_mapping, > .raw_event = gfrm_raw_event, > .input_configured = gfrm_input_configured, > diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c > index 364bc7f11d9d..96fa2a2c2cd3 100644 > --- a/drivers/hid/hid-lenovo.c > +++ b/drivers/hid/hid-lenovo.c > @@ -866,8 +866,6 @@ static void lenovo_remove_tpkbd(struct hid_device *hdev) > > led_classdev_unregister(&data_pointer->led_micmute); > led_classdev_unregister(&data_pointer->led_mute); > - > - hid_set_drvdata(hdev, NULL); > } > > static void lenovo_remove_cptkbd(struct hid_device *hdev) > diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c > index 5f7a39a5d4af..1b5c63241af0 100644 > --- a/drivers/hid/hid-picolcd_core.c > +++ b/drivers/hid/hid-picolcd_core.c > @@ -534,8 +534,7 @@ static int picolcd_probe(struct hid_device *hdev, > data = kzalloc(sizeof(struct picolcd_data), GFP_KERNEL); > if (data == NULL) { > hid_err(hdev, "can't allocate space for Minibox PicoLCD device data\n"); > - error = -ENOMEM; > - goto err_no_cleanup; > + return -ENOMEM; > } > > spin_lock_init(&data->lock); > @@ -597,9 +596,6 @@ static int picolcd_probe(struct hid_device *hdev, > hid_hw_stop(hdev); > err_cleanup_data: > kfree(data); > -err_no_cleanup: > - hid_set_drvdata(hdev, NULL); > - > return error; > } > > @@ -635,7 +631,6 @@ static void picolcd_remove(struct hid_device *hdev) > picolcd_exit_cir(data); > picolcd_exit_keys(data); > > - hid_set_drvdata(hdev, NULL); > mutex_destroy(&data->mutex); > /* Finally, clean up the picolcd data itself */ > kfree(data); > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c > index be92a6f79687..94c7398b5c27 100644 > --- a/drivers/hid/hid-sensor-hub.c > +++ b/drivers/hid/hid-sensor-hub.c > @@ -742,7 +742,6 @@ static void sensor_hub_remove(struct hid_device *hdev) > } > spin_unlock_irqrestore(&data->lock, flags); > mfd_remove_devices(&hdev->dev); > - hid_set_drvdata(hdev, NULL); > mutex_destroy(&data->mutex); > } > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/2] HID: do not call hid_set_drvdata(hdev, NULL) in drivers 2019-08-13 5:53 ` Bruno Prémont @ 2019-08-22 15:50 ` Benjamin Tissoires 0 siblings, 0 replies; 8+ messages in thread From: Benjamin Tissoires @ 2019-08-22 15:50 UTC (permalink / raw) To: Bruno Prémont Cc: Jonathan Cameron, Srinivas Pandruvada, Ping Cheng, Jason Gerecke, Jiri Kosina, open list:HID CORE LAYER, lkml On Tue, Aug 13, 2019 at 7:54 AM Bruno Prémont <bonbons@linux-vserver.org> wrote: > > On Mon, 12 Aug 2019 18:27:39 +0200 Benjamin Tissoires wrote: > > This is a common pattern in the HID drivers to reset the drvdata. Some > > do it properly, some do it only in case of failure. > > > > But, this is actually already handled by driver core, so there is no need > > to do it manually. > > > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > For hid-picolcd_core.c: > Acked-by: Bruno Prémont <bonbons@linux-vserver.org> Thanks for the acks. Applied to for-5.4/cleanup Cheers, Benjamin > > > --- > > drivers/hid/hid-cougar.c | 6 ++---- > > drivers/hid/hid-gfrm.c | 7 ------- > > drivers/hid/hid-lenovo.c | 2 -- > > drivers/hid/hid-picolcd_core.c | 7 +------ > > drivers/hid/hid-sensor-hub.c | 1 - > > 5 files changed, 3 insertions(+), 20 deletions(-) > > > > diff --git a/drivers/hid/hid-cougar.c b/drivers/hid/hid-cougar.c > > index e0bb7b34f3a4..4ff3bc1d25e2 100644 > > --- a/drivers/hid/hid-cougar.c > > +++ b/drivers/hid/hid-cougar.c > > @@ -207,7 +207,7 @@ static int cougar_probe(struct hid_device *hdev, > > error = hid_parse(hdev); > > if (error) { > > hid_err(hdev, "parse failed\n"); > > - goto fail; > > + return error; > > } > > > > if (hdev->collection->usage == COUGAR_VENDOR_USAGE) { > > @@ -219,7 +219,7 @@ static int cougar_probe(struct hid_device *hdev, > > error = hid_hw_start(hdev, connect_mask); > > if (error) { > > hid_err(hdev, "hw start failed\n"); > > - goto fail; > > + return error; > > } > > > > error = cougar_bind_shared_data(hdev, cougar); > > @@ -249,8 +249,6 @@ static int cougar_probe(struct hid_device *hdev, > > > > fail_stop_and_cleanup: > > hid_hw_stop(hdev); > > -fail: > > - hid_set_drvdata(hdev, NULL); > > return error; > > } > > > > diff --git a/drivers/hid/hid-gfrm.c b/drivers/hid/hid-gfrm.c > > index 86c317320bf2..699186ff2349 100644 > > --- a/drivers/hid/hid-gfrm.c > > +++ b/drivers/hid/hid-gfrm.c > > @@ -123,12 +123,6 @@ static int gfrm_probe(struct hid_device *hdev, const struct hid_device_id *id) > > return ret; > > } > > > > -static void gfrm_remove(struct hid_device *hdev) > > -{ > > - hid_hw_stop(hdev); > > - hid_set_drvdata(hdev, NULL); > > -} > > - > > static const struct hid_device_id gfrm_devices[] = { > > { HID_BLUETOOTH_DEVICE(0x58, 0x2000), > > .driver_data = GFRM100 }, > > @@ -142,7 +136,6 @@ static struct hid_driver gfrm_driver = { > > .name = "gfrm", > > .id_table = gfrm_devices, > > .probe = gfrm_probe, > > - .remove = gfrm_remove, > > .input_mapping = gfrm_input_mapping, > > .raw_event = gfrm_raw_event, > > .input_configured = gfrm_input_configured, > > diff --git a/drivers/hid/hid-lenovo.c b/drivers/hid/hid-lenovo.c > > index 364bc7f11d9d..96fa2a2c2cd3 100644 > > --- a/drivers/hid/hid-lenovo.c > > +++ b/drivers/hid/hid-lenovo.c > > @@ -866,8 +866,6 @@ static void lenovo_remove_tpkbd(struct hid_device *hdev) > > > > led_classdev_unregister(&data_pointer->led_micmute); > > led_classdev_unregister(&data_pointer->led_mute); > > - > > - hid_set_drvdata(hdev, NULL); > > } > > > > static void lenovo_remove_cptkbd(struct hid_device *hdev) > > diff --git a/drivers/hid/hid-picolcd_core.c b/drivers/hid/hid-picolcd_core.c > > index 5f7a39a5d4af..1b5c63241af0 100644 > > --- a/drivers/hid/hid-picolcd_core.c > > +++ b/drivers/hid/hid-picolcd_core.c > > @@ -534,8 +534,7 @@ static int picolcd_probe(struct hid_device *hdev, > > data = kzalloc(sizeof(struct picolcd_data), GFP_KERNEL); > > if (data == NULL) { > > hid_err(hdev, "can't allocate space for Minibox PicoLCD device data\n"); > > - error = -ENOMEM; > > - goto err_no_cleanup; > > + return -ENOMEM; > > } > > > > spin_lock_init(&data->lock); > > @@ -597,9 +596,6 @@ static int picolcd_probe(struct hid_device *hdev, > > hid_hw_stop(hdev); > > err_cleanup_data: > > kfree(data); > > -err_no_cleanup: > > - hid_set_drvdata(hdev, NULL); > > - > > return error; > > } > > > > @@ -635,7 +631,6 @@ static void picolcd_remove(struct hid_device *hdev) > > picolcd_exit_cir(data); > > picolcd_exit_keys(data); > > > > - hid_set_drvdata(hdev, NULL); > > mutex_destroy(&data->mutex); > > /* Finally, clean up the picolcd data itself */ > > kfree(data); > > diff --git a/drivers/hid/hid-sensor-hub.c b/drivers/hid/hid-sensor-hub.c > > index be92a6f79687..94c7398b5c27 100644 > > --- a/drivers/hid/hid-sensor-hub.c > > +++ b/drivers/hid/hid-sensor-hub.c > > @@ -742,7 +742,6 @@ static void sensor_hub_remove(struct hid_device *hdev) > > } > > spin_unlock_irqrestore(&data->lock, flags); > > mfd_remove_devices(&hdev->dev); > > - hid_set_drvdata(hdev, NULL); > > mutex_destroy(&data->mutex); > > } > > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH 2/2] HID: wacom: do not call hid_set_drvdata(hdev, NULL) 2019-08-12 16:27 [PATCH 0/2] HID: do not call hid_set_drvdata(hdev, NULL) in drivers Benjamin Tissoires 2019-08-12 16:27 ` [PATCH 1/2] " Benjamin Tissoires @ 2019-08-12 16:27 ` Benjamin Tissoires 2019-08-16 22:03 ` Jason Gerecke 1 sibling, 1 reply; 8+ messages in thread From: Benjamin Tissoires @ 2019-08-12 16:27 UTC (permalink / raw) To: Bruno Prémont, Jonathan Cameron, Srinivas Pandruvada, Ping Cheng, Jason Gerecke, Jiri Kosina Cc: linux-input, linux-kernel, Benjamin Tissoires This is a common pattern in the HID drivers to reset the drvdata. However, this is actually already handled by driver core, so there is no need to do it manually. Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/hid/wacom_sys.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-) diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c index 53bddb50aeba..69ccfdd51a6f 100644 --- a/drivers/hid/wacom_sys.c +++ b/drivers/hid/wacom_sys.c @@ -2718,14 +2718,12 @@ static int wacom_probe(struct hid_device *hdev, wacom_wac->features = *((struct wacom_features *)id->driver_data); features = &wacom_wac->features; - if (features->check_for_hid_type && features->hid_type != hdev->type) { - error = -ENODEV; - goto fail; - } + if (features->check_for_hid_type && features->hid_type != hdev->type) + return -ENODEV; error = kfifo_alloc(&wacom_wac->pen_fifo, WACOM_PKGLEN_MAX, GFP_KERNEL); if (error) - goto fail; + return error; wacom_wac->hid_data.inputmode = -1; wacom_wac->mode_report = -1; @@ -2743,12 +2741,12 @@ static int wacom_probe(struct hid_device *hdev, error = hid_parse(hdev); if (error) { hid_err(hdev, "parse failed\n"); - goto fail; + return error; } error = wacom_parse_and_register(wacom, false); if (error) - goto fail; + return error; if (hdev->bus == BUS_BLUETOOTH) { error = device_create_file(&hdev->dev, &dev_attr_speed); @@ -2759,10 +2757,6 @@ static int wacom_probe(struct hid_device *hdev, } return 0; - -fail: - hid_set_drvdata(hdev, NULL); - return error; } static void wacom_remove(struct hid_device *hdev) @@ -2791,8 +2785,6 @@ static void wacom_remove(struct hid_device *hdev) wacom_release_resources(wacom); kfifo_free(&wacom_wac->pen_fifo); - - hid_set_drvdata(hdev, NULL); } #ifdef CONFIG_PM -- 2.19.2 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] HID: wacom: do not call hid_set_drvdata(hdev, NULL) 2019-08-12 16:27 ` [PATCH 2/2] HID: wacom: do not call hid_set_drvdata(hdev, NULL) Benjamin Tissoires @ 2019-08-16 22:03 ` Jason Gerecke 2019-08-22 15:49 ` Benjamin Tissoires 0 siblings, 1 reply; 8+ messages in thread From: Jason Gerecke @ 2019-08-16 22:03 UTC (permalink / raw) To: Benjamin Tissoires Cc: Bruno Prémont, Jonathan Cameron, Srinivas Pandruvada, Ping Cheng, Jason Gerecke, Jiri Kosina, Linux Input, LKML On Mon, Aug 12, 2019 at 9:29 AM Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > > This is a common pattern in the HID drivers to reset the drvdata. > However, this is actually already handled by driver core, so there > is no need to do it manually. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Acked-by: Jason Gerecke <jason.gerecke@wacom.com> Jason --- Now instead of four in the eights place / you’ve got three, ‘Cause you added one / (That is to say, eight) to the two, / But you can’t take seven from three, / So you look at the sixty-fours.... > --- > drivers/hid/wacom_sys.c | 18 +++++------------- > 1 file changed, 5 insertions(+), 13 deletions(-) > > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c > index 53bddb50aeba..69ccfdd51a6f 100644 > --- a/drivers/hid/wacom_sys.c > +++ b/drivers/hid/wacom_sys.c > @@ -2718,14 +2718,12 @@ static int wacom_probe(struct hid_device *hdev, > wacom_wac->features = *((struct wacom_features *)id->driver_data); > features = &wacom_wac->features; > > - if (features->check_for_hid_type && features->hid_type != hdev->type) { > - error = -ENODEV; > - goto fail; > - } > + if (features->check_for_hid_type && features->hid_type != hdev->type) > + return -ENODEV; > > error = kfifo_alloc(&wacom_wac->pen_fifo, WACOM_PKGLEN_MAX, GFP_KERNEL); > if (error) > - goto fail; > + return error; > > wacom_wac->hid_data.inputmode = -1; > wacom_wac->mode_report = -1; > @@ -2743,12 +2741,12 @@ static int wacom_probe(struct hid_device *hdev, > error = hid_parse(hdev); > if (error) { > hid_err(hdev, "parse failed\n"); > - goto fail; > + return error; > } > > error = wacom_parse_and_register(wacom, false); > if (error) > - goto fail; > + return error; > > if (hdev->bus == BUS_BLUETOOTH) { > error = device_create_file(&hdev->dev, &dev_attr_speed); > @@ -2759,10 +2757,6 @@ static int wacom_probe(struct hid_device *hdev, > } > > return 0; > - > -fail: > - hid_set_drvdata(hdev, NULL); > - return error; > } > > static void wacom_remove(struct hid_device *hdev) > @@ -2791,8 +2785,6 @@ static void wacom_remove(struct hid_device *hdev) > wacom_release_resources(wacom); > > kfifo_free(&wacom_wac->pen_fifo); > - > - hid_set_drvdata(hdev, NULL); > } > > #ifdef CONFIG_PM > -- > 2.19.2 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 2/2] HID: wacom: do not call hid_set_drvdata(hdev, NULL) 2019-08-16 22:03 ` Jason Gerecke @ 2019-08-22 15:49 ` Benjamin Tissoires 0 siblings, 0 replies; 8+ messages in thread From: Benjamin Tissoires @ 2019-08-22 15:49 UTC (permalink / raw) To: Jason Gerecke Cc: Bruno Prémont, Jonathan Cameron, Srinivas Pandruvada, Ping Cheng, Jason Gerecke, Jiri Kosina, Linux Input, LKML On Sat, Aug 17, 2019 at 12:04 AM Jason Gerecke <killertofu@gmail.com> wrote: > > On Mon, Aug 12, 2019 at 9:29 AM Benjamin Tissoires > <benjamin.tissoires@redhat.com> wrote: > > > > This is a common pattern in the HID drivers to reset the drvdata. > > However, this is actually already handled by driver core, so there > > is no need to do it manually. > > > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > Acked-by: Jason Gerecke <jason.gerecke@wacom.com> Thanks Applied to for-5.4/wacom Cheers, Benjamin > > Jason > --- > Now instead of four in the eights place / > you’ve got three, ‘Cause you added one / > (That is to say, eight) to the two, / > But you can’t take seven from three, / > So you look at the sixty-fours.... > > > > --- > > drivers/hid/wacom_sys.c | 18 +++++------------- > > 1 file changed, 5 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/hid/wacom_sys.c b/drivers/hid/wacom_sys.c > > index 53bddb50aeba..69ccfdd51a6f 100644 > > --- a/drivers/hid/wacom_sys.c > > +++ b/drivers/hid/wacom_sys.c > > @@ -2718,14 +2718,12 @@ static int wacom_probe(struct hid_device *hdev, > > wacom_wac->features = *((struct wacom_features *)id->driver_data); > > features = &wacom_wac->features; > > > > - if (features->check_for_hid_type && features->hid_type != hdev->type) { > > - error = -ENODEV; > > - goto fail; > > - } > > + if (features->check_for_hid_type && features->hid_type != hdev->type) > > + return -ENODEV; > > > > error = kfifo_alloc(&wacom_wac->pen_fifo, WACOM_PKGLEN_MAX, GFP_KERNEL); > > if (error) > > - goto fail; > > + return error; > > > > wacom_wac->hid_data.inputmode = -1; > > wacom_wac->mode_report = -1; > > @@ -2743,12 +2741,12 @@ static int wacom_probe(struct hid_device *hdev, > > error = hid_parse(hdev); > > if (error) { > > hid_err(hdev, "parse failed\n"); > > - goto fail; > > + return error; > > } > > > > error = wacom_parse_and_register(wacom, false); > > if (error) > > - goto fail; > > + return error; > > > > if (hdev->bus == BUS_BLUETOOTH) { > > error = device_create_file(&hdev->dev, &dev_attr_speed); > > @@ -2759,10 +2757,6 @@ static int wacom_probe(struct hid_device *hdev, > > } > > > > return 0; > > - > > -fail: > > - hid_set_drvdata(hdev, NULL); > > - return error; > > } > > > > static void wacom_remove(struct hid_device *hdev) > > @@ -2791,8 +2785,6 @@ static void wacom_remove(struct hid_device *hdev) > > wacom_release_resources(wacom); > > > > kfifo_free(&wacom_wac->pen_fifo); > > - > > - hid_set_drvdata(hdev, NULL); > > } > > > > #ifdef CONFIG_PM > > -- > > 2.19.2 > > ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2019-08-22 15:50 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2019-08-12 16:27 [PATCH 0/2] HID: do not call hid_set_drvdata(hdev, NULL) in drivers Benjamin Tissoires 2019-08-12 16:27 ` [PATCH 1/2] " Benjamin Tissoires 2019-08-12 16:39 ` Srinivas Pandruvada 2019-08-13 5:53 ` Bruno Prémont 2019-08-22 15:50 ` Benjamin Tissoires 2019-08-12 16:27 ` [PATCH 2/2] HID: wacom: do not call hid_set_drvdata(hdev, NULL) Benjamin Tissoires 2019-08-16 22:03 ` Jason Gerecke 2019-08-22 15:49 ` Benjamin Tissoires
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).