Re: [PATCH 1/2] input: wacom - retrieve maximum number of touch points

[Dmitry Torokhov <dmitry.torokhov@gmail.com>]

On Tue, Apr 24, 2012 at 01:46:15PM -0700, Ping Cheng wrote:
> On Fri, Apr 20, 2012 at 10:56 PM, Dmitry Torokhov <dmitry.torokhov@gmail.com
> > wrote:
> 
> > Hi Chris, Ping,
> >
> > On Sun, Apr 15, 2012 at 05:50:23PM -0500, chris@cnpbagwell.com wrote:
> > > From: Ping Cheng <pinglinux@gmail.com>
> > >
> > > From the HID usage table when it is supported.
> > >
> > > Tested-by: Jason Gerecke <killertofu@gmail.com>
> > > Signed-off-by: Chris Bagwell <chris@cnpbagwell.com>
> > > Signed-off-by: Ping Cheng <pingc@wacom.com>
> > > ---
> > >  drivers/input/tablet/wacom_sys.c |   22 +++++++++++++-
> > >  drivers/input/tablet/wacom_wac.c |   59
> > +++++++++++++++++++++++---------------
> > >  drivers/input/tablet/wacom_wac.h |    1 +
> > >  3 files changed, 58 insertions(+), 24 deletions(-)
> > >
> > > diff --git a/drivers/input/tablet/wacom_sys.c
> > b/drivers/input/tablet/wacom_sys.c
> > > index 1c874ad..469f6ce 100644
> > > --- a/drivers/input/tablet/wacom_sys.c
> > > +++ b/drivers/input/tablet/wacom_sys.c
> > > @@ -28,6 +28,7 @@
> > >  #define HID_USAGE_Y_TILT             0x3e
> > >  #define HID_USAGE_FINGER             0x22
> > >  #define HID_USAGE_STYLUS             0x20
> > > +#define HID_USAGE_CONTACTMAX         0x55
> > >  #define HID_COLLECTION                       0xa1
> > >  #define HID_COLLECTION_LOGICAL               0x02
> > >  #define HID_COLLECTION_END           0xc0
> > > @@ -201,6 +202,20 @@ static int wacom_parse_logical_collection(unsigned
> > char *report,
> > >       return length;
> > >  }
> > >
> > > +static void wacom_retrieve_report_data(struct usb_interface *intf,
> > > +                                    struct wacom_features *features)
> > > +{
> > > +     int result = 0;
> > > +     unsigned char rep_data[2];
> >
> > You can't use on-stack buffers with USB. I see we allocating memory
> > elsewhere but I wonder if we should simply piggyback on wacom_wac->data
> > in all these interrogation methods.
> >
> 
> When you say piggyback on wacom_wac->data, I guess you meant using the same
> wacom_wac->data instead of creating a new entry for the others. However,
> wacom_wac->data has a sizeof(struct wacom_usbdev_data). The existing
> kzalloc requests in wacom_sys.c range from 2 to 259, assuming we do not
> consider those kzallocs that handle variable sizes, such as
> hid_desc->wDescriptorLength.
> 
> What size would be reasonable for the new data? I feel we need to introduce
> a new entry in wacom_wac if we want to unify the kzalloc request a bit.

Hmm, maybe piggybacking was not such a good idea. We want to keep
buffers used by leds separate since these operations can appen
simultaneously and since we also have to deal with variable length of
hid descriptors I think we should just kmalloc memory in
wacom_retrieve_report_data() and be done with it.

-- 
Dmitry