| Message ID | 20231006081858.17677-3-hdegoede@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: logitech-hidpp: Avoid hidpp_connect_event() running while probe() restarts IO | expand |
Hi, On 10/6/23 10:18, Hans de Goede wrote: > Drop delayed_work_cb() instead make hidpp_connect_event() the workqueue > function itself. > > Besides resulting in a small cleanup this will hopefully also make > it clearer that going forward hidpp_connect_event() should only > be run from a workqueue and not be directly involved. Typo: s/involved/invoked/ Regards, Hans > > Signed-off-by: Hans de Goede <hdegoede@redhat.com> > --- > drivers/hid/hid-logitech-hidpp.c | 14 +++----------- > 1 file changed, 3 insertions(+), 11 deletions(-) > > diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c > index 33f9cd98485a..15c36112902b 100644 > --- a/drivers/hid/hid-logitech-hidpp.c > +++ b/drivers/hid/hid-logitech-hidpp.c > @@ -236,8 +236,6 @@ struct hidpp_device { > #define HIDPP20_ERROR_UNSUPPORTED 0x09 > #define HIDPP20_ERROR 0xff > > -static void hidpp_connect_event(struct hidpp_device *hidpp_dev); > - > static int __hidpp_send_report(struct hid_device *hdev, > struct hidpp_report *hidpp_report) > { > @@ -451,13 +449,6 @@ static int hidpp_send_rap_command_sync(struct hidpp_device *hidpp_dev, > return ret; > } > > -static void delayed_work_cb(struct work_struct *work) > -{ > - struct hidpp_device *hidpp = container_of(work, struct hidpp_device, > - work); > - hidpp_connect_event(hidpp); > -} > - > static inline bool hidpp_match_answer(struct hidpp_report *question, > struct hidpp_report *answer) > { > @@ -4190,8 +4181,9 @@ static struct input_dev *hidpp_allocate_input(struct hid_device *hdev) > return input_dev; > } > > -static void hidpp_connect_event(struct hidpp_device *hidpp) > +static void hidpp_connect_event(struct work_struct *work) > { > + struct hidpp_device *hidpp = container_of(work, struct hidpp_device, work); > struct hid_device *hdev = hidpp->hid_dev; > int ret = 0; > bool connected = atomic_read(&hidpp->connected); > @@ -4455,7 +4447,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id) > hidpp->quirks & HIDPP_QUIRK_UNIFYING) > will_restart = true; > > - INIT_WORK(&hidpp->work, delayed_work_cb); > + INIT_WORK(&hidpp->work, hidpp_connect_event); > mutex_init(&hidpp->io_mutex); > mutex_init(&hidpp->send_mutex); > init_waitqueue_head(&hidpp->wait);
diff --git a/drivers/hid/hid-logitech-hidpp.c b/drivers/hid/hid-logitech-hidpp.c index 33f9cd98485a..15c36112902b 100644 --- a/drivers/hid/hid-logitech-hidpp.c +++ b/drivers/hid/hid-logitech-hidpp.c @@ -236,8 +236,6 @@ struct hidpp_device { #define HIDPP20_ERROR_UNSUPPORTED 0x09 #define HIDPP20_ERROR 0xff -static void hidpp_connect_event(struct hidpp_device *hidpp_dev); - static int __hidpp_send_report(struct hid_device *hdev, struct hidpp_report *hidpp_report) { @@ -451,13 +449,6 @@ static int hidpp_send_rap_command_sync(struct hidpp_device *hidpp_dev, return ret; } -static void delayed_work_cb(struct work_struct *work) -{ - struct hidpp_device *hidpp = container_of(work, struct hidpp_device, - work); - hidpp_connect_event(hidpp); -} - static inline bool hidpp_match_answer(struct hidpp_report *question, struct hidpp_report *answer) { @@ -4190,8 +4181,9 @@ static struct input_dev *hidpp_allocate_input(struct hid_device *hdev) return input_dev; } -static void hidpp_connect_event(struct hidpp_device *hidpp) +static void hidpp_connect_event(struct work_struct *work) { + struct hidpp_device *hidpp = container_of(work, struct hidpp_device, work); struct hid_device *hdev = hidpp->hid_dev; int ret = 0; bool connected = atomic_read(&hidpp->connected); @@ -4455,7 +4447,7 @@ static int hidpp_probe(struct hid_device *hdev, const struct hid_device_id *id) hidpp->quirks & HIDPP_QUIRK_UNIFYING) will_restart = true; - INIT_WORK(&hidpp->work, delayed_work_cb); + INIT_WORK(&hidpp->work, hidpp_connect_event); mutex_init(&hidpp->io_mutex); mutex_init(&hidpp->send_mutex); init_waitqueue_head(&hidpp->wait);
Drop delayed_work_cb() instead make hidpp_connect_event() the workqueue function itself. Besides resulting in a small cleanup this will hopefully also make it clearer that going forward hidpp_connect_event() should only be run from a workqueue and not be directly involved. Signed-off-by: Hans de Goede <hdegoede@redhat.com> --- drivers/hid/hid-logitech-hidpp.c | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-)