Re: [RESEND PATCH 1/2] HID: Add driver for RC Simulator Controllers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 9/15/22 09:35, Benjamin Tissoires wrote:
On Thu, Sep 15, 2022 at 8:28 AM Marcus Folkesson
<marcus.folkesson@xxxxxxxxx> wrote:

Hi Benjamin,

On Tue, Aug 30, 2022 at 02:45:11PM +0200, Benjamin Tissoires wrote:
On Thu, Aug 25, 2022 at 8:44 AM Marcus Folkesson
<marcus.folkesson@xxxxxxxxx> wrote:



[...]


Is the fact that more than one button share the same
byte hard to describe in the report?

No, this is actually easy to describe. You say that there is one usage
of "something" which has a report size of 1 bit, and then you have
another usage of "something else" with the same report size.

But usually you have to add padding after to make up to 8 bits (so 6
bits in that case).

I was referring to the case  where you are parsing the same bit on the
wire, and give a different usage based if you have received an odd or
an even number of reports. In that case, we probably need to use move
this bit to a const field in the original report descriptor and say
that the data is now not const:

- initial report (completely random example):
   X (2 bytes) | Y (2 bytes) | button this_or_that (1 bit, depending of
odd or even received reports) | 7 bits of padding
- we can declare it as:
   X (2 bytes) | Y (2 bytes) | button this (1 bit) | button that (1
bit) | 6 bits of padding

How about if there is no unused bytes?

The XTRG2FMS has 8 10-bit channels and use every byte in the report.
Should I specify 8 8-bit channels instead and fix that in raw_event?
If so, should I only use 8bit values then?

If I am not wrong, you should be able to add another byte in the
report descriptor, as long as your raw_event function always adds it.
Though now that I am typing it, I am actually wondering if this will
work. You can always try, there is a chance it'll work, but I can't
remember if it'll result in a timeout on the USB front because it'll
expect one more byte that will never arrive.

I am back home, and I just tested that. I had a doubt, and it is indeed
failing. You need the following change for this to be working (I need to
send it as a proper patch after assessing it hasn't side effects)

---

diff --git a/drivers/hid/usbhid/hid-core.c b/drivers/hid/usbhid/hid-core.c
index 13cce286247e..f37ffe2bd488 100644
--- a/drivers/hid/usbhid/hid-core.c
+++ b/drivers/hid/usbhid/hid-core.c
@@ -275,6 +275,7 @@ static void hid_irq_in(struct urb *urb)
        int                     status;
switch (urb->status) {
+       case -EOVERFLOW:        /* happens with modified report descriptors */
        case 0:                 /* success */
                usbhid->retry_delay = 0;
                if (!test_bit(HID_OPENED, &usbhid->iofl))
---

Cheers,
Benjamin



(Are you at the ELCE conference btw?)

I was at Plumbers this week, but got an extra day today. But yeah, I'm
in Dublin today.

Cheers,
Benjamin


Best regards
Marcus Folkesson




[Index of Archives]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Video 4 Linux]     [Device Mapper]     [Linux Resources]

  Powered by Linux