| Message ID | 20240502042335.15611-1-mmyangfl@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | HID: kye: Change Device Usage from Puck to Mouse | expand |
On Thu, 2 May 2024, David Yang wrote: > Change device type because > a. it is exactly a mouse, with left/right buttons and scroll wheel; > b. it does not have visible marks or crosshairs, thus does not provide > higher accuracy than stylus. Let's CC Milan, who originally added all this in feb6faf1e5d46 ("HID: kye: Fix report descriptor for Genius PenSketch M912") ... Milan, any concerns about the below? Thanks. > > Signed-off-by: David Yang <mmyangfl@gmail.com> > --- > drivers/hid/hid-kye.c | 75 +++++++++++++++++++++++++------------------ > 1 file changed, 44 insertions(+), 31 deletions(-) > > diff --git a/drivers/hid/hid-kye.c b/drivers/hid/hid-kye.c > index eb9bf2829937..70ceb9437332 100644 > --- a/drivers/hid/hid-kye.c > +++ b/drivers/hid/hid-kye.c > @@ -209,7 +209,7 @@ static const __u8 pensketch_t609a_control_rdesc[] = { > 0xC0 /* End Collection */ > }; > > -/* Fix indexes in kye_tablet_fixup if you change this */ > +/* Fix indexes in kye_tablet_fixup() if you change this */ > static const __u8 kye_tablet_rdesc[] = { > 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */ > 0x09, 0x01, /* Usage (01h), */ > @@ -262,12 +262,16 @@ static const __u8 kye_tablet_rdesc[] = { > 0x27, 0xFF, 0x07, 0x00, 0x00, /* Logical Maximum (2047), */ > 0x81, 0x02, /* Input (Variable), */ > 0xC0, /* End Collection, */ > - 0xC0, /* End Collection, */ > - 0x05, 0x0D, /* Usage Page (Digitizer), */ > - 0x09, 0x21, /* Usage (Puck), */ > + 0xC0 /* End Collection, */ > +}; > + > +/* Fix indexes in kye_tablet_fixup() if you change this */ > +static const __u8 kye_tablet_mouse_rdesc[] = { > + 0x05, 0x01, /* Usage Page (Desktop), */ > + 0x09, 0x02, /* Usage (Mouse), */ > 0xA1, 0x01, /* Collection (Application), */ > 0x85, 0x11, /* Report ID (17), */ > - 0x09, 0x21, /* Usage (Puck), */ > + 0x09, 0x01, /* Usage (Pointer), */ > 0xA0, /* Collection (Physical), */ > 0x05, 0x09, /* Usage Page (Button), */ > 0x19, 0x01, /* Usage Minimum (01h), */ > @@ -280,7 +284,7 @@ static const __u8 kye_tablet_rdesc[] = { > 0x95, 0x04, /* Report Count (4), */ > 0x81, 0x01, /* Input (Constant), */ > 0x05, 0x0D, /* Usage Page (Digitizer), */ > - 0x09, 0x32, /* Usage (In Range), */ > + 0x09, 0x37, /* Usage (Data Valid), */ > 0x95, 0x01, /* Report Count (1), */ > 0x81, 0x02, /* Input (Variable), */ > 0x05, 0x01, /* Usage Page (Desktop), */ > @@ -317,7 +321,7 @@ static const struct kye_tablet_info { > __s32 y_physical_maximum; > __s8 unit_exponent; > __s8 unit; > - bool has_punk; > + bool has_mouse; > unsigned int control_rsize; > const __u8 *control_rdesc; > } kye_tablets_info[] = { > @@ -402,7 +406,7 @@ static __u8 *kye_consumer_control_fixup(struct hid_device *hdev, __u8 *rdesc, > static __u8 *kye_tablet_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *rsize) > { > const struct kye_tablet_info *info; > - unsigned int newsize; > + __u8 *newdesc = rdesc; > > if (*rsize < sizeof(kye_tablet_rdesc)) { > hid_warn(hdev, > @@ -420,36 +424,45 @@ static __u8 *kye_tablet_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int > return rdesc; > } > > - newsize = info->has_punk ? sizeof(kye_tablet_rdesc) : 112; > - memcpy(rdesc, kye_tablet_rdesc, newsize); > - > - put_unaligned_le32(info->x_logical_maximum, rdesc + 66); > - put_unaligned_le32(info->x_physical_maximum, rdesc + 72); > - rdesc[77] = info->unit; > - rdesc[79] = info->unit_exponent; > - put_unaligned_le32(info->y_logical_maximum, rdesc + 87); > - put_unaligned_le32(info->y_physical_maximum, rdesc + 92); > - put_unaligned_le32(info->pressure_logical_maximum, rdesc + 104); > - > - if (info->has_punk) { > - put_unaligned_le32(info->x_logical_maximum, rdesc + 156); > - put_unaligned_le32(info->x_physical_maximum, rdesc + 162); > - rdesc[167] = info->unit; > - rdesc[169] = info->unit_exponent; > - put_unaligned_le32(info->y_logical_maximum, rdesc + 177); > - put_unaligned_le32(info->y_physical_maximum, rdesc + 182); > + memcpy(newdesc, kye_tablet_rdesc, sizeof(kye_tablet_rdesc)); > + > + put_unaligned_le32(info->x_logical_maximum, newdesc + 66); > + put_unaligned_le32(info->x_physical_maximum, newdesc + 72); > + newdesc[77] = info->unit; > + newdesc[79] = info->unit_exponent; > + put_unaligned_le32(info->y_logical_maximum, newdesc + 87); > + put_unaligned_le32(info->y_physical_maximum, newdesc + 92); > + put_unaligned_le32(info->pressure_logical_maximum, newdesc + 104); > + > + newdesc += sizeof(kye_tablet_rdesc); > + > + if (info->has_mouse) { > + if (newdesc + sizeof(kye_tablet_mouse_rdesc) > rdesc + *rsize) > + hid_err(hdev, "control desc unexpectedly large\n"); > + else { > + memcpy(newdesc, kye_tablet_mouse_rdesc, sizeof(kye_tablet_mouse_rdesc)); > + > + put_unaligned_le32(info->x_logical_maximum, newdesc + 44); > + put_unaligned_le32(info->x_physical_maximum, newdesc + 50); > + newdesc[55] = info->unit; > + newdesc[57] = info->unit_exponent; > + put_unaligned_le32(info->y_logical_maximum, newdesc + 65); > + put_unaligned_le32(info->y_physical_maximum, newdesc + 70); > + > + newdesc += sizeof(kye_tablet_mouse_rdesc); > + } > } > > if (info->control_rsize) { > - if (newsize + info->control_rsize > *rsize) > - hid_err(hdev, "control rdesc unexpectedly large"); > + if (newdesc + info->control_rsize > rdesc + *rsize) > + hid_err(hdev, "control desc unexpectedly large\n"); > else { > - memcpy(rdesc + newsize, info->control_rdesc, info->control_rsize); > - newsize += info->control_rsize; > + memcpy(newdesc, info->control_rdesc, info->control_rsize); > + newdesc += info->control_rsize; > } > } > > - *rsize = newsize; > + *rsize = newdesc - rdesc; > return rdesc; > } > > -- > 2.43.0 >
Hello Jiri, David, no objections here. Unfortunately, I can't test the change at the moment (I barely remember there was a mouse that could be used with that tablet), but the change sounds good to me. Also, thanks a lot for keeping the HID drivers up-to-date even for such old hardware :) Best, Milan On Mon, May 6, 2024 at 11:30 PM Jiri Kosina <jikos@kernel.org> wrote: > > On Thu, 2 May 2024, David Yang wrote: > > > Change device type because > > a. it is exactly a mouse, with left/right buttons and scroll wheel; > > b. it does not have visible marks or crosshairs, thus does not provide > > higher accuracy than stylus. > > Let's CC Milan, who originally added all this in feb6faf1e5d46 ("HID: kye: > Fix report descriptor for Genius PenSketch M912") ... Milan, any concerns > about the below? > > Thanks. > > > > > Signed-off-by: David Yang <mmyangfl@gmail.com> > > --- > > drivers/hid/hid-kye.c | 75 +++++++++++++++++++++++++------------------ > > 1 file changed, 44 insertions(+), 31 deletions(-) > > > > diff --git a/drivers/hid/hid-kye.c b/drivers/hid/hid-kye.c > > index eb9bf2829937..70ceb9437332 100644 > > --- a/drivers/hid/hid-kye.c > > +++ b/drivers/hid/hid-kye.c > > @@ -209,7 +209,7 @@ static const __u8 pensketch_t609a_control_rdesc[] = { > > 0xC0 /* End Collection */ > > }; > > > > -/* Fix indexes in kye_tablet_fixup if you change this */ > > +/* Fix indexes in kye_tablet_fixup() if you change this */ > > static const __u8 kye_tablet_rdesc[] = { > > 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */ > > 0x09, 0x01, /* Usage (01h), */ > > @@ -262,12 +262,16 @@ static const __u8 kye_tablet_rdesc[] = { > > 0x27, 0xFF, 0x07, 0x00, 0x00, /* Logical Maximum (2047), */ > > 0x81, 0x02, /* Input (Variable), */ > > 0xC0, /* End Collection, */ > > - 0xC0, /* End Collection, */ > > - 0x05, 0x0D, /* Usage Page (Digitizer), */ > > - 0x09, 0x21, /* Usage (Puck), */ > > + 0xC0 /* End Collection, */ > > +}; > > + > > +/* Fix indexes in kye_tablet_fixup() if you change this */ > > +static const __u8 kye_tablet_mouse_rdesc[] = { > > + 0x05, 0x01, /* Usage Page (Desktop), */ > > + 0x09, 0x02, /* Usage (Mouse), */ > > 0xA1, 0x01, /* Collection (Application), */ > > 0x85, 0x11, /* Report ID (17), */ > > - 0x09, 0x21, /* Usage (Puck), */ > > + 0x09, 0x01, /* Usage (Pointer), */ > > 0xA0, /* Collection (Physical), */ > > 0x05, 0x09, /* Usage Page (Button), */ > > 0x19, 0x01, /* Usage Minimum (01h), */ > > @@ -280,7 +284,7 @@ static const __u8 kye_tablet_rdesc[] = { > > 0x95, 0x04, /* Report Count (4), */ > > 0x81, 0x01, /* Input (Constant), */ > > 0x05, 0x0D, /* Usage Page (Digitizer), */ > > - 0x09, 0x32, /* Usage (In Range), */ > > + 0x09, 0x37, /* Usage (Data Valid), */ > > 0x95, 0x01, /* Report Count (1), */ > > 0x81, 0x02, /* Input (Variable), */ > > 0x05, 0x01, /* Usage Page (Desktop), */ > > @@ -317,7 +321,7 @@ static const struct kye_tablet_info { > > __s32 y_physical_maximum; > > __s8 unit_exponent; > > __s8 unit; > > - bool has_punk; > > + bool has_mouse; > > unsigned int control_rsize; > > const __u8 *control_rdesc; > > } kye_tablets_info[] = { > > @@ -402,7 +406,7 @@ static __u8 *kye_consumer_control_fixup(struct hid_device *hdev, __u8 *rdesc, > > static __u8 *kye_tablet_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *rsize) > > { > > const struct kye_tablet_info *info; > > - unsigned int newsize; > > + __u8 *newdesc = rdesc; > > > > if (*rsize < sizeof(kye_tablet_rdesc)) { > > hid_warn(hdev, > > @@ -420,36 +424,45 @@ static __u8 *kye_tablet_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int > > return rdesc; > > } > > > > - newsize = info->has_punk ? sizeof(kye_tablet_rdesc) : 112; > > - memcpy(rdesc, kye_tablet_rdesc, newsize); > > - > > - put_unaligned_le32(info->x_logical_maximum, rdesc + 66); > > - put_unaligned_le32(info->x_physical_maximum, rdesc + 72); > > - rdesc[77] = info->unit; > > - rdesc[79] = info->unit_exponent; > > - put_unaligned_le32(info->y_logical_maximum, rdesc + 87); > > - put_unaligned_le32(info->y_physical_maximum, rdesc + 92); > > - put_unaligned_le32(info->pressure_logical_maximum, rdesc + 104); > > - > > - if (info->has_punk) { > > - put_unaligned_le32(info->x_logical_maximum, rdesc + 156); > > - put_unaligned_le32(info->x_physical_maximum, rdesc + 162); > > - rdesc[167] = info->unit; > > - rdesc[169] = info->unit_exponent; > > - put_unaligned_le32(info->y_logical_maximum, rdesc + 177); > > - put_unaligned_le32(info->y_physical_maximum, rdesc + 182); > > + memcpy(newdesc, kye_tablet_rdesc, sizeof(kye_tablet_rdesc)); > > + > > + put_unaligned_le32(info->x_logical_maximum, newdesc + 66); > > + put_unaligned_le32(info->x_physical_maximum, newdesc + 72); > > + newdesc[77] = info->unit; > > + newdesc[79] = info->unit_exponent; > > + put_unaligned_le32(info->y_logical_maximum, newdesc + 87); > > + put_unaligned_le32(info->y_physical_maximum, newdesc + 92); > > + put_unaligned_le32(info->pressure_logical_maximum, newdesc + 104); > > + > > + newdesc += sizeof(kye_tablet_rdesc); > > + > > + if (info->has_mouse) { > > + if (newdesc + sizeof(kye_tablet_mouse_rdesc) > rdesc + *rsize) > > + hid_err(hdev, "control desc unexpectedly large\n"); > > + else { > > + memcpy(newdesc, kye_tablet_mouse_rdesc, sizeof(kye_tablet_mouse_rdesc)); > > + > > + put_unaligned_le32(info->x_logical_maximum, newdesc + 44); > > + put_unaligned_le32(info->x_physical_maximum, newdesc + 50); > > + newdesc[55] = info->unit; > > + newdesc[57] = info->unit_exponent; > > + put_unaligned_le32(info->y_logical_maximum, newdesc + 65); > > + put_unaligned_le32(info->y_physical_maximum, newdesc + 70); > > + > > + newdesc += sizeof(kye_tablet_mouse_rdesc); > > + } > > } > > > > if (info->control_rsize) { > > - if (newsize + info->control_rsize > *rsize) > > - hid_err(hdev, "control rdesc unexpectedly large"); > > + if (newdesc + info->control_rsize > rdesc + *rsize) > > + hid_err(hdev, "control desc unexpectedly large\n"); > > else { > > - memcpy(rdesc + newsize, info->control_rdesc, info->control_rsize); > > - newsize += info->control_rsize; > > + memcpy(newdesc, info->control_rdesc, info->control_rsize); > > + newdesc += info->control_rsize; > > } > > } > > > > - *rsize = newsize; > > + *rsize = newdesc - rdesc; > > return rdesc; > > } > > > > -- > > 2.43.0 > > > > -- > Jiri Kosina > SUSE Labs >
On Tue, 7 May 2024, Milan Plžík wrote: > no objections here. Unfortunately, I can't test the change at the moment > (I barely remember there was a mouse that could be used with that > tablet), but the change sounds good to me. Also, thanks a lot for > keeping the HID drivers up-to-date even for such old hardware :) Thanks for the prompt response. I've applied David's patch now.
diff --git a/drivers/hid/hid-kye.c b/drivers/hid/hid-kye.c index eb9bf2829937..70ceb9437332 100644 --- a/drivers/hid/hid-kye.c +++ b/drivers/hid/hid-kye.c @@ -209,7 +209,7 @@ static const __u8 pensketch_t609a_control_rdesc[] = { 0xC0 /* End Collection */ }; -/* Fix indexes in kye_tablet_fixup if you change this */ +/* Fix indexes in kye_tablet_fixup() if you change this */ static const __u8 kye_tablet_rdesc[] = { 0x06, 0x00, 0xFF, /* Usage Page (FF00h), */ 0x09, 0x01, /* Usage (01h), */ @@ -262,12 +262,16 @@ static const __u8 kye_tablet_rdesc[] = { 0x27, 0xFF, 0x07, 0x00, 0x00, /* Logical Maximum (2047), */ 0x81, 0x02, /* Input (Variable), */ 0xC0, /* End Collection, */ - 0xC0, /* End Collection, */ - 0x05, 0x0D, /* Usage Page (Digitizer), */ - 0x09, 0x21, /* Usage (Puck), */ + 0xC0 /* End Collection, */ +}; + +/* Fix indexes in kye_tablet_fixup() if you change this */ +static const __u8 kye_tablet_mouse_rdesc[] = { + 0x05, 0x01, /* Usage Page (Desktop), */ + 0x09, 0x02, /* Usage (Mouse), */ 0xA1, 0x01, /* Collection (Application), */ 0x85, 0x11, /* Report ID (17), */ - 0x09, 0x21, /* Usage (Puck), */ + 0x09, 0x01, /* Usage (Pointer), */ 0xA0, /* Collection (Physical), */ 0x05, 0x09, /* Usage Page (Button), */ 0x19, 0x01, /* Usage Minimum (01h), */ @@ -280,7 +284,7 @@ static const __u8 kye_tablet_rdesc[] = { 0x95, 0x04, /* Report Count (4), */ 0x81, 0x01, /* Input (Constant), */ 0x05, 0x0D, /* Usage Page (Digitizer), */ - 0x09, 0x32, /* Usage (In Range), */ + 0x09, 0x37, /* Usage (Data Valid), */ 0x95, 0x01, /* Report Count (1), */ 0x81, 0x02, /* Input (Variable), */ 0x05, 0x01, /* Usage Page (Desktop), */ @@ -317,7 +321,7 @@ static const struct kye_tablet_info { __s32 y_physical_maximum; __s8 unit_exponent; __s8 unit; - bool has_punk; + bool has_mouse; unsigned int control_rsize; const __u8 *control_rdesc; } kye_tablets_info[] = { @@ -402,7 +406,7 @@ static __u8 *kye_consumer_control_fixup(struct hid_device *hdev, __u8 *rdesc, static __u8 *kye_tablet_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int *rsize) { const struct kye_tablet_info *info; - unsigned int newsize; + __u8 *newdesc = rdesc; if (*rsize < sizeof(kye_tablet_rdesc)) { hid_warn(hdev, @@ -420,36 +424,45 @@ static __u8 *kye_tablet_fixup(struct hid_device *hdev, __u8 *rdesc, unsigned int return rdesc; } - newsize = info->has_punk ? sizeof(kye_tablet_rdesc) : 112; - memcpy(rdesc, kye_tablet_rdesc, newsize); - - put_unaligned_le32(info->x_logical_maximum, rdesc + 66); - put_unaligned_le32(info->x_physical_maximum, rdesc + 72); - rdesc[77] = info->unit; - rdesc[79] = info->unit_exponent; - put_unaligned_le32(info->y_logical_maximum, rdesc + 87); - put_unaligned_le32(info->y_physical_maximum, rdesc + 92); - put_unaligned_le32(info->pressure_logical_maximum, rdesc + 104); - - if (info->has_punk) { - put_unaligned_le32(info->x_logical_maximum, rdesc + 156); - put_unaligned_le32(info->x_physical_maximum, rdesc + 162); - rdesc[167] = info->unit; - rdesc[169] = info->unit_exponent; - put_unaligned_le32(info->y_logical_maximum, rdesc + 177); - put_unaligned_le32(info->y_physical_maximum, rdesc + 182); + memcpy(newdesc, kye_tablet_rdesc, sizeof(kye_tablet_rdesc)); + + put_unaligned_le32(info->x_logical_maximum, newdesc + 66); + put_unaligned_le32(info->x_physical_maximum, newdesc + 72); + newdesc[77] = info->unit; + newdesc[79] = info->unit_exponent; + put_unaligned_le32(info->y_logical_maximum, newdesc + 87); + put_unaligned_le32(info->y_physical_maximum, newdesc + 92); + put_unaligned_le32(info->pressure_logical_maximum, newdesc + 104); + + newdesc += sizeof(kye_tablet_rdesc); + + if (info->has_mouse) { + if (newdesc + sizeof(kye_tablet_mouse_rdesc) > rdesc + *rsize) + hid_err(hdev, "control desc unexpectedly large\n"); + else { + memcpy(newdesc, kye_tablet_mouse_rdesc, sizeof(kye_tablet_mouse_rdesc)); + + put_unaligned_le32(info->x_logical_maximum, newdesc + 44); + put_unaligned_le32(info->x_physical_maximum, newdesc + 50); + newdesc[55] = info->unit; + newdesc[57] = info->unit_exponent; + put_unaligned_le32(info->y_logical_maximum, newdesc + 65); + put_unaligned_le32(info->y_physical_maximum, newdesc + 70); + + newdesc += sizeof(kye_tablet_mouse_rdesc); + } } if (info->control_rsize) { - if (newsize + info->control_rsize > *rsize) - hid_err(hdev, "control rdesc unexpectedly large"); + if (newdesc + info->control_rsize > rdesc + *rsize) + hid_err(hdev, "control desc unexpectedly large\n"); else { - memcpy(rdesc + newsize, info->control_rdesc, info->control_rsize); - newsize += info->control_rsize; + memcpy(newdesc, info->control_rdesc, info->control_rsize); + newdesc += info->control_rsize; } } - *rsize = newsize; + *rsize = newdesc - rdesc; return rdesc; }
Change device type because a. it is exactly a mouse, with left/right buttons and scroll wheel; b. it does not have visible marks or crosshairs, thus does not provide higher accuracy than stylus. Signed-off-by: David Yang <mmyangfl@gmail.com> --- drivers/hid/hid-kye.c | 75 +++++++++++++++++++++++++------------------ 1 file changed, 44 insertions(+), 31 deletions(-)