| Message ID | 1393633237-26496-3-git-send-email-benjamin.tissoires@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
On Fri, 28 Feb 2014, Benjamin Tissoires wrote: > I don't have access to the device, so I copied/pasted the code > from hidraw. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > --- > drivers/hid/hid-cp2112.c | 16 +++++++++++++++- > 1 file changed, 15 insertions(+), 1 deletion(-) > > diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c > index 860db694..c4f87bd 100644 > --- a/drivers/hid/hid-cp2112.c > +++ b/drivers/hid/hid-cp2112.c > @@ -290,7 +290,21 @@ static int cp2112_hid_output(struct hid_device *hdev, u8 *data, size_t count, > if (!buf) > return -ENOMEM; > > - ret = hdev->hid_output_raw_report(hdev, buf, count, report_type); > + /* Fixme: test which function is actually called for output reports */ I don't completely understand this Fixme (oh, and please spell it as 'FIXME:' so that we are consistent with all the other instances), could you please elaborate? Thanks,
On Mar 04 2014 or thereabouts, Jiri Kosina wrote: > On Fri, 28 Feb 2014, Benjamin Tissoires wrote: > > > I don't have access to the device, so I copied/pasted the code > > from hidraw. > > > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> > > --- > > drivers/hid/hid-cp2112.c | 16 +++++++++++++++- > > 1 file changed, 15 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c > > index 860db694..c4f87bd 100644 > > --- a/drivers/hid/hid-cp2112.c > > +++ b/drivers/hid/hid-cp2112.c > > @@ -290,7 +290,21 @@ static int cp2112_hid_output(struct hid_device *hdev, u8 *data, size_t count, > > if (!buf) > > return -ENOMEM; > > > > - ret = hdev->hid_output_raw_report(hdev, buf, count, report_type); > > + /* Fixme: test which function is actually called for output reports */ > > I don't completely understand this Fixme (oh, and please spell it as > 'FIXME:' so that we are consistent with all the other instances), could > you please elaborate? Well, sorry: As I said, this part is a copy/paste of what is in hidraw. However, this just reflect that we don't know how the device actually behave, which is not very elegant. I have currently no clues of which function will be actually called for output reports: hid_hw_output_report() or hid_hw_raw_request(). Once we got the confirmation of which function is called, we could make the path more straightforward. I bought one of these (it may help debugging some Synaptics devices), and I'll receive it by the end of the week. So by next week, we should get the actual code path and remove this FIXME. I need to send a v2 of hid-sony in any cases, so I guess you should not pull these 4 patches right away. If you prefer having this in linux-next, the sooner, I can also send the v2 right away, and we will fix this cp2112 driver next week. Cheers, Benjamin > > Thanks, > > -- > Jiri Kosina > SUSE Labs -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 4 Mar 2014, Benjamin Tissoires wrote: > I need to send a v2 of hid-sony in any cases, so I guess you should not > pull these 4 patches right away. If you prefer having this in linux-next, > the sooner, I can also send the v2 right away, and we will fix this > cp2112 driver next week. That's fine, I'll wait for the whole v2 respin. Thanks,
On Wed, Mar 5, 2014 at 1:11 PM, David Barksdale <dbarksdale@uplogix.com> wrote: > Sorry for the delay, hid_hw_output_report() is the correct function. No worries and thanks for the tests. This saves me some time! Sending the v2 right away. Cheers, Benjamin > > > On March 4, 2014 8:18:38 AM CST, Benjamin Tissoires > <benjamin.tissoires@redhat.com> wrote: >> >> On Mar 04 2014 or thereabouts, Jiri Kosina wrote: >>> >>> On Fri, 28 Feb 2014, Benjamin Tissoires wrote: >>> >>>> >>>> I don't have access to the device, so I copied/pasted the code >>>> from hidraw. >>>> >>>> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> >>>> --- >>>> drivers/hid/hid-cp2112.c | 16 +++++++++++++++- >>>> 1 file changed, 15 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c >>>> index 860db694..c4f87bd 100644 >>>> --- a/drivers/hid/hid-cp2112.c >>>> +++ b/drivers/hid/hid-cp2112.c >>>> @@ -290,7 +290,21 @@ static int cp2112_hid_output(struct hid_device >>>> *hdev, u8 *data, size_t count, >>>> if (!buf) >>>> return -ENOMEM; >>>> >>>> - ret = hdev->hid_output_raw_report(hdev, buf, count, report_type); >>>> + /* Fixme: test which function is actually called for output reports >>>> */ >>> >>> >>> I don't completely understand this Fixme (oh, and please spell it as >>> 'FIXME:' so that we are consistent with all the other instances), could >>> you please elaborate? >> >> >> Well, sorry: >> As I said, this part is a copy/paste of what is in hidraw. However, this >> just reflect that we don't know how the device actually behave, which is >> not very elegant. I have currently no clues of which function will be >> actually called for output reports: hid_hw_output_report() or >> hid_hw_raw_request(). Once we got the confirmation of which function is >> called, we could make the path more straightforward. >> >> I bought one of these (it may help debugging some Synaptics devices), >> and I'll receive it by the end of the week. So by next week, we should >> get the actual code path and remove this FIXME. >> >> I need to send a v2 of hid-sony in any cases, so I guess you should not >> pull these 4 patches right away. If you prefer having this in linux-next, >> the sooner, I can also send the v2 right away, and we will fix this >> cp2112 driver next week. >> >> Cheers, >> Benjamin >> >>> >>> Thanks, >>> >>> -- >>> Jiri Kosina >>> SUSE Labs > > > -- > Sent from my Android device with K-9 Mail. Please excuse my brevity. -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/hid/hid-cp2112.c b/drivers/hid/hid-cp2112.c index 860db694..c4f87bd 100644 --- a/drivers/hid/hid-cp2112.c +++ b/drivers/hid/hid-cp2112.c @@ -290,7 +290,21 @@ static int cp2112_hid_output(struct hid_device *hdev, u8 *data, size_t count, if (!buf) return -ENOMEM; - ret = hdev->hid_output_raw_report(hdev, buf, count, report_type); + /* Fixme: test which function is actually called for output reports */ + if (report_type == HID_OUTPUT_REPORT) { + ret = hid_hw_output_report(hdev, buf, count); + /* + * compatibility with old implementation of USB-HID: + * if the device does not support receiving output reports, + * on an interrupt endpoint, fallback to SET_REPORT HID command. + */ + if (ret != -ENOSYS) + goto out_free; + } + + ret = hid_hw_raw_request(hdev, buf[0], buf, count, report_type, + HID_REQ_SET_REPORT); +out_free: kfree(buf); return ret; }
I don't have access to the device, so I copied/pasted the code from hidraw. Signed-off-by: Benjamin Tissoires <benjamin.tissoires@redhat.com> --- drivers/hid/hid-cp2112.c | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-)