| Message ID | 1354634875-5182-10-git-send-email-benjamin.tissoires@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Jiri Kosina |
| Headers | show |
On Tue, 4 Dec 2012 16:27:50 +0100, Benjamin Tissoires wrote: > If i2c_hid_get_report fails, exit i2c_hid_init_report. > The printk log is already called by i2c_hid_get_report, so no need > to add some more printks. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com> > --- > drivers/hid/i2c-hid/i2c-hid.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c > index da04948..dcacfc4 100644 > --- a/drivers/hid/i2c-hid/i2c-hid.c > +++ b/drivers/hid/i2c-hid/i2c-hid.c > @@ -400,9 +400,10 @@ static void i2c_hid_init_report(struct hid_report *report, u8 *buffer, > unsigned int size, ret_size; > > size = i2c_hid_get_report_length(report); > - i2c_hid_get_report(client, > + if (i2c_hid_get_report(client, > report->type == HID_FEATURE_REPORT ? 0x03 : 0x01, > - report->id, buffer, size); > + report->id, buffer, size)) > + return; > > i2c_hid_dbg(ihid, "report (len=%d): %*ph\n", size, size, ihid->inbuf); > OK, although I don't quite get the rationale for not reporting the errors from i2c_hid_init_report() and i2c_hid_init_reports() to their respective callers. Does the device have any chance to work properly if i2c_hid_init_reports() fails? Reviewed-by: Jean Delvare <khali@linux-fr.org>
On Wed, Dec 5, 2012 at 10:59 AM, Jean Delvare <khali@linux-fr.org> wrote: > On Tue, 4 Dec 2012 16:27:50 +0100, Benjamin Tissoires wrote: >> If i2c_hid_get_report fails, exit i2c_hid_init_report. >> The printk log is already called by i2c_hid_get_report, so no need >> to add some more printks. >> >> Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com> >> --- >> drivers/hid/i2c-hid/i2c-hid.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c >> index da04948..dcacfc4 100644 >> --- a/drivers/hid/i2c-hid/i2c-hid.c >> +++ b/drivers/hid/i2c-hid/i2c-hid.c >> @@ -400,9 +400,10 @@ static void i2c_hid_init_report(struct hid_report *report, u8 *buffer, >> unsigned int size, ret_size; >> >> size = i2c_hid_get_report_length(report); >> - i2c_hid_get_report(client, >> + if (i2c_hid_get_report(client, >> report->type == HID_FEATURE_REPORT ? 0x03 : 0x01, >> - report->id, buffer, size); >> + report->id, buffer, size)) >> + return; >> >> i2c_hid_dbg(ihid, "report (len=%d): %*ph\n", size, size, ihid->inbuf); >> > > OK, although I don't quite get the rationale for not reporting the > errors from i2c_hid_init_report() and i2c_hid_init_reports() to their > respective callers. Does the device have any chance to work properly if > i2c_hid_init_reports() fails? Hi Jean, first thanks for the review you have started. For your question, the answer is yes, the device works properly even if i2c_hid_init_reports(). IIRC, the only required steps are: - get HID descriptor - get HID report descriptors - send reset - set power on i2c_hid_init_reports is not part of the specification for the boot process, and the Windows driver does not retrieve the reports for input reports at all (it does it though for the features). Actually, the device I have does not implement get_report for inputs (it returns 0), but it's not a problem for it to work. I put the whole init_reports in place to get the features values before sending them to the hid driver, and also to copy the behavior of usbhid. Cheers, Benjamin > > Reviewed-by: Jean Delvare <khali@linux-fr.org> > > -- > Jean Delvare -- To unsubscribe from this list: send the line "unsubscribe linux-input" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 4 Dec 2012, Benjamin Tissoires wrote: > If i2c_hid_get_report fails, exit i2c_hid_init_report. > The printk log is already called by i2c_hid_get_report, so no need > to add some more printks. > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com> > --- > drivers/hid/i2c-hid/i2c-hid.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c > index da04948..dcacfc4 100644 > --- a/drivers/hid/i2c-hid/i2c-hid.c > +++ b/drivers/hid/i2c-hid/i2c-hid.c > @@ -400,9 +400,10 @@ static void i2c_hid_init_report(struct hid_report *report, u8 *buffer, > unsigned int size, ret_size; > > size = i2c_hid_get_report_length(report); > - i2c_hid_get_report(client, > + if (i2c_hid_get_report(client, > report->type == HID_FEATURE_REPORT ? 0x03 : 0x01, > - report->id, buffer, size); > + report->id, buffer, size)) > + return; > > i2c_hid_dbg(ihid, "report (len=%d): %*ph\n", size, size, ihid->inbuf); Applied.
diff --git a/drivers/hid/i2c-hid/i2c-hid.c b/drivers/hid/i2c-hid/i2c-hid.c index da04948..dcacfc4 100644 --- a/drivers/hid/i2c-hid/i2c-hid.c +++ b/drivers/hid/i2c-hid/i2c-hid.c @@ -400,9 +400,10 @@ static void i2c_hid_init_report(struct hid_report *report, u8 *buffer, unsigned int size, ret_size; size = i2c_hid_get_report_length(report); - i2c_hid_get_report(client, + if (i2c_hid_get_report(client, report->type == HID_FEATURE_REPORT ? 0x03 : 0x01, - report->id, buffer, size); + report->id, buffer, size)) + return; i2c_hid_dbg(ihid, "report (len=%d): %*ph\n", size, size, ihid->inbuf);
If i2c_hid_get_report fails, exit i2c_hid_init_report. The printk log is already called by i2c_hid_get_report, so no need to add some more printks. Signed-off-by: Benjamin Tissoires <benjamin.tissoires@gmail.com> --- drivers/hid/i2c-hid/i2c-hid.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)