| Message ID | 20240909203208.47339-1-jason.gerecke@wacom.com (mailing list archive) |
|---|---|
| State | New |
| Delegated to: | Jiri Kosina |
| Headers | show |
| Series | [1/2] HID: wacom: Support sequence numbers smaller than 16-bit | expand |
On Mon, 9 Sep 2024, Gerecke, Jason wrote: > From: Jason Gerecke <jason.gerecke@wacom.com> > > The current dropped packet reporting assumes that all sequence numbers > are 16 bits in length. This results in misleading "Dropped" messages if > the hardware uses fewer bits. For example, if a tablet uses only 8 bits > to store its sequence number, once it rolls over from 255 -> 0, the > driver will still be expecting a packet "256". This patch adjusts the > logic to reset the next expected packet to logical_minimum whenever > it overflows beyond logical_maximum. > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> > Tested-by: Joshua Dickens <joshua.dickens@wacom.com> > Fixes: 6d09085b38e5 ("HID: wacom: Adding Support for new usages") Hi Jason, thanks for both fixes. Looking at them, it seems like it'd normally be 6.11 material, but given - how far we currently are in a 6.11 development cycle - how long this issue has been present in the code - the fact that it actually does change the event processing behavior I have to ask you for a judgement -- would you like to absolutely see these in 6.11 as last-minute fixes, or do you consider this to be 6.12 material? Thanks,
On Mon, Sep 9, 2024 at 1:40 PM Jiri Kosina <jikos@kernel.org> wrote: > > On Mon, 9 Sep 2024, Gerecke, Jason wrote: > > > From: Jason Gerecke <jason.gerecke@wacom.com> > > > > The current dropped packet reporting assumes that all sequence numbers > > are 16 bits in length. This results in misleading "Dropped" messages if > > the hardware uses fewer bits. For example, if a tablet uses only 8 bits > > to store its sequence number, once it rolls over from 255 -> 0, the > > driver will still be expecting a packet "256". This patch adjusts the > > logic to reset the next expected packet to logical_minimum whenever > > it overflows beyond logical_maximum. > > > > Signed-off-by: Jason Gerecke <jason.gerecke@wacom.com> > > Tested-by: Joshua Dickens <joshua.dickens@wacom.com> > > Fixes: 6d09085b38e5 ("HID: wacom: Adding Support for new usages") > > Hi Jason, > > thanks for both fixes. > > Looking at them, it seems like it'd normally be 6.11 material, but given > > - how far we currently are in a 6.11 development cycle > - how long this issue has been present in the code > - the fact that it actually does change the event processing behavior > > I have to ask you for a judgement -- would you like to absolutely see > these in 6.11 as last-minute fixes, or do you consider this to be 6.12 > material? > > Thanks, > > -- > Jiri Kosina > SUSE Labs > Hi Jiri, These are pretty minor fixes, so I agree that there is no urgent need to get them into 6.11. Feel free to queue them for 6.12 instead. Jason (she/they) --- Now instead of four in the eights place / you’ve got three, ‘Cause you added one / (That is to say, eight) to the two, / But you can’t take seven from three, / So you look at the sixty-fours....
On Tue, 10 Sep 2024, Jason Gerecke wrote: > These are pretty minor fixes, so I agree that there is no urgent need > to get them into 6.11. Feel free to queue them for 6.12 instead. Thanks. Both now queued in hid.git#for-6.12/wacom.
diff --git a/drivers/hid/wacom_wac.c b/drivers/hid/wacom_wac.c index 74258a78d0408..cef08737a6240 100644 --- a/drivers/hid/wacom_wac.c +++ b/drivers/hid/wacom_wac.c @@ -2512,9 +2512,14 @@ static void wacom_wac_pen_event(struct hid_device *hdev, struct hid_field *field wacom_wac->hid_data.barrelswitch3 = value; return; case WACOM_HID_WD_SEQUENCENUMBER: - if (wacom_wac->hid_data.sequence_number != value) - hid_warn(hdev, "Dropped %hu packets", (unsigned short)(value - wacom_wac->hid_data.sequence_number)); + if (wacom_wac->hid_data.sequence_number != value) { + int sequence_size = field->logical_maximum - field->logical_minimum + 1; + int drop_count = (value - wacom_wac->hid_data.sequence_number) % sequence_size; + hid_warn(hdev, "Dropped %d packets", drop_count); + } wacom_wac->hid_data.sequence_number = value + 1; + if (wacom_wac->hid_data.sequence_number > field->logical_maximum) + wacom_wac->hid_data.sequence_number = field->logical_minimum; return; }