RE: [PATCH] i2c-hid: fix length for set/get report in i2c hid

[Kankroliwala, Huzefa NomanX]

Hi Benjamin,
Thanks for reviewing!

> -----Original Message-----
> From: Benjamin Tissoires [mailto:benjamin.tissoires@xxxxxxxxx]
> Sent: Wednesday, April 03, 2013 2:00 AM
> To: Kankroliwala, Huzefa NomanX
> Cc: jkosina@xxxxxxx; Westerberg, Mika; linux-kernel@xxxxxxxxxxxxxxx
> Subject: Re: [PATCH] i2c-hid: fix length for set/get report in i2c hid
> 
> Hi Huzefa,
> 
> On Tue, Apr 2, 2013 at 12:52 PM, Huzefa Kankroliwala
> <huzefa.nomanx.kankroliwala@xxxxxxxxx> wrote:
> > With the current i2c hid driver set/get report does not work as
> > expected, for e.g sensor hub properties like power state, frequency
> > etc is not set properly on the device as a result we do not get
> > events.
> > The problem is that i2c hid driver in function i2c_hid_request sets
> > length equal to default buffer size for which the sensor hub does not
> > respond on get/set commands. Use report length and calculate it based
> > on report size and id.
> 
> oops, seems like I've been too lazy to get the actual size. Thanks for fixing it.
> 
> I just have one small formatting comment (see below).
> Except that, Reviewed-by: Benjamin Tissoires
> <benjamin.tissoires@xxxxxxxxxx>
> 
> >
> > Reviewed-by: Mika Westerberg <mika.westerberg@xxxxxxxxxxxxxxx>
> > Signed-off-by: Huzefa Kankroliwala
> > <huzefa.nomanx.kankroliwala@xxxxxxxxx>
> > ---
> >  drivers/hid/i2c-hid/i2c-hid.c |   10 +++++-----
> >  1 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/hid/i2c-hid/i2c-hid.c
> > b/drivers/hid/i2c-hid/i2c-hid.c index 935f387..c8cd0c0 100644
> > --- a/drivers/hid/i2c-hid/i2c-hid.c
> > +++ b/drivers/hid/i2c-hid/i2c-hid.c
> > @@ -567,18 +567,18 @@ static void i2c_hid_request(struct hid_device
> *hid, struct hid_report *rep,
> >                 int reqtype)
> >  {
> >         struct i2c_client *client = hid->driver_data;
> > -       struct i2c_hid *ihid = i2c_get_clientdata(client);
> >         char *buf;
> >         int ret;
> > +       int len = i2c_hid_get_report_length(rep) - 2;
> >
> > -       buf = kzalloc(ihid->bufsize, GFP_KERNEL);
> > +       buf = kzalloc(len, GFP_KERNEL);
> >         if (!buf)
> >                 return;
> >
> >         switch (reqtype) {
> >         case HID_REQ_GET_REPORT:
> > -               ret = i2c_hid_get_raw_report(hid, rep->id, buf, ihid->bufsize,
> > -                                            rep->type);
> > +               ret = i2c_hid_get_raw_report(hid, rep->id, buf, len,
> > +                                                       rep->type);
> 
> No need to add a new line for rep->type when the 80 columns limit is not
> reached.
Fixed this in Patch v2

Thanks
Huzefa
> 
> Cheers,
> Benjamin
> 
> >                 if (ret < 0)
> >                         dev_err(&client->dev, "%s: unable to get report: %d\n",
> >                                 __func__, ret); @@ -587,7 +587,7 @@
> > static void i2c_hid_request(struct hid_device *hid, struct hid_report *rep,
> >                 break;
> >         case HID_REQ_SET_REPORT:
> >                 hid_output_report(rep, buf);
> > -               i2c_hid_output_raw_report(hid, buf, ihid->bufsize, rep->type);
> > +               i2c_hid_output_raw_report(hid, buf, len, rep->type);
> >                 break;
> >         }
> >
> > --
> > 1.7.4.1
> >
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/