All of lore.kernel.org
 help / color / mirror / Atom feed
* Clarification about the hidraw documentation on HIDIOCGFEATURE
@ 2023-06-19  0:27 Xiaofan Chen
  2023-06-19  6:09 ` Xiaofan Chen
  0 siblings, 1 reply; 7+ messages in thread
From: Xiaofan Chen @ 2023-06-19  0:27 UTC (permalink / raw)
  To: linux-input; +Cc: Ihor Dutchak

Current documentation
https://docs.kernel.org/hid/hidraw.html
+++++++++++++++++++++++++++++++++++++++++++++++++++++
HIDIOCGFEATURE(len):

Get a Feature Report

This ioctl will request a feature report from the device using the
control endpoint. The first byte
of the supplied buffer should be set to the report number of the
requested report. For devices
which do not use numbered reports, set the first byte to 0. The
returned report buffer will contain
the report number in the first byte, followed by the report data read
from the device. For devices
which do not use numbered reports, the report data will begin at the
first byte of the returned buffer.
++++++++++++++++++++++++++++++++++++++++++++++++++++++

I have doubts about the last sentence. It seems to me that for
devices which do not use
numbered reports, the first byte will be 0 and the report data begins
in the second byte.

This is based on testing results using hidapi and hidapitester, which
use the ioctl.
int HID_API_EXPORT hid_get_feature_report(hid_device *dev, unsigned
char *data, size_t length)
{
    int res;

    register_device_error(dev, NULL);

    res = ioctl(dev->device_handle, HIDIOCGFEATURE(length), data);
    if (res < 0)
         register_device_error_format(dev, "ioctl (GFEATURE): %s",
strerror(errno));

    return res;
}

Reference discussion:
https://github.com/libusb/hidapi/issues/589

Test device is Jan Axelson's generic HID example which I have tested using
libusb and hidapi across platforms as well as using Windows HID API.
So I believe
the FW is good.
http://janaxelson.com/hidpage.htm#MyExampleCode (USB PIC MCU)


-- 
Xiaofan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Clarification about the hidraw documentation on HIDIOCGFEATURE
  2023-06-19  0:27 Clarification about the hidraw documentation on HIDIOCGFEATURE Xiaofan Chen
@ 2023-06-19  6:09 ` Xiaofan Chen
  2023-06-19 15:14   ` Xiaofan Chen
  0 siblings, 1 reply; 7+ messages in thread
From: Xiaofan Chen @ 2023-06-19  6:09 UTC (permalink / raw)
  To: linux-input; +Cc: Ihor Dutchak

I know that thurrent documentation has been there since it was created by
Alan Ott many years ago. And he started the HIDAPI project around that
time as well. However, I am starting to doubt whether it is correct or not
based on the testing using HIDAPI.

Please help to clarify. Thanks.

https://docs.kernel.org/hid/hidraw.html
+++++++++++++++++++++++++++++++++++++++++++++++++++++
HIDIOCGFEATURE(len):

Get a Feature Report

This ioctl will request a feature report from the device using the
control endpoint. The first byte of the supplied buffer should be
set to the report number of the requested report. For devices
which do not use numbered reports, set the first byte to 0. The
returned report buffer will contain the report number in the first
byte, followed by the report data read from the device. For devices
which do not use numbered reports, the report data will begin at the
first byte of the returned buffer.
++++++++++++++++++++++++++++++++++++++++++++++++++++++

I have doubts about the last sentence. It seems to me that for
devices which do not use numbered reports, the first byte will
be 0 and the report data begins in the second byte.

This is based on testing results using hidapi and hidapitester, which
use the ioctl.
int HID_API_EXPORT hid_get_feature_report(hid_device *dev, unsigned
char *data, size_t length)
{
    int res;

    register_device_error(dev, NULL);

    res = ioctl(dev->device_handle, HIDIOCGFEATURE(length), data);
    if (res < 0)
         register_device_error_format(dev, "ioctl (GFEATURE): %s",
strerror(errno));

    return res;
}

Reference discussion:
https://github.com/libusb/hidapi/issues/589

Test device is Jan Axelson's generic HID example which I have tested using
libusb and hidapi across platforms as well as using Windows HID API.
So I believe the FW is good.
http://janaxelson.com/hidpage.htm#MyExampleCode (USB PIC MCU)


--
Xiaofan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Clarification about the hidraw documentation on HIDIOCGFEATURE
  2023-06-19  6:09 ` Xiaofan Chen
@ 2023-06-19 15:14   ` Xiaofan Chen
  2023-06-19 23:28     ` Xiaofan Chen
  0 siblings, 1 reply; 7+ messages in thread
From: Xiaofan Chen @ 2023-06-19 15:14 UTC (permalink / raw)
  To: linux-input; +Cc: Ihor Dutchak

On Mon, Jun 19, 2023 at 2:09 PM Xiaofan Chen <xiaofanc@gmail.com> wrote:
>
> I know that thurrent documentation has been there since it was created by
> Alan Ott many years ago. And he started the HIDAPI project around that
> time as well. However, I am starting to doubt whether it is correct or not
> based on the testing using HIDAPI.
>
> Please help to clarify. Thanks.
>
> https://docs.kernel.org/hid/hidraw.html
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
> HIDIOCGFEATURE(len):
>
> Get a Feature Report
>
> This ioctl will request a feature report from the device using the
> control endpoint. The first byte of the supplied buffer should be
> set to the report number of the requested report. For devices
> which do not use numbered reports, set the first byte to 0. The
> returned report buffer will contain the report number in the first
> byte, followed by the report data read from the device. For devices
> which do not use numbered reports, the report data will begin at the
> first byte of the returned buffer.
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>
> I have doubts about the last sentence. It seems to me that for
> devices which do not use numbered reports, the first byte will
> be 0 and the report data begins in the second byte.
>
> This is based on testing results using hidapi and hidapitester, which
> use the ioctl.
> int HID_API_EXPORT hid_get_feature_report(hid_device *dev, unsigned
> char *data, size_t length)
> {
>     int res;
>
>     register_device_error(dev, NULL);
>
>     res = ioctl(dev->device_handle, HIDIOCGFEATURE(length), data);
>     if (res < 0)
>          register_device_error_format(dev, "ioctl (GFEATURE): %s",
> strerror(errno));
>
>     return res;
> }
>
> Reference discussion:
> https://github.com/libusb/hidapi/issues/589
>
> Test device is Jan Axelson's generic HID example which I have tested using
> libusb and hidapi across platforms as well as using Windows HID API.
> So I believe the FW is good.
> http://janaxelson.com/hidpage.htm#MyExampleCode (USB PIC MCU)
>

Modified testing code from the following Linux kernel
samples/hidraw/hid-example.c
https://github.com/libusb/hidapi/issues/589#issuecomment-1597356054

Results are shown here. We can clearly see that the "Fake Report ID" 0 is
in the first byte of the returned buffer, matching the output from
hidapi/hidapitester,

mcuee@UbuntuSwift3:~/build/hid/hidraw$ gcc -o myhid-example myhid-example.c

mcuee@UbuntuSwift3:~/build/hid/hidraw$ sudo ./myhid-example
Report Descriptor Size: 47
Report Descriptor:
6 a0 ff 9 1 a1 1 9 3 15 0 26 ff 0 75 8 95 3f 81 2 9 4 15 0 26 ff 0 75
8 95 3f 91 2 9 5 15 0 26 ff 0 75 8 95 3f b1 2 c0

Raw Name: Microchip Technology Inc. Generic HID
Raw Phys: usb-0000:00:14.0-1/input0
Raw Info:
bustype: 3 (USB)
vendor: 0x0925
product: 0x7001
ioctl HIDIOCSFEATURE returned: 64
ioctl HIDIOCGFEATURE returned: 64
Report data:
0 41 42 43 44 45 46 47 48 14 4 18 4 4d 72 31 50 51 52 53 54 55 56 57
58 59 5a 5b 5c 5d 5e 5f 60 61 62 63 64 65 66 67 68 69 6a 6b 6c 6d 6e
6f 70 71 72 73 74 75 76 77 78 79 7a 7b 7c 7d 7e 7f

write() wrote 64 bytes
read() read 63 bytes:
21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 30 31 32 33 34 35 36 37
38 39 3a 3b 3c 3d 3e 3f 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e
4f 50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f


-- 
Xiaofan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Clarification about the hidraw documentation on HIDIOCGFEATURE
  2023-06-19 15:14   ` Xiaofan Chen
@ 2023-06-19 23:28     ` Xiaofan Chen
  2023-06-21 11:26       ` Xiaofan Chen
  0 siblings, 1 reply; 7+ messages in thread
From: Xiaofan Chen @ 2023-06-19 23:28 UTC (permalink / raw)
  To: linux-input; +Cc: Ihor Dutchak

On Mon, Jun 19, 2023 at 11:14 PM Xiaofan Chen <xiaofanc@gmail.com> wrote:
>
> On Mon, Jun 19, 2023 at 2:09 PM Xiaofan Chen <xiaofanc@gmail.com> wrote:
> >
> > I know that thurrent documentation has been there since it was created by
> > Alan Ott many years ago. And he started the HIDAPI project around that
> > time as well. However, I am starting to doubt whether it is correct or not
> > based on the testing using HIDAPI.
> >
> > Please help to clarify. Thanks.
> >
> > https://docs.kernel.org/hid/hidraw.html
> > +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > HIDIOCGFEATURE(len):
> >
> > Get a Feature Report
> >
> > This ioctl will request a feature report from the device using the
> > control endpoint. The first byte of the supplied buffer should be
> > set to the report number of the requested report. For devices
> > which do not use numbered reports, set the first byte to 0. The
> > returned report buffer will contain the report number in the first
> > byte, followed by the report data read from the device. For devices
> > which do not use numbered reports, the report data will begin at the
> > first byte of the returned buffer.
> > ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >
> > I have doubts about the last sentence. It seems to me that for
> > devices which do not use numbered reports, the first byte will
> > be 0 and the report data begins in the second byte.
> >
> > This is based on testing results using hidapi and hidapitester, which
> > use the ioctl.
> > int HID_API_EXPORT hid_get_feature_report(hid_device *dev, unsigned
> > char *data, size_t length)
> > {
> >     int res;
> >
> >     register_device_error(dev, NULL);
> >
> >     res = ioctl(dev->device_handle, HIDIOCGFEATURE(length), data);
> >     if (res < 0)
> >          register_device_error_format(dev, "ioctl (GFEATURE): %s",
> > strerror(errno));
> >
> >     return res;
> > }
> >
> > Reference discussion:
> > https://github.com/libusb/hidapi/issues/589
> >
> > Test device is Jan Axelson's generic HID example which I have tested using
> > libusb and hidapi across platforms as well as using Windows HID API.
> > So I believe the FW is good.
> > http://janaxelson.com/hidpage.htm#MyExampleCode (USB PIC MCU)
> >
>
> Modified testing code from the following Linux kernel
> samples/hidraw/hid-example.c
> https://github.com/libusb/hidapi/issues/589#issuecomment-1597356054
>
> Results are shown here. We can clearly see that the "Fake Report ID" 0 is
> in the first byte of the returned buffer, matching the output from
> hidapi/hidapitester,
>
> mcuee@UbuntuSwift3:~/build/hid/hidraw$ gcc -o myhid-example myhid-example.c
>
> mcuee@UbuntuSwift3:~/build/hid/hidraw$ sudo ./myhid-example
> Report Descriptor Size: 47
> Report Descriptor:
> 6 a0 ff 9 1 a1 1 9 3 15 0 26 ff 0 75 8 95 3f 81 2 9 4 15 0 26 ff 0 75
> 8 95 3f 91 2 9 5 15 0 26 ff 0 75 8 95 3f b1 2 c0
>
> Raw Name: Microchip Technology Inc. Generic HID
> Raw Phys: usb-0000:00:14.0-1/input0
> Raw Info:
> bustype: 3 (USB)
> vendor: 0x0925
> product: 0x7001
> ioctl HIDIOCSFEATURE returned: 64
> ioctl HIDIOCGFEATURE returned: 64
> Report data:
> 0 41 42 43 44 45 46 47 48 14 4 18 4 4d 72 31 50 51 52 53 54 55 56 57
> 58 59 5a 5b 5c 5d 5e 5f 60 61 62 63 64 65 66 67 68 69 6a 6b 6c 6d 6e
> 6f 70 71 72 73 74 75 76 77 78 79 7a 7b 7c 7d 7e 7f
>
> write() wrote 64 bytes
> read() read 63 bytes:
> 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 30 31 32 33 34 35 36 37
> 38 39 3a 3b 3c 3d 3e 3f 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e
> 4f 50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f
>

Moreover the kernel codes here seem to prove that the documentation
is wrong for both HIDIOCGFEATURE and HIDIOCGINPUT.

https://github.com/torvalds/linux/blob/master/include/uapi/linux/hidraw.h

/* The first byte of SFEATURE and GFEATURE is the report number */
#define HIDIOCSFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
#define HIDIOCGFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
#define HIDIOCGRAWUNIQ(len)     _IOC(_IOC_READ, 'H', 0x08, len)
/* The first byte of SINPUT and GINPUT is the report number */
#define HIDIOCSINPUT(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x09, len)
#define HIDIOCGINPUT(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x0A, len)
/* The first byte of SOUTPUT and GOUTPUT is the report number */
#define HIDIOCSOUTPUT(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x0B, len)
#define HIDIOCGOUTPUT(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x0C, len)

-- 
Xiaofan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Clarification about the hidraw documentation on HIDIOCGFEATURE
  2023-06-19 23:28     ` Xiaofan Chen
@ 2023-06-21 11:26       ` Xiaofan Chen
  2023-06-21 15:05         ` Benjamin Tissoires
  0 siblings, 1 reply; 7+ messages in thread
From: Xiaofan Chen @ 2023-06-21 11:26 UTC (permalink / raw)
  To: Jiri Kosina, Benjamin Tissoires; +Cc: Ihor Dutchak, linux-input

On Tue, Jun 20, 2023 at 7:28 AM Xiaofan Chen <xiaofanc@gmail.com> wrote:
>
> On Mon, Jun 19, 2023 at 11:14 PM Xiaofan Chen <xiaofanc@gmail.com> wrote:
> >
> > On Mon, Jun 19, 2023 at 2:09 PM Xiaofan Chen <xiaofanc@gmail.com> wrote:
> > >
> > > I know that thurrent documentation has been there since it was created by
> > > Alan Ott many years ago. And he started the HIDAPI project around that
> > > time as well. However, I am starting to doubt whether it is correct or not
> > > based on the testing using HIDAPI.
> > >
> > > Please help to clarify. Thanks.
> > >
> > > https://docs.kernel.org/hid/hidraw.html
> > > +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > HIDIOCGFEATURE(len):
> > >
> > > Get a Feature Report
> > >
> > > This ioctl will request a feature report from the device using the
> > > control endpoint. The first byte of the supplied buffer should be
> > > set to the report number of the requested report. For devices
> > > which do not use numbered reports, set the first byte to 0. The
> > > returned report buffer will contain the report number in the first
> > > byte, followed by the report data read from the device. For devices
> > > which do not use numbered reports, the report data will begin at the
> > > first byte of the returned buffer.
> > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >
> > > I have doubts about the last sentence. It seems to me that for
> > > devices which do not use numbered reports, the first byte will
> > > be 0 and the report data begins in the second byte.
> > >
> > > This is based on testing results using hidapi and hidapitester, which
> > > use the ioctl.
> > > int HID_API_EXPORT hid_get_feature_report(hid_device *dev, unsigned
> > > char *data, size_t length)
> > > {
> > >     int res;
> > >
> > >     register_device_error(dev, NULL);
> > >
> > >     res = ioctl(dev->device_handle, HIDIOCGFEATURE(length), data);
> > >     if (res < 0)
> > >          register_device_error_format(dev, "ioctl (GFEATURE): %s",
> > > strerror(errno));
> > >
> > >     return res;
> > > }
> > >
> > > Reference discussion:
> > > https://github.com/libusb/hidapi/issues/589
> > >
> > > Test device is Jan Axelson's generic HID example which I have tested using
> > > libusb and hidapi across platforms as well as using Windows HID API.
> > > So I believe the FW is good.
> > > http://janaxelson.com/hidpage.htm#MyExampleCode (USB PIC MCU)
> > >
> >
> > Modified testing code from the following Linux kernel
> > samples/hidraw/hid-example.c
> > https://github.com/libusb/hidapi/issues/589#issuecomment-1597356054
> >
> > Results are shown here. We can clearly see that the "Fake Report ID" 0 is
> > in the first byte of the returned buffer, matching the output from
> > hidapi/hidapitester,
> >
> > mcuee@UbuntuSwift3:~/build/hid/hidraw$ gcc -o myhid-example myhid-example.c
> >
> > mcuee@UbuntuSwift3:~/build/hid/hidraw$ sudo ./myhid-example
> > Report Descriptor Size: 47
> > Report Descriptor:
> > 6 a0 ff 9 1 a1 1 9 3 15 0 26 ff 0 75 8 95 3f 81 2 9 4 15 0 26 ff 0 75
> > 8 95 3f 91 2 9 5 15 0 26 ff 0 75 8 95 3f b1 2 c0
> >
> > Raw Name: Microchip Technology Inc. Generic HID
> > Raw Phys: usb-0000:00:14.0-1/input0
> > Raw Info:
> > bustype: 3 (USB)
> > vendor: 0x0925
> > product: 0x7001
> > ioctl HIDIOCSFEATURE returned: 64
> > ioctl HIDIOCGFEATURE returned: 64
> > Report data:
> > 0 41 42 43 44 45 46 47 48 14 4 18 4 4d 72 31 50 51 52 53 54 55 56 57
> > 58 59 5a 5b 5c 5d 5e 5f 60 61 62 63 64 65 66 67 68 69 6a 6b 6c 6d 6e
> > 6f 70 71 72 73 74 75 76 77 78 79 7a 7b 7c 7d 7e 7f
> >
> > write() wrote 64 bytes
> > read() read 63 bytes:
> > 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 30 31 32 33 34 35 36 37
> > 38 39 3a 3b 3c 3d 3e 3f 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e
> > 4f 50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f
> >
>
> Moreover the kernel codes here seem to prove that the documentation
> is wrong for both HIDIOCGFEATURE and HIDIOCGINPUT.
>
> https://github.com/torvalds/linux/blob/master/include/uapi/linux/hidraw.h
>
> /* The first byte of SFEATURE and GFEATURE is the report number */
> #define HIDIOCSFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
> #define HIDIOCGFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
> #define HIDIOCGRAWUNIQ(len)     _IOC(_IOC_READ, 'H', 0x08, len)
> /* The first byte of SINPUT and GINPUT is the report number */
> #define HIDIOCSINPUT(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x09, len)
> #define HIDIOCGINPUT(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x0A, len)
> /* The first byte of SOUTPUT and GOUTPUT is the report number */
> #define HIDIOCSOUTPUT(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x0B, len)
> #define HIDIOCGOUTPUT(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x0C, len)

Hi Jiri and Benjamin,

Sorry to ping the two maintainers, hopefully you two can give the
answer. Thanks.


-- 
Xiaofan

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Clarification about the hidraw documentation on HIDIOCGFEATURE
  2023-06-21 11:26       ` Xiaofan Chen
@ 2023-06-21 15:05         ` Benjamin Tissoires
  2023-06-22  9:08           ` Xiaofan Chen
  0 siblings, 1 reply; 7+ messages in thread
From: Benjamin Tissoires @ 2023-06-21 15:05 UTC (permalink / raw)
  To: Xiaofan Chen; +Cc: Jiri Kosina, Ihor Dutchak, linux-input



On Wed, Jun 21, 2023 at 1:27 PM Xiaofan Chen <xiaofanc@gmail.com> wrote:
>
> On Tue, Jun 20, 2023 at 7:28 AM Xiaofan Chen <xiaofanc@gmail.com> wrote:
> >
> > On Mon, Jun 19, 2023 at 11:14 PM Xiaofan Chen <xiaofanc@gmail.com> wrote:
> > >
> > > On Mon, Jun 19, 2023 at 2:09 PM Xiaofan Chen <xiaofanc@gmail.com> wrote:
> > > >
> > > > I know that thurrent documentation has been there since it was created by
> > > > Alan Ott many years ago. And he started the HIDAPI project around that
> > > > time as well. However, I am starting to doubt whether it is correct or not
> > > > based on the testing using HIDAPI.
> > > >
> > > > Please help to clarify. Thanks.
> > > >
> > > > https://docs.kernel.org/hid/hidraw.html
> > > > +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > HIDIOCGFEATURE(len):
> > > >
> > > > Get a Feature Report
> > > >
> > > > This ioctl will request a feature report from the device using the
> > > > control endpoint. The first byte of the supplied buffer should be
> > > > set to the report number of the requested report. For devices
> > > > which do not use numbered reports, set the first byte to 0. The
> > > > returned report buffer will contain the report number in the first
> > > > byte, followed by the report data read from the device. For devices
> > > > which do not use numbered reports, the report data will begin at the
> > > > first byte of the returned buffer.


Yep, this is wrong.

This should be read:
```
The returned report buffer will contain the report number in the first 
byte or 0 when the device doesn't use numbered reports. The data read 
from the device will be stored in the following bytes.
```

FWIW, the difficulty to find out what the code does is because this part 
is handled in each HID transport driver: USB, Bluetooth, I2C.

Looking at drivers/hid/usbhid/hid-core.c, lines 869+, the function 
usbhid_get_raw_report() is the one used by hidraw in the end and is 
still the original code from Alan:

```
/* Byte 0 is the report number. Report data starts at byte 1.*/
buf[0] = report_number;
if (report_number == 0x0) {
	/* Offset the return buffer by 1, so that the report ID
	   will remain in byte 0. */
	buf++;
	count--;
	skipped_report_id = 1;
}
```
  >
> > > > ++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > >
> > > > I have doubts about the last sentence. It seems to me that for
> > > > devices which do not use numbered reports, the first byte will
> > > > be 0 and the report data begins in the second byte.
> > > >
> > > > This is based on testing results using hidapi and hidapitester, which
> > > > use the ioctl.
> > > > int HID_API_EXPORT hid_get_feature_report(hid_device *dev, unsigned
> > > > char *data, size_t length)
> > > > {
> > > >     int res;
> > > >
> > > >     register_device_error(dev, NULL);
> > > >
> > > >     res = ioctl(dev->device_handle, HIDIOCGFEATURE(length), data);
> > > >     if (res < 0)
> > > >          register_device_error_format(dev, "ioctl (GFEATURE): %s",
> > > > strerror(errno));
> > > >
> > > >     return res;
> > > > }
> > > >
> > > > Reference discussion:
> > > > https://github.com/libusb/hidapi/issues/589
> > > >
> > > > Test device is Jan Axelson's generic HID example which I have tested using
> > > > libusb and hidapi across platforms as well as using Windows HID API.
> > > > So I believe the FW is good.
> > > > http://janaxelson.com/hidpage.htm#MyExampleCode (USB PIC MCU)
> > > >
> > >
> > > Modified testing code from the following Linux kernel
> > > samples/hidraw/hid-example.c
> > > https://github.com/libusb/hidapi/issues/589#issuecomment-1597356054
> > >
> > > Results are shown here. We can clearly see that the "Fake Report ID" 0 is
> > > in the first byte of the returned buffer, matching the output from
> > > hidapi/hidapitester,
> > >
> > > mcuee@UbuntuSwift3:~/build/hid/hidraw$ gcc -o myhid-example myhid-example.c
> > >
> > > mcuee@UbuntuSwift3:~/build/hid/hidraw$ sudo ./myhid-example
> > > Report Descriptor Size: 47
> > > Report Descriptor:
> > > 6 a0 ff 9 1 a1 1 9 3 15 0 26 ff 0 75 8 95 3f 81 2 9 4 15 0 26 ff 0 75
> > > 8 95 3f 91 2 9 5 15 0 26 ff 0 75 8 95 3f b1 2 c0
> > >
> > > Raw Name: Microchip Technology Inc. Generic HID
> > > Raw Phys: usb-0000:00:14.0-1/input0
> > > Raw Info:
> > > bustype: 3 (USB)
> > > vendor: 0x0925
> > > product: 0x7001
> > > ioctl HIDIOCSFEATURE returned: 64
> > > ioctl HIDIOCGFEATURE returned: 64
> > > Report data:
> > > 0 41 42 43 44 45 46 47 48 14 4 18 4 4d 72 31 50 51 52 53 54 55 56 57
> > > 58 59 5a 5b 5c 5d 5e 5f 60 61 62 63 64 65 66 67 68 69 6a 6b 6c 6d 6e
> > > 6f 70 71 72 73 74 75 76 77 78 79 7a 7b 7c 7d 7e 7f
> > >
> > > write() wrote 64 bytes
> > > read() read 63 bytes:
> > > 21 22 23 24 25 26 27 28 29 2a 2b 2c 2d 2e 2f 30 31 32 33 34 35 36 37
> > > 38 39 3a 3b 3c 3d 3e 3f 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e
> > > 4f 50 51 52 53 54 55 56 57 58 59 5a 5b 5c 5d 5e 5f
> > >
> >
> > Moreover the kernel codes here seem to prove that the documentation
> > is wrong for both HIDIOCGFEATURE and HIDIOCGINPUT.
> >
> > https://github.com/torvalds/linux/blob/master/include/uapi/linux/hidraw.h
> >
> > /* The first byte of SFEATURE and GFEATURE is the report number */
> > #define HIDIOCSFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x06, len)
> > #define HIDIOCGFEATURE(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x07, len)
> > #define HIDIOCGRAWUNIQ(len)     _IOC(_IOC_READ, 'H', 0x08, len)
> > /* The first byte of SINPUT and GINPUT is the report number */
> > #define HIDIOCSINPUT(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x09, len)
> > #define HIDIOCGINPUT(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x0A, len)
> > /* The first byte of SOUTPUT and GOUTPUT is the report number */
> > #define HIDIOCSOUTPUT(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x0B, len)
> > #define HIDIOCGOUTPUT(len)    _IOC(_IOC_WRITE|_IOC_READ, 'H', 0x0C, len)
>
> Hi Jiri and Benjamin,
>
> Sorry to ping the two maintainers, hopefully you two can give the
> answer. Thanks.

See above, I also think the documentation is wrong.

Cheers,
Benjamin

>
>
> --
> Xiaofan
>


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: Clarification about the hidraw documentation on HIDIOCGFEATURE
  2023-06-21 15:05         ` Benjamin Tissoires
@ 2023-06-22  9:08           ` Xiaofan Chen
  0 siblings, 0 replies; 7+ messages in thread
From: Xiaofan Chen @ 2023-06-22  9:08 UTC (permalink / raw)
  To: Benjamin Tissoires; +Cc: Jiri Kosina, Ihor Dutchak, linux-input

On Wed, Jun 21, 2023 at 11:05 PM Benjamin Tissoires
<benjamin.tissoires@redhat.com> wrote:
>
> On Wed, Jun 21, 2023 at 1:27 PM Xiaofan Chen <xiaofanc@gmail.com> wrote:
> >
> > On Tue, Jun 20, 2023 at 7:28 AM Xiaofan Chen <xiaofanc@gmail.com> wrote:
> > >
> > > On Mon, Jun 19, 2023 at 11:14 PM Xiaofan Chen <xiaofanc@gmail.com> wrote:
> > > >
> > > > On Mon, Jun 19, 2023 at 2:09 PM Xiaofan Chen <xiaofanc@gmail.com> wrote:
> > > > >
> > > > > I know that thurrent documentation has been there since it was created by
> > > > > Alan Ott many years ago. And he started the HIDAPI project around that
> > > > > time as well. However, I am starting to doubt whether it is correct or not
> > > > > based on the testing using HIDAPI.
> > > > >
> > > > > Please help to clarify. Thanks.
> > > > >
> > > > > https://docs.kernel.org/hid/hidraw.html
> > > > > +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > > > HIDIOCGFEATURE(len):
> > > > >
> > > > > Get a Feature Report
> > > > >
> > > > > This ioctl will request a feature report from the device using the
> > > > > control endpoint. The first byte of the supplied buffer should be
> > > > > set to the report number of the requested report. For devices
> > > > > which do not use numbered reports, set the first byte to 0. The
> > > > > returned report buffer will contain the report number in the first
> > > > > byte, followed by the report data read from the device. For devices
> > > > > which do not use numbered reports, the report data will begin at the
> > > > > first byte of the returned buffer.
>
>
> Yep, this is wrong.
>
> This should be read:
> ```
> The returned report buffer will contain the report number in the first
> byte or 0 when the device doesn't use numbered reports. The data read
> from the device will be stored in the following bytes.
> ```
>
> FWIW, the difficulty to find out what the code does is because this part
> is handled in each HID transport driver: USB, Bluetooth, I2C.
>
> Looking at drivers/hid/usbhid/hid-core.c, lines 869+, the function
> usbhid_get_raw_report() is the one used by hidraw in the end and is
> still the original code from Alan:
>
> ```
> /* Byte 0 is the report number. Report data starts at byte 1.*/
> buf[0] = report_number;
> if (report_number == 0x0) {
>         /* Offset the return buffer by 1, so that the report ID
>            will remain in byte 0. */
>         buf++;
>         count--;
>         skipped_report_id = 1;
> }
> ```
>
> > Hi Jiri and Benjamin,
> >
> > Sorry to ping the two maintainers, hopefully you two can give the
> > answer. Thanks.
>
> See above, I also think the documentation is wrong.
>

Thanks a lot for the confirmation.

On the same note, I think it is the same for HIDIOCGINPUT as well, but there
is no need to update the documentation once the documentation is fixed
for HIDIOCGFEATURE.

++++++++++++++
HIDIOCGINPUT(len):

Get an Input Report

This ioctl will request an input report from the device using the
control endpoint. This
is slower on most devices where a dedicated In endpoint exists for
regular input reports,
but allows the host to request the value of a specific report number.
Typically, this is
used to request the initial states of an input report of a device,
before an application
listens for normal reports via the regular device read() interface.
The format of the
buffer issued with this report is identical to that of HIDIOCGFEATURE.
++++++++++++++


-- 
Xiaofan

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2023-06-22  9:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-19  0:27 Clarification about the hidraw documentation on HIDIOCGFEATURE Xiaofan Chen
2023-06-19  6:09 ` Xiaofan Chen
2023-06-19 15:14   ` Xiaofan Chen
2023-06-19 23:28     ` Xiaofan Chen
2023-06-21 11:26       ` Xiaofan Chen
2023-06-21 15:05         ` Benjamin Tissoires
2023-06-22  9:08           ` Xiaofan Chen

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.