> From: Pavel Machek <pa...@ucw.cz>
> Sent: Tuesday, November 3, 2020 11:52 PM
>
> > To correct this, we restore a version of the `WACOM_PAD_FIELD` check
> > in `wacom_wac_collection()` and return early. This effectively prevents
> > pad / battery collections from being reported until the very end of the
> > report as originally intended.
> 
> Okay... but code is either wrong or very confusing:
> 
> > +++ b/drivers/hid/wacom_wac.c
> > @@ -2729,7 +2729,9 @@ static int wacom_wac_collection(struct h
> >        if (report->type != HID_INPUT_REPORT)
> >                return -1;
> >  
> > -     if (WACOM_PEN_FIELD(field) && wacom->wacom_wac.pen_input)
> > +     if (WACOM_PAD_FIELD(field))
> > +             return 0;
> > +     else if (WACOM_PEN_FIELD(field) && wacom->wacom_wac.pen_input)
> >                wacom_wac_pen_report(hdev, report);
> 
> wacom_wac_pen_report() can never be called, because WACOM_PEN_FIELD()
> can not be true here; if it was we'd return in the line above.

For reference, here's the definition for WACOM_PAD_FIELD() and 
WACOM_PEN_FIELD():

> #define WACOM_PAD_FIELD(f)    (((f)->physical == HID_DG_TABLETFUNCTIONKEY) || 
> \
>                                ((f)->physical == 
> WACOM_HID_WD_DIGITIZERFNKEYS) || \
>                                ((f)->physical == WACOM_HID_WD_DIGITIZERINFO))
> 
> #define WACOM_PEN_FIELD(f)    (((f)->logical == HID_DG_STYLUS) || \
>                                ((f)->physical == HID_DG_STYLUS) || \
>                                ((f)->physical == HID_DG_PEN) || \
>                                ((f)->application == HID_DG_PEN) || \
>                                ((f)->application == HID_DG_DIGITIZER) || \
>                                ((f)->application == WACOM_HID_WD_PEN) || \
>                                ((f)->application == WACOM_HID_WD_DIGITIZER) 
> || \
>                                ((f)->application == WACOM_HID_G9_PEN) || \
>                                ((f)->application == WACOM_HID_G11_PEN))

WACOM_PAD_FIELD() evaluates to `true` for pad data *not* pen data because pen 
data is not inside any of the 3 physical collections its looks for.

WACOM_PEN_FIELD() evaluates to `true` for pad data *and* pen data because both 
types of data are inside of the Digitizer application collection.

Without the WACOM_PAD_FIELD() check in place at the very beginning, both pad 
and pen data would trigger a call to wacom_wac_pen_report(). This is undesired: 
only pen data should result in that function being called. Adding the check 
causes the function to return early for pad data while pen data falls into the 
"else if" and is processed as before. Pad data is only reported once the entire 
report has been valuated by making a call to wacom_wac_pad_report() at the very 
end of wacom_wac_report().

Jason Gerecke, Senior Linux Software Engineer
Wacom Technology Corporation
tel: 503-525-3100 ext. 3229 (direct)
http://www.wacom.com

Reply via email to