Hi!

> > > 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().
>

Yep, you are right. I failed to notice the difference between _PAD_
and _PEN_ macros, and so the code did not make sense.

Sorry for the noise.

Best regards,
                                                                Pavel
                                                                
-- 
http://www.livejournal.com/~pavelmachek

Attachment: signature.asc
Description: PGP signature

Reply via email to