On Thu, 3 Apr 2003 16:09:42 +0200
[EMAIL PROTECTED] wrote:

>  void hid_write_report(struct hid_device *hid, struct hid_report *report)
>  {
>      if (hid->claimed & HID_CLAIMED_HIDDEV) {

This should probably check (report->id != 0) instead - it does not
matter where the output request came from (hiddev or whatever), the
important thing is that we have report IDs.

>           hid->out[hid->outhead].buffer[0] = report->id; //XXX
>           hid_output_report(report, hid->out[hid->outhead].buffer + 1);
> 
>           hid->out[hid->outhead].dr.wValue = cpu_to_le16(((report->type + 1) <<
> 8) | report->id);

This line should be in the common code (outside the 'if'); see below.

>           hid->out[hid->outhead].dr.wLength = cpu_to_le16((report->size + 7) >>
> 3) + 1; //XXX

There is a bug here (endianness).

>      } else {
>           hid_output_report(report, hid->out[hid->outhead].buffer);
> 
>           hid->out[hid->outhead].dr.wValue = cpu_to_le16(0x200 | report->id);

This line is wrong - it assumes that report->type is always
HID_OUTPUT_REPORT, which is not always the case.

HID spec says that even HID_INPUT_REPORT may be valid here in some cases
(7.2.2 Set_Report Request):

    - A device might choose to ignore input Set_Report requests as
      meaningless.  Alternatively these reports could be used to reset
      the origin of a control (that is, current position should report
      zero).  The effect of sent reports will also depend on whether the
      recipient controls are absolute or relative.


>           hid->out[hid->outhead].dr.wLength = cpu_to_le16((report->size + 7) >>
> 3);
>      }
>      hid->outhead = (hid->outhead + 1) & (HID_CONTROL_FIFO_SIZE - 1);


So we get something like this:

void hid_write_report(struct hid_device *hid, struct hid_report *report)
{
        if (report->id) {
                hid->out[hid->outhead].buffer[0] = report->id;
                hid_output_report(report, hid->out[hid->outhead].buffer + 1);
                hid->out[hid->outhead].dr.wLength = cpu_to_le16(((report->size + 7) >> 
3) + 1);
        } else {
                hid_output_report(report, hid->out[hid->outhead].buffer);
                hid->out[hid->outhead].dr.wLength = cpu_to_le16((report->size + 7) >> 
3);
        }

        hid->out[hid->outhead].dr.wValue = cpu_to_le16(((report->type + 1) << 8) | 
report->id);

        hid->outhead = (hid->outhead + 1) & (HID_CONTROL_FIFO_SIZE - 1);

        if (hid->outhead == hid->outtail)
                hid->outtail = (hid->outtail + 1) & (HID_CONTROL_FIFO_SIZE - 1);

        if (hid->urbout.status != -EINPROGRESS)
                hid_submit_out(hid);
}

(I have also attached the same changes in patch form; please test.)

Attachment: linux-2.4.20-alt-hidups.patch
Description: Binary data

Reply via email to