* [PATCH 0/2] Bluetooth: hidp: cleanup HID init and re-enable hidinput_input_event callback @ 2013-07-11 13:41 Benjamin Tissoires 2013-07-11 13:41 ` Benjamin Tissoires 2013-07-11 13:41 ` [PATCH 2/2] Bluetooth: hidp: remove wrong send_report at init Benjamin Tissoires 0 siblings, 2 replies; 15+ messages in thread From: Benjamin Tissoires @ 2013-07-11 13:41 UTC (permalink / raw) To: Benjamin Tissoires, Marcel Holtmann, Gustavo Padovan, David Herrmann, Jiri Kosina, linux-bluetooth, linux-input, linux-kernel Hi guys, These two patches enhance a little bit the HIDp driver: - the first one re-implement in a safer way the callback hidinput_input_event, allowing keyboards leds to be set over bluetooth - the second one disable the send_report commands called during init, which are not part of the spec and may fail with some devices (the Wiimote is in this case, but David guarded it against this problem). Cheers, Benjamin Benjamin Tissoires (2): Bluetooth: hidp: implement hidinput_input_event callback Bluetooth: hidp: remove wrong send_report at init net/bluetooth/hidp/core.c | 40 ++++++++++++++++++++++++++-------------- 1 file changed, 26 insertions(+), 14 deletions(-) -- 1.8.3.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 1/2] Bluetooth: hidp: implement hidinput_input_event callback @ 2013-07-11 13:41 ` Benjamin Tissoires 0 siblings, 0 replies; 15+ messages in thread From: Benjamin Tissoires @ 2013-07-11 13:41 UTC (permalink / raw) To: Benjamin Tissoires, Marcel Holtmann, Gustavo Padovan, David Herrmann, Jiri Kosina, linux-bluetooth, linux-input, linux-kernel We can re-enable hidinput_input_event to allow the leds of bluetooth keyboards to be set. Now the callbacks uses hid core to retrieve the right HID report to send, so this version is safer. Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- net/bluetooth/hidp/core.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c index f13a8da..9c8b50d 100644 --- a/net/bluetooth/hidp/core.c +++ b/net/bluetooth/hidp/core.c @@ -238,6 +238,31 @@ static int hidp_send_report(struct hidp_session *session, struct hid_report *rep return hidp_send_intr_message(session, hdr, buf, rsize); } +static int hidp_hidinput_event(struct input_dev *dev, unsigned int type, + unsigned int code, int value) +{ + struct hid_device *hid = input_get_drvdata(dev); + struct hidp_session *session = hid->driver_data; + struct hid_field *field; + int offset; + + BT_DBG("session %p type %d code %d value %d", + session, type, code, value); + + if (type != EV_LED) + return -1; + + offset = hidinput_find_field(hid, type, code, &field); + if (offset == -1) { + hid_warn(dev, "event field not found\n"); + return -1; + } + + hid_set_field(field, offset, value); + + return hidp_send_report(session, field->report); +} + static int hidp_get_raw_report(struct hid_device *hid, unsigned char report_number, unsigned char *data, size_t count, @@ -711,6 +736,7 @@ static struct hid_ll_driver hidp_hid_driver = { .stop = hidp_stop, .open = hidp_open, .close = hidp_close, + .hidinput_input_event = hidp_hidinput_event, }; /* This function sets up the hid device. It does not add it -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH 1/2] Bluetooth: hidp: implement hidinput_input_event callback @ 2013-07-11 13:41 ` Benjamin Tissoires 0 siblings, 0 replies; 15+ messages in thread From: Benjamin Tissoires @ 2013-07-11 13:41 UTC (permalink / raw) To: Benjamin Tissoires, Marcel Holtmann, Gustavo Padovan, David Herrmann, Jiri Kosina, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA We can re-enable hidinput_input_event to allow the leds of bluetooth keyboards to be set. Now the callbacks uses hid core to retrieve the right HID report to send, so this version is safer. Signed-off-by: Benjamin Tissoires <benjamin.tissoires-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> --- net/bluetooth/hidp/core.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c index f13a8da..9c8b50d 100644 --- a/net/bluetooth/hidp/core.c +++ b/net/bluetooth/hidp/core.c @@ -238,6 +238,31 @@ static int hidp_send_report(struct hidp_session *session, struct hid_report *rep return hidp_send_intr_message(session, hdr, buf, rsize); } +static int hidp_hidinput_event(struct input_dev *dev, unsigned int type, + unsigned int code, int value) +{ + struct hid_device *hid = input_get_drvdata(dev); + struct hidp_session *session = hid->driver_data; + struct hid_field *field; + int offset; + + BT_DBG("session %p type %d code %d value %d", + session, type, code, value); + + if (type != EV_LED) + return -1; + + offset = hidinput_find_field(hid, type, code, &field); + if (offset == -1) { + hid_warn(dev, "event field not found\n"); + return -1; + } + + hid_set_field(field, offset, value); + + return hidp_send_report(session, field->report); +} + static int hidp_get_raw_report(struct hid_device *hid, unsigned char report_number, unsigned char *data, size_t count, @@ -711,6 +736,7 @@ static struct hid_ll_driver hidp_hid_driver = { .stop = hidp_stop, .open = hidp_open, .close = hidp_close, + .hidinput_input_event = hidp_hidinput_event, }; /* This function sets up the hid device. It does not add it -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] Bluetooth: hidp: implement hidinput_input_event callback 2013-07-11 13:41 ` Benjamin Tissoires (?) @ 2013-07-11 14:02 ` David Herrmann 2013-07-11 14:10 ` Benjamin Tissoires -1 siblings, 1 reply; 15+ messages in thread From: David Herrmann @ 2013-07-11 14:02 UTC (permalink / raw) To: Benjamin Tissoires Cc: Benjamin Tissoires, Marcel Holtmann, Gustavo Padovan, Jiri Kosina, linux-bluetooth, open list:HID CORE LAYER, linux-kernel Hi On Thu, Jul 11, 2013 at 3:41 PM, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > We can re-enable hidinput_input_event to allow the leds of bluetooth > keyboards to be set. > Now the callbacks uses hid core to retrieve the right HID report to > send, so this version is safer. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > net/bluetooth/hidp/core.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c > index f13a8da..9c8b50d 100644 > --- a/net/bluetooth/hidp/core.c > +++ b/net/bluetooth/hidp/core.c > @@ -238,6 +238,31 @@ static int hidp_send_report(struct hidp_session *session, struct hid_report *rep > return hidp_send_intr_message(session, hdr, buf, rsize); > } > > +static int hidp_hidinput_event(struct input_dev *dev, unsigned int type, > + unsigned int code, int value) > +{ > + struct hid_device *hid = input_get_drvdata(dev); I dislike that we have to deal with input_get_drvdata() in a transport driver. It is not obvious from the transport driver that this gives us an hid_device. But ok, that's how we do it in usbhid and hid-input so it's another issue.. > + struct hidp_session *session = hid->driver_data; > + struct hid_field *field; > + int offset; > + > + BT_DBG("session %p type %d code %d value %d", > + session, type, code, value); > + > + if (type != EV_LED) > + return -1; > + > + offset = hidinput_find_field(hid, type, code, &field); > + if (offset == -1) { > + hid_warn(dev, "event field not found\n"); > + return -1; > + } > + > + hid_set_field(field, offset, value); > + > + return hidp_send_report(session, field->report); > +} > + We had this discussion before (regarding uhid and hidpinput_event), it would be nice to have a helper in hid-input.c which does this. We copy the code into every transport driver, which bugs me. But no-one stepped up to do this, so I am fine. Reviewed-by: David Herrmann <dh.herrmann@gmail.com> Thanks! David > static int hidp_get_raw_report(struct hid_device *hid, > unsigned char report_number, > unsigned char *data, size_t count, > @@ -711,6 +736,7 @@ static struct hid_ll_driver hidp_hid_driver = { > .stop = hidp_stop, > .open = hidp_open, > .close = hidp_close, > + .hidinput_input_event = hidp_hidinput_event, > }; > > /* This function sets up the hid device. It does not add it > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] Bluetooth: hidp: implement hidinput_input_event callback 2013-07-11 14:02 ` David Herrmann @ 2013-07-11 14:10 ` Benjamin Tissoires 2013-07-11 16:07 ` David Herrmann 0 siblings, 1 reply; 15+ messages in thread From: Benjamin Tissoires @ 2013-07-11 14:10 UTC (permalink / raw) To: David Herrmann Cc: Benjamin Tissoires, Marcel Holtmann, Gustavo Padovan, Jiri Kosina, linux-bluetooth, open list:HID CORE LAYER, linux-kernel Hi David, On Thu, Jul 11, 2013 at 4:02 PM, David Herrmann <dh.herrmann@gmail.com> wrote: > Hi > >> +static int hidp_hidinput_event(struct input_dev *dev, unsigned int type, >> + unsigned int code, int value) >> +{ >> + struct hid_device *hid = input_get_drvdata(dev); > > I dislike that we have to deal with input_get_drvdata() in a transport > driver. It is not obvious from the transport driver that this gives us > an hid_device. But ok, that's how we do it in usbhid and hid-input so > it's another issue.. right. So we should definitively change the callback signature here. > >> + struct hidp_session *session = hid->driver_data; >> + struct hid_field *field; >> + int offset; >> + >> + BT_DBG("session %p type %d code %d value %d", >> + session, type, code, value); >> + >> + if (type != EV_LED) >> + return -1; >> + >> + offset = hidinput_find_field(hid, type, code, &field); >> + if (offset == -1) { >> + hid_warn(dev, "event field not found\n"); >> + return -1; >> + } >> + >> + hid_set_field(field, offset, value); >> + >> + return hidp_send_report(session, field->report); >> +} >> + > > We had this discussion before (regarding uhid and hidpinput_event), it > would be nice to have a helper in hid-input.c which does this. We copy > the code into every transport driver, which bugs me. But no-one > stepped up to do this, so I am fine. oh, yes, you are right. I'll add it to my todo list (now that I am responsible of 2/4 code duplication :-S ) > > Reviewed-by: David Herrmann <dh.herrmann@gmail.com> Thanks for the review! Cheers, Benjamin > > Thanks! > David > >> static int hidp_get_raw_report(struct hid_device *hid, >> unsigned char report_number, >> unsigned char *data, size_t count, >> @@ -711,6 +736,7 @@ static struct hid_ll_driver hidp_hid_driver = { >> .stop = hidp_stop, >> .open = hidp_open, >> .close = hidp_close, >> + .hidinput_input_event = hidp_hidinput_event, >> }; >> >> /* This function sets up the hid device. It does not add it >> -- >> 1.8.3.1 >> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] Bluetooth: hidp: implement hidinput_input_event callback 2013-07-11 14:10 ` Benjamin Tissoires @ 2013-07-11 16:07 ` David Herrmann 0 siblings, 0 replies; 15+ messages in thread From: David Herrmann @ 2013-07-11 16:07 UTC (permalink / raw) To: Benjamin Tissoires Cc: Benjamin Tissoires, Marcel Holtmann, Gustavo Padovan, Jiri Kosina, linux-bluetooth, open list:HID CORE LAYER, linux-kernel Hi On Thu, Jul 11, 2013 at 4:10 PM, Benjamin Tissoires <benjamin.tissoires@gmail.com> wrote: > Hi David, > > On Thu, Jul 11, 2013 at 4:02 PM, David Herrmann <dh.herrmann@gmail.com> wrote: >> Hi >> >>> +static int hidp_hidinput_event(struct input_dev *dev, unsigned int type, >>> + unsigned int code, int value) >>> +{ >>> + struct hid_device *hid = input_get_drvdata(dev); >> >> I dislike that we have to deal with input_get_drvdata() in a transport >> driver. It is not obvious from the transport driver that this gives us >> an hid_device. But ok, that's how we do it in usbhid and hid-input so >> it's another issue.. > > right. So we should definitively change the callback signature here. > >> >>> + struct hidp_session *session = hid->driver_data; >>> + struct hid_field *field; >>> + int offset; >>> + >>> + BT_DBG("session %p type %d code %d value %d", >>> + session, type, code, value); >>> + >>> + if (type != EV_LED) >>> + return -1; >>> + >>> + offset = hidinput_find_field(hid, type, code, &field); >>> + if (offset == -1) { >>> + hid_warn(dev, "event field not found\n"); >>> + return -1; >>> + } >>> + >>> + hid_set_field(field, offset, value); >>> + >>> + return hidp_send_report(session, field->report); >>> +} >>> + >> >> We had this discussion before (regarding uhid and hidpinput_event), it >> would be nice to have a helper in hid-input.c which does this. We copy >> the code into every transport driver, which bugs me. But no-one >> stepped up to do this, so I am fine. > > oh, yes, you are right. I'll add it to my todo list (now that I am > responsible of 2/4 code duplication :-S ) Feel free to push this series forward unchanged, if it is urgent. I have an experimental series here which implements a generic hidinput_input_event() handler (including the scheduled worker) that I will send later today or tomorrow. Cheers David >> >> Reviewed-by: David Herrmann <dh.herrmann@gmail.com> > > Thanks for the review! > > Cheers, > Benjamin > >> >> Thanks! >> David >> >>> static int hidp_get_raw_report(struct hid_device *hid, >>> unsigned char report_number, >>> unsigned char *data, size_t count, >>> @@ -711,6 +736,7 @@ static struct hid_ll_driver hidp_hid_driver = { >>> .stop = hidp_stop, >>> .open = hidp_open, >>> .close = hidp_close, >>> + .hidinput_input_event = hidp_hidinput_event, >>> }; >>> >>> /* This function sets up the hid device. It does not add it >>> -- >>> 1.8.3.1 >>> ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] Bluetooth: hidp: implement hidinput_input_event callback @ 2013-07-11 22:50 ` Jiri Kosina 0 siblings, 0 replies; 15+ messages in thread From: Jiri Kosina @ 2013-07-11 22:50 UTC (permalink / raw) To: Benjamin Tissoires Cc: Benjamin Tissoires, Marcel Holtmann, Gustavo Padovan, David Herrmann, linux-bluetooth, linux-input, linux-kernel On Thu, 11 Jul 2013, Benjamin Tissoires wrote: > We can re-enable hidinput_input_event to allow the leds of bluetooth > keyboards to be set. > Now the callbacks uses hid core to retrieve the right HID report to > send, so this version is safer. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Acked-by: Jiri Kosina <jkosina@suse.cz> > --- > net/bluetooth/hidp/core.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c > index f13a8da..9c8b50d 100644 > --- a/net/bluetooth/hidp/core.c > +++ b/net/bluetooth/hidp/core.c > @@ -238,6 +238,31 @@ static int hidp_send_report(struct hidp_session *session, struct hid_report *rep > return hidp_send_intr_message(session, hdr, buf, rsize); > } > > +static int hidp_hidinput_event(struct input_dev *dev, unsigned int type, > + unsigned int code, int value) > +{ > + struct hid_device *hid = input_get_drvdata(dev); > + struct hidp_session *session = hid->driver_data; > + struct hid_field *field; > + int offset; > + > + BT_DBG("session %p type %d code %d value %d", > + session, type, code, value); > + > + if (type != EV_LED) > + return -1; > + > + offset = hidinput_find_field(hid, type, code, &field); > + if (offset == -1) { > + hid_warn(dev, "event field not found\n"); > + return -1; > + } > + > + hid_set_field(field, offset, value); > + > + return hidp_send_report(session, field->report); > +} > + > static int hidp_get_raw_report(struct hid_device *hid, > unsigned char report_number, > unsigned char *data, size_t count, > @@ -711,6 +736,7 @@ static struct hid_ll_driver hidp_hid_driver = { > .stop = hidp_stop, > .open = hidp_open, > .close = hidp_close, > + .hidinput_input_event = hidp_hidinput_event, > }; > > /* This function sets up the hid device. It does not add it > -- > 1.8.3.1 > -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 1/2] Bluetooth: hidp: implement hidinput_input_event callback @ 2013-07-11 22:50 ` Jiri Kosina 0 siblings, 0 replies; 15+ messages in thread From: Jiri Kosina @ 2013-07-11 22:50 UTC (permalink / raw) To: Benjamin Tissoires Cc: Benjamin Tissoires, Marcel Holtmann, Gustavo Padovan, David Herrmann, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On Thu, 11 Jul 2013, Benjamin Tissoires wrote: > We can re-enable hidinput_input_event to allow the leds of bluetooth > keyboards to be set. > Now the callbacks uses hid core to retrieve the right HID report to > send, so this version is safer. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> Acked-by: Jiri Kosina <jkosina-AlSwsSmVLrQ@public.gmane.org> > --- > net/bluetooth/hidp/core.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c > index f13a8da..9c8b50d 100644 > --- a/net/bluetooth/hidp/core.c > +++ b/net/bluetooth/hidp/core.c > @@ -238,6 +238,31 @@ static int hidp_send_report(struct hidp_session *session, struct hid_report *rep > return hidp_send_intr_message(session, hdr, buf, rsize); > } > > +static int hidp_hidinput_event(struct input_dev *dev, unsigned int type, > + unsigned int code, int value) > +{ > + struct hid_device *hid = input_get_drvdata(dev); > + struct hidp_session *session = hid->driver_data; > + struct hid_field *field; > + int offset; > + > + BT_DBG("session %p type %d code %d value %d", > + session, type, code, value); > + > + if (type != EV_LED) > + return -1; > + > + offset = hidinput_find_field(hid, type, code, &field); > + if (offset == -1) { > + hid_warn(dev, "event field not found\n"); > + return -1; > + } > + > + hid_set_field(field, offset, value); > + > + return hidp_send_report(session, field->report); > +} > + > static int hidp_get_raw_report(struct hid_device *hid, > unsigned char report_number, > unsigned char *data, size_t count, > @@ -711,6 +736,7 @@ static struct hid_ll_driver hidp_hid_driver = { > .stop = hidp_stop, > .open = hidp_open, > .close = hidp_close, > + .hidinput_input_event = hidp_hidinput_event, > }; > > /* This function sets up the hid device. It does not add it > -- > 1.8.3.1 > -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH 2/2] Bluetooth: hidp: remove wrong send_report at init 2013-07-11 13:41 [PATCH 0/2] Bluetooth: hidp: cleanup HID init and re-enable hidinput_input_event callback Benjamin Tissoires 2013-07-11 13:41 ` Benjamin Tissoires @ 2013-07-11 13:41 ` Benjamin Tissoires 2013-07-11 13:51 ` David Herrmann ` (2 more replies) 1 sibling, 3 replies; 15+ messages in thread From: Benjamin Tissoires @ 2013-07-11 13:41 UTC (permalink / raw) To: Benjamin Tissoires, Marcel Holtmann, Gustavo Padovan, David Herrmann, Jiri Kosina, linux-bluetooth, linux-input, linux-kernel The USB hid implementation does retrieve the reports during the start. However, this implementation does not call the HID command GET_REPORT (which would fetch the current status of each report), but use the DATA command, which is an Output Report (so transmitting data from the host to the device). The Wiimote controller is already guarded against this problem in the protocol, but it is not conformant to the specification to set all the reports to 0 on start. Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- net/bluetooth/hidp/core.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c index 9c8b50d..59d132a 100644 --- a/net/bluetooth/hidp/core.c +++ b/net/bluetooth/hidp/core.c @@ -703,20 +703,6 @@ static int hidp_parse(struct hid_device *hid) static int hidp_start(struct hid_device *hid) { - struct hidp_session *session = hid->driver_data; - struct hid_report *report; - - if (hid->quirks & HID_QUIRK_NO_INIT_REPORTS) - return 0; - - list_for_each_entry(report, &hid->report_enum[HID_INPUT_REPORT]. - report_list, list) - hidp_send_report(session, report); - - list_for_each_entry(report, &hid->report_enum[HID_FEATURE_REPORT]. - report_list, list) - hidp_send_report(session, report); - return 0; } -- 1.8.3.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] Bluetooth: hidp: remove wrong send_report at init 2013-07-11 13:41 ` [PATCH 2/2] Bluetooth: hidp: remove wrong send_report at init Benjamin Tissoires @ 2013-07-11 13:51 ` David Herrmann 2013-07-11 22:51 ` Jiri Kosina 2013-07-12 10:09 ` Gustavo Padovan 2 siblings, 0 replies; 15+ messages in thread From: David Herrmann @ 2013-07-11 13:51 UTC (permalink / raw) To: Benjamin Tissoires Cc: Benjamin Tissoires, Marcel Holtmann, Gustavo Padovan, Jiri Kosina, linux-bluetooth, open list:HID CORE LAYER, linux-kernel Hi On Thu, Jul 11, 2013 at 3:41 PM, Benjamin Tissoires <benjamin.tissoires@redhat.com> wrote: > The USB hid implementation does retrieve the reports during the start. > However, this implementation does not call the HID command GET_REPORT > (which would fetch the current status of each report), but use the > DATA command, which is an Output Report (so transmitting data from the > host to the device). > The Wiimote controller is already guarded against this problem in the > protocol, but it is not conformant to the specification to set all the > reports to 0 on start. I always wondered whether report-setup is really needed for BT-HIDP. The BT profile doesn't mention it but I thought it was part of the USBHID core specification. During hid-wiimote development I added support for HID_QUIRK_NO_INIT_REPORTS to HIDP to silence the wiimote errors. But if you say that it's specific to USBHID, I am fine with this. I never read the USBHID specs, though, I rely on your comment here. Anyway, code looks good: Reviewed-by: David Herrmann <dh.herrmann@gmail.com> Thanks! David > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > net/bluetooth/hidp/core.c | 14 -------------- > 1 file changed, 14 deletions(-) > > diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c > index 9c8b50d..59d132a 100644 > --- a/net/bluetooth/hidp/core.c > +++ b/net/bluetooth/hidp/core.c > @@ -703,20 +703,6 @@ static int hidp_parse(struct hid_device *hid) > > static int hidp_start(struct hid_device *hid) > { > - struct hidp_session *session = hid->driver_data; > - struct hid_report *report; > - > - if (hid->quirks & HID_QUIRK_NO_INIT_REPORTS) > - return 0; > - > - list_for_each_entry(report, &hid->report_enum[HID_INPUT_REPORT]. > - report_list, list) > - hidp_send_report(session, report); > - > - list_for_each_entry(report, &hid->report_enum[HID_FEATURE_REPORT]. > - report_list, list) > - hidp_send_report(session, report); > - > return 0; > } > > -- > 1.8.3.1 > ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] Bluetooth: hidp: remove wrong send_report at init 2013-07-11 13:41 ` [PATCH 2/2] Bluetooth: hidp: remove wrong send_report at init Benjamin Tissoires 2013-07-11 13:51 ` David Herrmann @ 2013-07-11 22:51 ` Jiri Kosina 2013-07-12 10:09 ` Gustavo Padovan 2 siblings, 0 replies; 15+ messages in thread From: Jiri Kosina @ 2013-07-11 22:51 UTC (permalink / raw) To: Benjamin Tissoires Cc: Benjamin Tissoires, Marcel Holtmann, Gustavo Padovan, David Herrmann, linux-bluetooth, linux-input, linux-kernel On Thu, 11 Jul 2013, Benjamin Tissoires wrote: > The USB hid implementation does retrieve the reports during the start. > However, this implementation does not call the HID command GET_REPORT > (which would fetch the current status of each report), but use the > DATA command, which is an Output Report (so transmitting data from the > host to the device). > The Wiimote controller is already guarded against this problem in the > protocol, but it is not conformant to the specification to set all the > reports to 0 on start. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> Acked-by: Jiri Kosina <jkosina@suse.cz> Gustavo, could you please take it through your tree? Thanks Benjamin, thanks David. > --- > net/bluetooth/hidp/core.c | 14 -------------- > 1 file changed, 14 deletions(-) > > diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c > index 9c8b50d..59d132a 100644 > --- a/net/bluetooth/hidp/core.c > +++ b/net/bluetooth/hidp/core.c > @@ -703,20 +703,6 @@ static int hidp_parse(struct hid_device *hid) > > static int hidp_start(struct hid_device *hid) > { > - struct hidp_session *session = hid->driver_data; > - struct hid_report *report; > - > - if (hid->quirks & HID_QUIRK_NO_INIT_REPORTS) > - return 0; > - > - list_for_each_entry(report, &hid->report_enum[HID_INPUT_REPORT]. > - report_list, list) > - hidp_send_report(session, report); > - > - list_for_each_entry(report, &hid->report_enum[HID_FEATURE_REPORT]. > - report_list, list) > - hidp_send_report(session, report); > - > return 0; > } > > -- > 1.8.3.1 > -- Jiri Kosina SUSE Labs ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] Bluetooth: hidp: remove wrong send_report at init @ 2013-07-12 10:09 ` Gustavo Padovan 0 siblings, 0 replies; 15+ messages in thread From: Gustavo Padovan @ 2013-07-12 10:09 UTC (permalink / raw) To: Benjamin Tissoires Cc: Benjamin Tissoires, Marcel Holtmann, David Herrmann, Jiri Kosina, linux-bluetooth, linux-input, linux-kernel Hi Benjamin, * Benjamin Tissoires <benjamin.tissoires@redhat.com> [2013-07-11 15:41:30 +0200]: > The USB hid implementation does retrieve the reports during the start. > However, this implementation does not call the HID command GET_REPORT > (which would fetch the current status of each report), but use the > DATA command, which is an Output Report (so transmitting data from the > host to the device). > The Wiimote controller is already guarded against this problem in the > protocol, but it is not conformant to the specification to set all the > reports to 0 on start. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > net/bluetooth/hidp/core.c | 14 -------------- > 1 file changed, 14 deletions(-) Both patches have been applied to bluetooth-next. Thanks. Gustavo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] Bluetooth: hidp: remove wrong send_report at init @ 2013-07-12 10:09 ` Gustavo Padovan 0 siblings, 0 replies; 15+ messages in thread From: Gustavo Padovan @ 2013-07-12 10:09 UTC (permalink / raw) To: Benjamin Tissoires Cc: Benjamin Tissoires, Marcel Holtmann, David Herrmann, Jiri Kosina, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA Hi Benjamin, * Benjamin Tissoires <benjamin.tissoires-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> [2013-07-11 15:41:30 +0200]: > The USB hid implementation does retrieve the reports during the start. > However, this implementation does not call the HID command GET_REPORT > (which would fetch the current status of each report), but use the > DATA command, which is an Output Report (so transmitting data from the > host to the device). > The Wiimote controller is already guarded against this problem in the > protocol, but it is not conformant to the specification to set all the > reports to 0 on start. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> > --- > net/bluetooth/hidp/core.c | 14 -------------- > 1 file changed, 14 deletions(-) Both patches have been applied to bluetooth-next. Thanks. Gustavo ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] Bluetooth: hidp: remove wrong send_report at init 2013-07-12 10:09 ` Gustavo Padovan @ 2013-07-12 12:55 ` Benjamin Tissoires -1 siblings, 0 replies; 15+ messages in thread From: Benjamin Tissoires @ 2013-07-12 12:55 UTC (permalink / raw) To: Gustavo Padovan, linux-bluetooth, linux-input, linux-kernel On 07/12/2013 12:09 PM, Gustavo Padovan wrote: > Hi Benjamin, > > * Benjamin Tissoires <benjamin.tissoires@redhat.com> [2013-07-11 15:41:30 +0200]: > >> The USB hid implementation does retrieve the reports during the start. >> However, this implementation does not call the HID command GET_REPORT >> (which would fetch the current status of each report), but use the >> DATA command, which is an Output Report (so transmitting data from the >> host to the device). >> The Wiimote controller is already guarded against this problem in the >> protocol, but it is not conformant to the specification to set all the >> reports to 0 on start. >> >> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> >> --- >> net/bluetooth/hidp/core.c | 14 -------------- >> 1 file changed, 14 deletions(-) > > Both patches have been applied to bluetooth-next. Thanks. > > Gustavo > Thanks Gustavo! Cheers, Benjamin ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 2/2] Bluetooth: hidp: remove wrong send_report at init @ 2013-07-12 12:55 ` Benjamin Tissoires 0 siblings, 0 replies; 15+ messages in thread From: Benjamin Tissoires @ 2013-07-12 12:55 UTC (permalink / raw) To: Gustavo Padovan, linux-bluetooth-u79uwXL29TY76Z2rM5mHXA, linux-input-u79uwXL29TY76Z2rM5mHXA, linux-kernel-u79uwXL29TY76Z2rM5mHXA On 07/12/2013 12:09 PM, Gustavo Padovan wrote: > Hi Benjamin, > > * Benjamin Tissoires <benjamin.tissoires-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> [2013-07-11 15:41:30 +0200]: > >> The USB hid implementation does retrieve the reports during the start. >> However, this implementation does not call the HID command GET_REPORT >> (which would fetch the current status of each report), but use the >> DATA command, which is an Output Report (so transmitting data from the >> host to the device). >> The Wiimote controller is already guarded against this problem in the >> protocol, but it is not conformant to the specification to set all the >> reports to 0 on start. >> >> Signed-off-by: Benjamin Tissoires <benjamin.tissoires-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> >> --- >> net/bluetooth/hidp/core.c | 14 -------------- >> 1 file changed, 14 deletions(-) > > Both patches have been applied to bluetooth-next. Thanks. > > Gustavo > Thanks Gustavo! Cheers, Benjamin ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2013-07-12 12:55 UTC | newest] Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2013-07-11 13:41 [PATCH 0/2] Bluetooth: hidp: cleanup HID init and re-enable hidinput_input_event callback Benjamin Tissoires 2013-07-11 13:41 ` [PATCH 1/2] Bluetooth: hidp: implement " Benjamin Tissoires 2013-07-11 13:41 ` Benjamin Tissoires 2013-07-11 14:02 ` David Herrmann 2013-07-11 14:10 ` Benjamin Tissoires 2013-07-11 16:07 ` David Herrmann 2013-07-11 22:50 ` Jiri Kosina 2013-07-11 22:50 ` Jiri Kosina 2013-07-11 13:41 ` [PATCH 2/2] Bluetooth: hidp: remove wrong send_report at init Benjamin Tissoires 2013-07-11 13:51 ` David Herrmann 2013-07-11 22:51 ` Jiri Kosina 2013-07-12 10:09 ` Gustavo Padovan 2013-07-12 10:09 ` Gustavo Padovan 2013-07-12 12:55 ` Benjamin Tissoires 2013-07-12 12:55 ` Benjamin Tissoires
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.