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.
Reviewed-by: Mika Westerberg <[email protected]>
Signed-off-by: Huzefa Kankroliwala <[email protected]>
---
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);
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
Hi Huzefa,
On Tue, Apr 2, 2013 at 12:52 PM, Huzefa Kankroliwala
<[email protected]> 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 <[email protected]>
>
> Reviewed-by: Mika Westerberg <[email protected]>
> Signed-off-by: Huzefa Kankroliwala <[email protected]>
> ---
> 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.
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
>
Hi Benjamin,
Thanks for reviewing!
> -----Original Message-----
> From: Benjamin Tissoires [mailto:[email protected]]
> Sent: Wednesday, April 03, 2013 2:00 AM
> To: Kankroliwala, Huzefa NomanX
> Cc: [email protected]; Westerberg, Mika; [email protected]
> 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
> <[email protected]> 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
> <[email protected]>
>
> >
> > Reviewed-by: Mika Westerberg <[email protected]>
> > Signed-off-by: Huzefa Kankroliwala
> > <[email protected]>
> > ---
> > 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
> >