Hi Benjamin, > -----Original Message----- > From: Benjamin Tissoires [mailto:benjamin.tissoires@xxxxxxxxx] > Sent: Thursday, October 17, 2013 11:27 PM > To: Bibek Basu > Cc: Jiri Kosina; linux-input; linux-kernel@xxxxxxxxxxxxxxx > Subject: Re: [PATCH] HID: i2c-hid: Stop querying for init reports > > Hi Bibek, > > On Tue, Oct 15, 2013 at 4:28 AM, Bibek Basu <bbasu@xxxxxxxxxx> wrote: > > According to specifications, HID over I2C devices are not bound to > > respond to query for INPUT REPORTS. Thus dropping the call during init > > as many devices does not respond causing error messages during boot. > > > > This time, the patch is removing too many things that are correct. :) > > see below to know what to remove and what to keep: > > > Signed-off-by: Bibek Basu <bbasu@xxxxxxxxxx> > > --- > > drivers/hid/i2c-hid/i2c-hid.c | 59 > > ------------------------------------------- > > 1 file changed, 59 deletions(-) > > > > diff --git a/drivers/hid/i2c-hid/i2c-hid.c > > b/drivers/hid/i2c-hid/i2c-hid.c index fd7ce37..58a4f12 100644 > > --- a/drivers/hid/i2c-hid/i2c-hid.c > > +++ b/drivers/hid/i2c-hid/i2c-hid.c > > @@ -409,62 +409,6 @@ static int i2c_hid_get_report_length(struct > hid_report *report) > > report->device->report_enum[report->type].numbered + > > 2; } > > > > -static void i2c_hid_init_report(struct hid_report *report, u8 *buffer, > > - size_t bufsize) > > -{ > > - struct hid_device *hid = report->device; > > - struct i2c_client *client = hid->driver_data; > > - struct i2c_hid *ihid = i2c_get_clientdata(client); > > - unsigned int size, ret_size; > > - > > - size = i2c_hid_get_report_length(report); > > - if (i2c_hid_get_report(client, > > - report->type == HID_FEATURE_REPORT ? 0x03 : 0x01, > > - report->id, buffer, size)) > > - return; > > - > > - i2c_hid_dbg(ihid, "report (len=%d): %*ph\n", size, size, ihid->inbuf); > > - > > - ret_size = buffer[0] | (buffer[1] << 8); > > - > > - if (ret_size != size) { > > - dev_err(&client->dev, "error in %s size:%d / ret_size:%d\n", > > - __func__, size, ret_size); > > - return; > > - } > > - > > - /* hid->driver_lock is held as we are in probe function, > > - * we just need to setup the input fields, so using > > - * hid_report_raw_event is safe. */ > > - hid_report_raw_event(hid, report->type, buffer + 2, size - 2, 1); > > -} > > This function should be kept > > > - > > -/* > > - * Initialize all reports > > - */ > > -static void i2c_hid_init_reports(struct hid_device *hid) -{ > > - struct hid_report *report; > > - struct i2c_client *client = hid->driver_data; > > - struct i2c_hid *ihid = i2c_get_clientdata(client); > > - u8 *inbuf = kzalloc(ihid->bufsize, GFP_KERNEL); > > - > > - if (!inbuf) { > > - dev_err(&client->dev, "can not retrieve initial reports\n"); > > - return; > > - } > > - > > Above should be kept > > > - list_for_each_entry(report, > > - &hid->report_enum[HID_INPUT_REPORT].report_list, list) > > - i2c_hid_init_report(report, inbuf, ihid->bufsize); > > - > > these for lines should be removed (they are the one giving the errors in the > logs) > > and please keep the rest of the code as is. > > > > - list_for_each_entry(report, > > - &hid->report_enum[HID_FEATURE_REPORT].report_list, list) > > - i2c_hid_init_report(report, inbuf, ihid->bufsize); > > Actually, this part is very important because we have no spontaneous events > emitted by features, so we don't know the value of the feature until we > probed it. So please keep this part as mentioned above. Thanks for the inputs. I am repushing the patch. Unfortunately, looks like I need to upgrade the firmware of my HID keyboard, as it does not responds to the FEATURE_REPORT query:( regards Bibek > > Cheers, > Benjamin > > > - > > - kfree(inbuf); > > -} > > - > > /* > > * Traverse the supplied list of reports and find the longest > > */ > > @@ -683,9 +627,6 @@ static int i2c_hid_start(struct hid_device *hid) > > return ret; > > } > > > > - if (!(hid->quirks & HID_QUIRK_NO_INIT_REPORTS)) > > - i2c_hid_init_reports(hid); > > - > > return 0; > > } > > > > -- > > 1.8.1.5 > > ��.n��������+%������w��{.n�����{��)��^n�r������&��z�ޗ�zf���h���~����������_��+v���)ߣ�
